Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
version: "2"
run:
timeout: 15m # cold CI: go/packages load can approach 5m before linters run
modules-download-mode: readonly
issues-exit-code: 1
tests: true
Expand Down
2 changes: 1 addition & 1 deletion hack/verify-golint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ for module in $(find . -name "go.mod" | xargs -n1 dirname); do
-e GOMODCACHE="/gomodcache" \
-e GOCACHE="/gocache" \
"golangci/golangci-lint:$VERSION" \
golangci-lint run -v --timeout=5m || failed=true
golangci-lint run -v --timeout=15m || failed=true
done

if ${failed}; then
Expand Down
61 changes: 61 additions & 0 deletions k8s/client/clientset/versioned/fake/clientset_constructor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
Copyright The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package fake

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/watch"
fakediscovery "k8s.io/client-go/discovery/fake"
"k8s.io/client-go/testing"
)

// NewClientset returns a fake Clientset seeded with the given objects.
//
// Prefer this over NewSimpleClientset (deprecated by client-gen). This constructor does not call
// NewSimpleClientset so callers avoid nesting deprecated APIs.
//
// Implementation matches generated NewSimpleClientset (testing.NewObjectTracker). Upstream
// Kubernetes fake NewClientset uses testing.NewFieldManagedObjectTracker when apply configurations
// exist; regenerate this clientset with apply configs (client-gen --with-applyconfig) to match that.
func NewClientset(objects ...runtime.Object) *Clientset {
o := testing.NewObjectTracker(scheme, codecs.UniversalDecoder())
for _, obj := range objects {
if err := o.Add(obj); err != nil {
panic(err)
}
}

cs := &Clientset{tracker: o}
cs.discovery = &fakediscovery.FakeDiscovery{Fake: &cs.Fake}
cs.AddReactor("*", "*", testing.ObjectReaction(o))
cs.AddWatchReactor("*", func(action testing.Action) (handled bool, ret watch.Interface, err error) {
var opts metav1.ListOptions
if watchAction, ok := action.(testing.WatchActionImpl); ok {
opts = watchAction.ListOptions
}
gvr := action.GetResource()
ns := action.GetNamespace()
watch, err := o.Watch(gvr, ns, opts)
if err != nil {
return false, nil, err
}
return true, watch, nil
})

return cs
}
78 changes: 58 additions & 20 deletions pkg/controller/accesspolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"reflect"
"strings"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
Expand All @@ -40,26 +41,6 @@ import (
"sigs.k8s.io/kube-agentic-networking/pkg/constants"
)

// AccessPolicyTargetRefIndex is the index name for looking up AccessPolicies by target ref (namespace/name of XBackend).
const AccessPolicyTargetRefIndex = "targetRef"

// accessPolicyTargetRefIndexFunc indexes AccessPolicies by each XBackend targetRef (namespace/name).
// Used by the informer cache to support O(1) lookup of policies targeting a given backend.
func accessPolicyTargetRefIndexFunc(obj interface{}) ([]string, error) {
policy, ok := obj.(*agenticv0alpha0.XAccessPolicy)
if !ok {
return nil, nil
}
var keys []string
for _, targetRef := range policy.Spec.TargetRefs {
if !isXBackendTargetRef(targetRef) {
continue
}
keys = append(keys, policy.Namespace+"/"+string(targetRef.Name))
}
return keys, nil
}

func (c *Controller) setupAccessPolicyEventHandlers(accessPolicyInformer agenticinformers.XAccessPolicyInformer) error {
_, err := accessPolicyInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: c.onAccessPolicyAdd,
Expand Down Expand Up @@ -331,3 +312,60 @@ func (c *Controller) updateAccessPolicyStatus(ctx context.Context, policy *agent
return err
})
}

// filterEmptyStrings returns a copy of ss without "" elements.
func filterEmptyStrings(ss []string) []string {
var out []string
for _, s := range ss {
if s != "" {
out = append(out, s)
}
}
return out
}

func accessPolicyHasInvalidTranslationStatus(policy *agenticv0alpha0.XAccessPolicy) bool {
for _, anc := range policy.Status.Ancestors {
cond := meta.FindStatusCondition(anc.Conditions, string(agenticv0alpha0.PolicyConditionAccepted))
if cond != nil && cond.Status == metav1.ConditionFalse && cond.Reason == string(gwapiv1.PolicyReasonInvalid) {
return true
}
}
return false
}

// reconcileAccessPolicyTranslationStatus updates XAccessPolicy status when the translator skips or
// partially applies rules (Gateway API PolicyReasonInvalid), and clears that reason by re-running
// the per-target limit check when translation succeeds again.
//
// ListAccessPoliciesAttachedToGateway defines which policies are in scope for this Gateway sync.
// The translator's issues map only contains policies that reported errors this pass; it does not
// include policies that now translate cleanly but still carry Accepted=False/Invalid from a prior
// reconciliation. Listing attachment lets us update both: set Invalid when issues[nn] is non-empty,
// and clear stale Invalid (via isPolicyUnderTargetLimit) when issues omits the policy.
func (c *Controller) reconcileAccessPolicyTranslationStatus(ctx context.Context, gateway *gwapiv1.Gateway, issues map[types.NamespacedName][]string) {
attached, err := c.translator.ListAccessPoliciesAttachedToGateway(gateway)
Comment thread
HarshithaMS005 marked this conversation as resolved.
if err != nil {
runtime.HandleError(fmt.Errorf("list AccessPolicies attached to gateway: %w", err))
return
}
for _, ap := range attached {
policy, err := c.agentic.accessPolicyLister.XAccessPolicies(ap.Namespace).Get(ap.Name)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the goal of getting the policy object again? Is it to ensure the latest spec.targetRefs after getting state of the object from listing at line 347.

This seems almost the same as #236 (comment). I wonder if we this step is really needed.

If we can confirm this builds on the informer's cache and won't issue a request to the API server for each policy object unless the generation of the resource has changed, then perhaps there's no harm in keeping it. Otherwise, I'd probably advocate for avoiding the overhead on the API server, given another reconciliation event may have been enqueued already by the time this happens.

if err != nil {
continue
}
nn := types.NamespacedName{Namespace: policy.Namespace, Name: policy.Name}
if msgs, hasErr := issues[nn]; hasErr {
msg := strings.Join(deduplicateStrings(filterEmptyStrings(msgs)), "; ")
for _, tr := range policy.Spec.TargetRefs {
if err := c.updateAccessPolicyStatus(ctx, policy, tr, false, gwapiv1.PolicyReasonInvalid, msg); err != nil {
runtime.HandleError(fmt.Errorf("update AccessPolicy %s status for translation failure: %w", nn, err))
}
}
continue
}
if accessPolicyHasInvalidTranslationStatus(policy) {
_ = c.isPolicyUnderTargetLimit(ctx, policy)
}
}
}
Loading
Loading