From dbb6f9e0e2cdfcf7523d9da35376a488da761cbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Tue, 20 May 2025 22:43:59 +0800 Subject: [PATCH 01/11] feat: Add support for Gateway API ReferenceGrant resource This commit introduces functionality to handle the ReferenceGrant resource in the Gateway API. It updates the Gateway controller logic, adds necessary permissions in RBAC manifests, and integrates condition handling for cross-namespace references. Additionally, skipped conformance tests related to ReferenceGrants are reinstated. --- charts/templates/cluster_role.yaml | 14 ++ internal/controller/gateway_controller.go | 93 +++++++++++- internal/controller/utils.go | 176 +++++++++++++--------- internal/manager/run.go | 14 ++ test/conformance/conformance_test.go | 3 - test/e2e/framework/manifests/ingress.yaml | 14 ++ 6 files changed, 231 insertions(+), 83 deletions(-) diff --git a/charts/templates/cluster_role.yaml b/charts/templates/cluster_role.yaml index 7effb5c2..ebe77e25 100644 --- a/charts/templates/cluster_role.yaml +++ b/charts/templates/cluster_role.yaml @@ -170,6 +170,20 @@ rules: - get - list - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - referencegrants + verbs: + - get + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - referencegrants/status + verbs: + - get - apiGroups: - networking.k8s.io resources: diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 77e9f10b..48b2213d 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -27,10 +27,12 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/internal/controller/indexer" @@ -83,6 +85,23 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error { &corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.listGatewaysForSecret), ). + Watches(&v1beta1.ReferenceGrant{}, + handler.EnqueueRequestsFromMapFunc(r.listReferenceGrantsForGateway), + builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return referenceGrantHasGatewayFrom(e.Object) + }, + UpdateFunc: func(e event.UpdateEvent) bool { + return referenceGrantHasGatewayFrom(e.ObjectOld) || referenceGrantHasGatewayFrom(e.ObjectNew) + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return referenceGrantHasGatewayFrom(e.Object) + }, + GenericFunc: func(e event.GenericEvent) bool { + return referenceGrantHasGatewayFrom(e.Object) + }, + }), + ). Complete(r) } @@ -117,7 +136,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct msg: acceptedMessage("gateway"), } - // create a translate context + // create a translation context tctx := provider.NewDefaultTranslateContext(ctx) r.processListenerConfig(tctx, gateway) @@ -163,21 +182,29 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct msg: err.Error(), } } + setAccepted := SetGatewayConditionAccepted(gateway, acceptStatus.status, acceptStatus.msg) + setProgrammed := SetGatewayConditionProgrammed(gateway, conditionProgrammedStatus, conditionProgrammedMsg) + if !acceptStatus.status { + return ctrl.Result{}, r.Status().Update(ctx, gateway) + } - ListenerStatuses, err := getListenerStatus(ctx, r.Client, gateway) + var referenceGrantList v1beta1.ReferenceGrantList + if err := r.List(ctx, &referenceGrantList); err != nil { + r.Log.Error(err, "failed to list reference grants") + return ctrl.Result{}, err + } + listenerStatuses, err := getListenerStatus(ctx, r.Client, gateway, referenceGrantList.Items) if err != nil { - log.Error(err, "failed to get listener status", "gateway", gateway.GetName()) + r.Log.Error(err, "failed to get listener status", "gateway", types.NamespacedName{Namespace: gateway.GetNamespace(), Name: gateway.GetName()}) return ctrl.Result{}, err } - accepted := SetGatewayConditionAccepted(gateway, acceptStatus.status, acceptStatus.msg) - Programmed := SetGatewayConditionProgrammed(gateway, conditionProgrammedStatus, conditionProgrammedMsg) - if accepted || Programmed || len(addrs) > 0 || len(ListenerStatuses) > 0 { + if setAccepted || setProgrammed || len(addrs) > 0 || len(listenerStatuses) > 0 { if len(addrs) > 0 { gateway.Status.Addresses = addrs } - if len(ListenerStatuses) > 0 { - gateway.Status.Listeners = ListenerStatuses + if len(listenerStatuses) > 0 { + gateway.Status.Listeners = listenerStatuses } return ctrl.Result{}, r.Status().Update(ctx, gateway) @@ -348,6 +375,56 @@ func (r *GatewayReconciler) listGatewaysForSecret(ctx context.Context, obj clien return requests } +func (r *GatewayReconciler) listReferenceGrantsForGateway(ctx context.Context, obj client.Object) (requests []reconcile.Request) { + grant, ok := obj.(*v1beta1.ReferenceGrant) + if !ok { + r.Log.Error( + errors.New("unexpected object type"), + "ReferenceGrant watch predicate received unexpected object type", + "expected", FullTypeName(new(v1beta1.ReferenceGrant)), "found", FullTypeName(obj), + ) + return nil + } + + var gatewayList gatewayv1.GatewayList + if err := r.List(ctx, &gatewayList); err != nil { + r.Log.Error(err, "failed to list gateways in watch predicate", "ReferenceGrant", grant.GetName()) + return nil + } + + for _, gateway := range gatewayList.Items { + for _, from := range grant.Spec.From { + gw := v1beta1.ReferenceGrantFrom{ + Group: gatewayv1.GroupName, + Kind: KindGateway, + Namespace: v1beta1.Namespace(gateway.GetNamespace()), + } + if from == gw { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: gateway.GetNamespace(), + Name: gateway.GetName(), + }, + }) + } + } + } + return requests +} + +func referenceGrantHasGatewayFrom(obj client.Object) bool { + grant, ok := obj.(*v1beta1.ReferenceGrant) + if !ok { + return false + } + for _, from := range grant.Spec.From { + if from.Kind == KindGateway && string(from.Group) == gatewayv1.GroupName { + return true + } + } + return false +} + func (r *GatewayReconciler) processInfrastructure(tctx *provider.TranslateContext, gateway *gatewayv1.Gateway) error { rk := provider.ResourceKind{ Kind: gateway.Kind, diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 5dd2e7f5..dd356dd6 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -33,6 +33,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/internal/controller/config" @@ -121,14 +122,9 @@ func IsConditionPresentAndEqual(conditions []metav1.Condition, condition metav1. } func SetGatewayConditionAccepted(gw *gatewayv1.Gateway, status bool, message string) (ok bool) { - conditionStatus := metav1.ConditionTrue - if !status { - conditionStatus = metav1.ConditionFalse - } - condition := metav1.Condition{ Type: string(gatewayv1.GatewayConditionAccepted), - Status: conditionStatus, + Status: ConditionStatus(status), Reason: string(gatewayv1.GatewayReasonAccepted), ObservedGeneration: gw.GetGeneration(), Message: message, @@ -661,6 +657,7 @@ func getListenerStatus( ctx context.Context, mrgc client.Client, gateway *gatewayv1.Gateway, + grants []v1beta1.ReferenceGrant, ) ([]gatewayv1.ListenerStatus, error) { statuses := make(map[gatewayv1.SectionName]gatewayv1.ListenerStatus, len(gateway.Spec.Listeners)) @@ -670,12 +667,35 @@ func getListenerStatus( return nil, err } var ( - reasonResolvedRef = string(gatewayv1.ListenerReasonResolvedRefs) - statusResolvedRef = metav1.ConditionTrue - messageResolvedRef string - - reasonProgrammed = string(gatewayv1.ListenerReasonProgrammed) - statusProgrammed = metav1.ConditionTrue + now = metav1.Now() + conditionProgrammed = metav1.Condition{ + Type: string(gatewayv1.ListenerConditionProgrammed), + Status: metav1.ConditionTrue, + ObservedGeneration: gateway.GetGeneration(), + LastTransitionTime: now, + Reason: string(gatewayv1.ListenerReasonProgrammed), + } + conditionAccepted = metav1.Condition{ + Type: string(gatewayv1.ListenerConditionAccepted), + Status: metav1.ConditionTrue, + ObservedGeneration: gateway.GetGeneration(), + LastTransitionTime: now, + Reason: string(gatewayv1.ListenerReasonAccepted), + } + conditionConflicted = metav1.Condition{ + Type: string(gatewayv1.ListenerConditionConflicted), + Status: metav1.ConditionTrue, + ObservedGeneration: gateway.GetGeneration(), + LastTransitionTime: now, + Reason: string(gatewayv1.ListenerReasonNoConflicts), + } + conditionResolvedRefs = metav1.Condition{ + Type: string(gatewayv1.ListenerConditionResolvedRefs), + Status: metav1.ConditionTrue, + ObservedGeneration: gateway.GetGeneration(), + LastTransitionTime: now, + Reason: string(gatewayv1.ListenerReasonResolvedRefs), + } supportedKinds = []gatewayv1.RouteGroupKind{} ) @@ -683,42 +703,54 @@ func getListenerStatus( if listener.AllowedRoutes == nil || listener.AllowedRoutes.Kinds == nil { supportedKinds = []gatewayv1.RouteGroupKind{ { - Kind: gatewayv1.Kind("HTTPRoute"), + Kind: KindHTTPRoute, }, } } else { for _, kind := range listener.AllowedRoutes.Kinds { if kind.Group != nil && *kind.Group != gatewayv1.GroupName { - reasonResolvedRef = string(gatewayv1.ListenerReasonInvalidRouteKinds) - statusResolvedRef = metav1.ConditionFalse + conditionResolvedRefs.Status = metav1.ConditionFalse + conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonInvalidRouteKinds) continue } switch kind.Kind { - case gatewayv1.Kind("HTTPRoute"): + case KindHTTPRoute: supportedKinds = append(supportedKinds, kind) default: - reasonResolvedRef = string(gatewayv1.ListenerReasonInvalidRouteKinds) - statusResolvedRef = metav1.ConditionFalse + conditionResolvedRefs.Status = metav1.ConditionFalse + conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonInvalidRouteKinds) } - } } if listener.TLS != nil { // TODO: support TLS var ( - secret corev1.Secret - resolved = true + secret corev1.Secret ) for _, ref := range listener.TLS.CertificateRefs { if ref.Group != nil && *ref.Group != corev1.GroupName { - resolved = false - messageResolvedRef = fmt.Sprintf(`Invalid Group, expect "", got "%s"`, *ref.Group) + conditionResolvedRefs.Status = metav1.ConditionFalse + conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonInvalidCertificateRef) + conditionResolvedRefs.Message = fmt.Sprintf(`Invalid Group, expect "", got "%s"`, *ref.Group) + conditionProgrammed.Status = metav1.ConditionFalse + conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) + break + } + if ref.Kind != nil && *ref.Kind != KindSecret { + conditionResolvedRefs.Status = metav1.ConditionFalse + conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonInvalidCertificateRef) + conditionResolvedRefs.Message = fmt.Sprintf(`Invalid Kind, expect "Secret", got "%s"`, *ref.Kind) + conditionProgrammed.Status = metav1.ConditionFalse + conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) break } - if ref.Kind != nil && *ref.Kind != "Secret" { - resolved = false - messageResolvedRef = fmt.Sprintf(`Invalid Kind, expect "Secret", got "%s"`, *ref.Kind) + if ok := checkReferenceGrantBetweenGatewayAndSecret(gateway.Namespace, ref, grants); !ok { + conditionResolvedRefs.Status = metav1.ConditionFalse + conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonRefNotPermitted) + conditionResolvedRefs.Message = "certificateRefs cross namespaces is not permitted" + conditionProgrammed.Status = metav1.ConditionFalse + conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) break } ns := gateway.Namespace @@ -726,59 +758,32 @@ func getListenerStatus( ns = string(*ref.Namespace) } if err := mrgc.Get(ctx, client.ObjectKey{Namespace: ns, Name: string(ref.Name)}, &secret); err != nil { - resolved = false - messageResolvedRef = err.Error() + conditionResolvedRefs.Status = metav1.ConditionFalse + conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonInvalidCertificateRef) + conditionResolvedRefs.Message = err.Error() + conditionProgrammed.Status = metav1.ConditionFalse + conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) break } - if reason, ok := isTLSSecretValid(&secret); !ok { - resolved = false - messageResolvedRef = fmt.Sprintf("Malformed Secret referenced: %s", reason) + if cause, ok := isTLSSecretValid(&secret); !ok { + conditionResolvedRefs.Status = metav1.ConditionFalse + conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonInvalidCertificateRef) + conditionResolvedRefs.Message = fmt.Sprintf("Malformed Secret referenced: %s", cause) + conditionProgrammed.Status = metav1.ConditionFalse + conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) break } } - if !resolved { - reasonResolvedRef = string(gatewayv1.ListenerReasonInvalidCertificateRef) - statusResolvedRef = metav1.ConditionFalse - reasonProgrammed = string(gatewayv1.ListenerReasonInvalid) - statusProgrammed = metav1.ConditionFalse - } - } - - conditions := []metav1.Condition{ - { - Type: string(gatewayv1.ListenerConditionProgrammed), - Status: statusProgrammed, - ObservedGeneration: gateway.Generation, - LastTransitionTime: metav1.Now(), - Reason: reasonProgrammed, - }, - { - Type: string(gatewayv1.ListenerConditionAccepted), - Status: metav1.ConditionTrue, - ObservedGeneration: gateway.Generation, - LastTransitionTime: metav1.Now(), - Reason: string(gatewayv1.ListenerReasonAccepted), - }, - { - Type: string(gatewayv1.ListenerConditionConflicted), - Status: metav1.ConditionTrue, - ObservedGeneration: gateway.Generation, - LastTransitionTime: metav1.Now(), - Reason: string(gatewayv1.ListenerReasonNoConflicts), - }, - { - Type: string(gatewayv1.ListenerConditionResolvedRefs), - Status: statusResolvedRef, - ObservedGeneration: gateway.Generation, - LastTransitionTime: metav1.Now(), - Reason: reasonResolvedRef, - Message: messageResolvedRef, - }, } status := gatewayv1.ListenerStatus{ - Name: listener.Name, - Conditions: conditions, + Name: listener.Name, + Conditions: []metav1.Condition{ + conditionProgrammed, + conditionAccepted, + conditionConflicted, + conditionResolvedRefs, + }, SupportedKinds: supportedKinds, AttachedRoutes: attachedRoutes, } @@ -788,7 +793,7 @@ func getListenerStatus( if gateway.Status.Listeners[i].AttachedRoutes != attachedRoutes { changed = true } - for _, condition := range conditions { + for _, condition := range status.Conditions { if !IsConditionPresentAndEqual(gateway.Status.Listeners[i].Conditions, condition) { changed = true break @@ -1089,3 +1094,30 @@ func isTLSSecretValid(secret *corev1.Secret) (string, bool) { } return "", true } + +func checkReferenceGrantBetweenGatewayAndSecret(gwNamespace string, certRef gatewayv1.SecretObjectReference, grants []v1beta1.ReferenceGrant) bool { + // if not cross namespaces + if certRef.Namespace == nil || string(*certRef.Namespace) == gwNamespace { + return true + } + + for _, grant := range grants { + if grant.Namespace == string(*certRef.Namespace) { + for _, from := range grant.Spec.From { + gw := v1beta1.ReferenceGrantFrom{ + Group: gatewayv1.GroupName, + Kind: KindGateway, + Namespace: v1beta1.Namespace(gwNamespace), + } + if from == gw { + for _, to := range grant.Spec.To { + if to.Group == corev1.GroupName && to.Kind == KindSecret && (to.Name == nil || *to.Name == certRef.Name) { + return true + } + } + } + } + } + } + return false +} diff --git a/internal/manager/run.go b/internal/manager/run.go index 769cd4f8..c8156ba9 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -19,7 +19,9 @@ import ( "time" "github.com/go-logr/logr" + "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/ptr" @@ -29,6 +31,7 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/internal/controller/config" @@ -48,6 +51,9 @@ func init() { if err := v1alpha1.AddToScheme(scheme); err != nil { panic(err) } + if err := v1beta1.Install(scheme); err != nil { + panic(err) + } // +kubebuilder:scaffold:scheme } @@ -177,6 +183,14 @@ func Run(ctx context.Context, logger logr.Logger) error { setupLog.Error(err, "unable to set up controllers") return err } + if _, err = mgr.GetRESTMapper().KindsFor(schema.GroupVersionResource{ + Group: v1beta1.GroupVersion.Group, + Version: v1beta1.GroupVersion.Version, + Resource: "referencegrants", + }); err != nil { + logger.Error(err, "CRD ReferenceGrants is not installed") + return errors.Wrap(err, "CRD ReferenceGrants is not installed") + } for _, c := range controllers { if err := c.SetupWithManager(mgr); err != nil { return err diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index bd6362fb..c664f1fe 100644 --- a/test/conformance/conformance_test.go +++ b/test/conformance/conformance_test.go @@ -25,9 +25,6 @@ var skippedTestsForSSL = []string{ } var skippedTestsForTraditionalRoutes = []string{ - // TODO: Support ReferenceGrant resource - tests.GatewaySecretInvalidReferenceGrant.ShortName, - tests.GatewaySecretMissingReferenceGrant.ShortName, tests.HTTPRouteInvalidCrossNamespaceBackendRef.ShortName, tests.HTTPRouteInvalidReferenceGrant.ShortName, tests.HTTPRoutePartiallyInvalidViaInvalidReferenceGrant.ShortName, diff --git a/test/e2e/framework/manifests/ingress.yaml b/test/e2e/framework/manifests/ingress.yaml index fc63585f..4aeabb89 100644 --- a/test/e2e/framework/manifests/ingress.yaml +++ b/test/e2e/framework/manifests/ingress.yaml @@ -229,6 +229,20 @@ rules: verbs: - get - update +- apiGroups: + - gateway.networking.k8s.io + resources: + - referencegrants + verbs: + - get + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - referencegrants/status + verbs: + - get - apiGroups: - networking.k8s.io resources: From 87bb31e4d04d8d89351db3411e4c2b33e475e31b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 00:19:56 +0800 Subject: [PATCH 02/11] refactor: Improve condition handling in gateway controller --- internal/controller/gateway_controller.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 48b2213d..332089ae 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -18,7 +18,6 @@ import ( "fmt" "reflect" - "github.com/api7/gopkg/pkg/log" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -34,6 +33,8 @@ import ( gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" + "github.com/api7/gopkg/pkg/log" + "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/internal/controller/indexer" "github.com/apache/apisix-ingress-controller/internal/provider" @@ -182,11 +183,6 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct msg: err.Error(), } } - setAccepted := SetGatewayConditionAccepted(gateway, acceptStatus.status, acceptStatus.msg) - setProgrammed := SetGatewayConditionProgrammed(gateway, conditionProgrammedStatus, conditionProgrammedMsg) - if !acceptStatus.status { - return ctrl.Result{}, r.Status().Update(ctx, gateway) - } var referenceGrantList v1beta1.ReferenceGrantList if err := r.List(ctx, &referenceGrantList); err != nil { @@ -199,7 +195,9 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, err } - if setAccepted || setProgrammed || len(addrs) > 0 || len(listenerStatuses) > 0 { + accepted := SetGatewayConditionAccepted(gateway, acceptStatus.status, acceptStatus.msg) + programmed := SetGatewayConditionProgrammed(gateway, conditionProgrammedStatus, conditionProgrammedMsg) + if accepted || programmed || len(addrs) > 0 || len(listenerStatuses) > 0 { if len(addrs) > 0 { gateway.Status.Addresses = addrs } From 0725fb5e1575340aeb070457e5e94325717ce32b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 03:03:01 +0800 Subject: [PATCH 03/11] feat: Add ReferenceGrant support and refactor backend reference handling This commit introduces support for Gateway API ReferenceGrant CRD, enabling cross-namespace references for HTTPRoutes. It refactors backend reference handling to validate Service references and check ReferenceGrants. Also includes minor code cleanups, added cluster role permissions for ReferenceGrants, and adjustments to e2e manifests. --- api/v1alpha1/backendtrafficpolicy_types.go | 4 +- api/v1alpha1/consumer_types.go | 18 +-- api/v1alpha1/gatewayproxy_types.go | 20 +-- api/v1alpha1/httproutepolicy_types.go | 4 +- charts/templates/cluster_role.yaml | 14 ++ internal/controller/httproute_controller.go | 169 +++++++++++++++----- internal/controller/utils.go | 96 ++++------- internal/manager/run.go | 14 ++ test/conformance/conformance_test.go | 4 - test/e2e/framework/manifests/ingress.yaml | 14 ++ 10 files changed, 227 insertions(+), 130 deletions(-) diff --git a/api/v1alpha1/backendtrafficpolicy_types.go b/api/v1alpha1/backendtrafficpolicy_types.go index b7ebc5fa..90bc8860 100644 --- a/api/v1alpha1/backendtrafficpolicy_types.go +++ b/api/v1alpha1/backendtrafficpolicy_types.go @@ -22,8 +22,8 @@ type BackendTrafficPolicy struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // BackendTrafficPolicySpec defines traffic handling policies applied to backend services, - // such as load balancing strategy, connection settings, and failover behavior. + // BackendTrafficPolicySpec defines traffic handling policies applied to backend services, + // such as load balancing strategy, connection settings, and failover behavior. Spec BackendTrafficPolicySpec `json:"spec,omitempty"` Status PolicyStatus `json:"status,omitempty"` } diff --git a/api/v1alpha1/consumer_types.go b/api/v1alpha1/consumer_types.go index 108c8f99..7e75f359 100644 --- a/api/v1alpha1/consumer_types.go +++ b/api/v1alpha1/consumer_types.go @@ -23,7 +23,7 @@ type Consumer struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // ConsumerSpec defines the configuration for a consumer, including consumer name, + // ConsumerSpec defines the configuration for a consumer, including consumer name, // authentication credentials, and plugin settings. Spec ConsumerSpec `json:"spec,omitempty"` Status Status `json:"status,omitempty"` @@ -31,11 +31,11 @@ type Consumer struct { type ConsumerSpec struct { // GatewayRef specifies the gateway details. - GatewayRef GatewayRef `json:"gatewayRef,omitempty"` + GatewayRef GatewayRef `json:"gatewayRef,omitempty"` // Credentials specifies the credential details of a consumer. Credentials []Credential `json:"credentials,omitempty"` // Plugins define the plugins associated with a consumer. - Plugins []Plugin `json:"plugins,omitempty"` + Plugins []Plugin `json:"plugins,omitempty"` } type GatewayRef struct { @@ -48,7 +48,7 @@ type GatewayRef struct { Kind *string `json:"kind,omitempty"` // Group is the API group the resource belongs to. Default is `gateway.networking.k8s.io`. // +kubebuilder:default=gateway.networking.k8s.io - Group *string `json:"group,omitempty"` + Group *string `json:"group,omitempty"` // Namespace is namespace of the resource. Namespace *string `json:"namespace,omitempty"` } @@ -58,18 +58,18 @@ type Credential struct { // +kubebuilder:validation:Enum=jwt-auth;basic-auth;key-auth;hmac-auth; // Type specifies the type of authentication to configure credentials for. // Can be one of `jwt-auth`, `basic-auth`, `key-auth`, or `hmac-auth`. - Type string `json:"type"` + Type string `json:"type"` // Config specifies the credential details for authentication. - Config apiextensionsv1.JSON `json:"config,omitempty"` + Config apiextensionsv1.JSON `json:"config,omitempty"` // SecretRef references to the Secret that contains the credentials. - SecretRef *SecretReference `json:"secretRef,omitempty"` + SecretRef *SecretReference `json:"secretRef,omitempty"` // Name is the name of the credential. - Name string `json:"name,omitempty"` + Name string `json:"name,omitempty"` } type SecretReference struct { // Name is the name of the secret. - Name string `json:"name"` + Name string `json:"name"` // Namespace is the namespace of the secret. Namespace *string `json:"namespace,omitempty"` } diff --git a/api/v1alpha1/gatewayproxy_types.go b/api/v1alpha1/gatewayproxy_types.go index cafd596f..871d43bd 100644 --- a/api/v1alpha1/gatewayproxy_types.go +++ b/api/v1alpha1/gatewayproxy_types.go @@ -27,14 +27,14 @@ type GatewayProxySpec struct { // PublishService specifies the LoadBalancer-type Service whose external address the controller uses to // update the status of Ingress resources. - PublishService string `json:"publishService,omitempty"` + PublishService string `json:"publishService,omitempty"` // StatusAddress specifies the external IP addresses that the controller uses to populate the status field // of GatewayProxy or Ingress resources for developers to access. - StatusAddress []string `json:"statusAddress,omitempty"` + StatusAddress []string `json:"statusAddress,omitempty"` // Provider configures the provider details. - Provider *GatewayProxyProvider `json:"provider,omitempty"` + Provider *GatewayProxyProvider `json:"provider,omitempty"` // Plugins configure global plugins. - Plugins []GatewayProxyPlugin `json:"plugins,omitempty"` + Plugins []GatewayProxyPlugin `json:"plugins,omitempty"` // PluginMetadata configures common configurations shared by all plugin instances of the same name. PluginMetadata map[string]apiextensionsv1.JSON `json:"pluginMetadata,omitempty"` } @@ -132,8 +132,8 @@ type GatewayProxy struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // GatewayProxySpec defines the desired state and configuration of a GatewayProxy, - // including networking settings, global plugins, and plugin metadata. + // GatewayProxySpec defines the desired state and configuration of a GatewayProxy, + // including networking settings, global plugins, and plugin metadata. Spec GatewayProxySpec `json:"spec,omitempty"` } @@ -148,11 +148,11 @@ type GatewayProxyList struct { // GatewayProxyPlugin contains plugin configurations. type GatewayProxyPlugin struct { // Name is the name of the plugin. - Name string `json:"name,omitempty"` - // Enabled defines whether the plugin is enabled. - Enabled bool `json:"enabled,omitempty"` + Name string `json:"name,omitempty"` + // Enabled defines whether the plugin is enabled. + Enabled bool `json:"enabled,omitempty"` // Config defines the plugin's configuration details. - Config apiextensionsv1.JSON `json:"config,omitempty"` + Config apiextensionsv1.JSON `json:"config,omitempty"` } func init() { diff --git a/api/v1alpha1/httproutepolicy_types.go b/api/v1alpha1/httproutepolicy_types.go index 5cce9a36..82c37bdd 100644 --- a/api/v1alpha1/httproutepolicy_types.go +++ b/api/v1alpha1/httproutepolicy_types.go @@ -25,9 +25,9 @@ type HTTPRoutePolicySpec struct { // +kubebuilder:validation:MaxItems=16 TargetRefs []gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName `json:"targetRefs"` // Priority sets the priority for route. A higher value sets a higher priority in route matching. - Priority *int64 `json:"priority,omitempty" yaml:"priority,omitempty"` + Priority *int64 `json:"priority,omitempty" yaml:"priority,omitempty"` // Vars sets the request matching conditions. - Vars []apiextensionsv1.JSON `json:"vars,omitempty" yaml:"vars,omitempty"` + Vars []apiextensionsv1.JSON `json:"vars,omitempty" yaml:"vars,omitempty"` } // +kubebuilder:object:root=true diff --git a/charts/templates/cluster_role.yaml b/charts/templates/cluster_role.yaml index 7effb5c2..ebe77e25 100644 --- a/charts/templates/cluster_role.yaml +++ b/charts/templates/cluster_role.yaml @@ -170,6 +170,20 @@ rules: - get - list - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - referencegrants + verbs: + - get + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - referencegrants/status + verbs: + - get - apiGroups: - networking.k8s.io resources: diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 9bfdd5dd..757df05a 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -13,9 +13,9 @@ package controller import ( + "cmp" "context" "fmt" - "strings" "github.com/api7/gopkg/pkg/log" "github.com/go-logr/logr" @@ -38,6 +38,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" + "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/internal/controller/indexer" @@ -101,6 +102,10 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches(&v1alpha1.GatewayProxy{}, handler.EnqueueRequestsFromMapFunc(r.listHTTPRoutesForGatewayProxy), ). + Watches(&v1beta1.ReferenceGrant{}, + handler.EnqueueRequestsFromMapFunc(r.lisHTTPRoutesForReferenceGrant), + builder.WithPredicates(httpRouteReferenceGrantPredicates()), + ). WatchesRawSource( source.Channel( r.genericEvent, @@ -173,7 +178,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.processHTTPRoute(tctx, hr); err != nil { httpRouteErr = err // When encountering a backend reference error, it should not affect the acceptance status - if !IsInvalidKindError(err) { + if !IsSomeReasonError(err, gatewayv1.RouteReasonInvalidKind) { acceptStatus.status = false acceptStatus.msg = err.Error() } @@ -186,12 +191,12 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // Store the backend reference error for later use var backendRefErr error - if err := r.processHTTPRouteBackendRefs(tctx); err != nil { + if err := r.processHTTPRouteBackendRefs(tctx, req.NamespacedName); err != nil { backendRefErr = err } // If the backend reference error is because of an invalid kind, use this error first - if httpRouteErr != nil && IsInvalidKindError(httpRouteErr) { + if IsSomeReasonError(err, gatewayv1.RouteReasonInvalidKind) { backendRefErr = httpRouteErr } ProcessBackendTrafficPolicy(r.Client, r.Log, tctx) @@ -419,14 +424,22 @@ func (r *HTTPRouteReconciler) listHTTPRouteForGenericEvent(ctx context.Context, } } -func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.TranslateContext) error { +func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.TranslateContext, hrNN types.NamespacedName) error { var terr error for _, backend := range tctx.BackendRefs { - namespace := string(*backend.Namespace) - name := string(backend.Name) + targetNN := types.NamespacedName{ + Namespace: hrNN.Namespace, + Name: string(backend.Name), + } + if backend.Namespace != nil { + targetNN.Namespace = string(*backend.Namespace) + } if backend.Kind != nil && *backend.Kind != "Service" { - terr = NewInvalidKindError(string(*backend.Kind)) + terr = &ReasonError{ + Reason: string(gatewayv1.RouteReasonInvalidKind), + Message: fmt.Sprintf("invalid kind %s, only Service is supported", *backend.Kind), + } continue } @@ -435,22 +448,36 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla continue } - serviceNS := types.NamespacedName{ - Namespace: namespace, - Name: name, - } - var service corev1.Service - if err := r.Get(tctx, serviceNS, &service); err != nil { + if err := r.Get(tctx, targetNN, &service); err != nil { + terr = err if client.IgnoreNotFound(err) == nil { - terr = NewBackendNotFoundError(namespace, name) - } else { - terr = err + terr = &ReasonError{ + Reason: string(gatewayv1.RouteReasonBackendNotFound), + Message: fmt.Sprintf("Service %s not found", targetNN), + } } continue } + + // if cross namespaces between HTTPRoute and referenced Service, check ReferenceGrant + if hrNN.Namespace != targetNN.Namespace { + var referenceGrantList v1beta1.ReferenceGrantList + if err := r.List(tctx, &referenceGrantList, client.InNamespace(targetNN.Namespace)); err != nil { + r.Log.Error(err, "failed to list ReferenceGrants", "namespace", targetNN.Namespace) + return err + } + if !checkReferenceGrantBetweenHTTPRouteAndService(hrNN, targetNN, referenceGrantList.Items) { + terr = &ReasonError{ + Reason: string(v1beta1.RouteReasonRefNotPermitted), + Message: fmt.Sprintf("%s is in a different namespace than the HTTPRoute %s and no ReferenceGrant allowing reference is configured", targetNN, hrNN), + } + continue + } + } + if service.Spec.Type == corev1.ServiceTypeExternalName { - tctx.Services[serviceNS] = &service + tctx.Services[targetNN] = &service return nil } @@ -462,24 +489,24 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla } } if !portExists { - terr = fmt.Errorf("port %d not found in service %s", *backend.Port, name) + terr = fmt.Errorf("port %d not found in service %s", *backend.Port, targetNN.Name) continue } - tctx.Services[serviceNS] = &service + tctx.Services[targetNN] = &service endpointSliceList := new(discoveryv1.EndpointSliceList) if err := r.List(tctx, endpointSliceList, - client.InNamespace(namespace), + client.InNamespace(targetNN.Namespace), client.MatchingLabels{ - discoveryv1.LabelServiceName: name, + discoveryv1.LabelServiceName: targetNN.Name, }, ); err != nil { - r.Log.Error(err, "failed to list endpoint slices", "namespace", namespace, "name", name) + r.Log.Error(err, "failed to list endpoint slices", "Service", targetNN) terr = err continue } - tctx.EndpointSlices[serviceNS] = endpointSliceList.Items + tctx.EndpointSlices[targetNN] = endpointSliceList.Items } return terr } @@ -507,29 +534,14 @@ func (r *HTTPRouteReconciler) processHTTPRoute(tctx *provider.TranslateContext, } } for _, backend := range rule.BackendRefs { - var kind string - if backend.Kind == nil { - kind = "service" - } else { - kind = strings.ToLower(string(*backend.Kind)) - } - if kind != "service" { - terror = NewInvalidKindError(kind) + if backend.Kind != nil && *backend.Kind != "Service" { + terror = newInvalidKindError(*backend.Kind) continue } - - var ns string - if backend.Namespace == nil { - ns = httpRoute.Namespace - } else { - ns = string(*backend.Namespace) - } - - backendNs := gatewayv1.Namespace(ns) tctx.BackendRefs = append(tctx.BackendRefs, gatewayv1.BackendRef{ BackendObjectReference: gatewayv1.BackendObjectReference{ Name: backend.Name, - Namespace: &backendNs, + Namespace: cmp.Or(backend.Namespace, (*gatewayv1.Namespace)(&httpRoute.Namespace)), Port: backend.Port, }, }) @@ -615,3 +627,76 @@ func (r *HTTPRouteReconciler) listHTTPRoutesForGatewayProxy(ctx context.Context, return requests } + +func (r *HTTPRouteReconciler) lisHTTPRoutesForReferenceGrant(ctx context.Context, obj client.Object) (requests []reconcile.Request) { + grant, ok := obj.(*v1beta1.ReferenceGrant) + if !ok { + r.Log.Error(fmt.Errorf("unexpected object type"), "failed to convert object to ReferenceGrant") + return nil + } + + var httpRouteList gatewayv1.HTTPRouteList + if err := r.List(ctx, &httpRouteList); err != nil { + r.Log.Error(err, "failed to list httproutes for reference ReferenceGrant", "ReferenceGrant", types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}) + return nil + } + + for _, httpRoute := range httpRouteList.Items { + for _, from := range grant.Spec.From { + hr := v1beta1.ReferenceGrantFrom{ + Group: gatewayv1.GroupName, + Kind: KindHTTPRoute, + Namespace: v1beta1.Namespace(httpRoute.GetNamespace()), + } + if from == hr { + requests = append(requests, reconcile.Request{ + NamespacedName: client.ObjectKey{ + Namespace: httpRoute.GetNamespace(), + Name: httpRoute.GetName(), + }, + }) + } + } + } + return requests +} + +func httpRouteReferenceGrantPredicates() predicate.Funcs { + var filter = func(obj client.Object) bool { + grant, ok := obj.(*v1beta1.ReferenceGrant) + if !ok { + return false + } + for _, from := range grant.Spec.From { + if from.Kind == KindHTTPRoute && string(from.Group) == gatewayv1.GroupName { + return true + } + } + return false + } + predicates := predicate.NewPredicateFuncs(filter) + predicates.UpdateFunc = func(e event.UpdateEvent) bool { + return filter(e.ObjectOld) || filter(e.ObjectNew) + } + return predicates +} + +func checkReferenceGrantBetweenHTTPRouteAndService(hrNN, targetNN types.NamespacedName, grants []v1beta1.ReferenceGrant) bool { + hr := v1beta1.ReferenceGrantFrom{ + Group: gatewayv1.GroupName, + Kind: KindHTTPRoute, + Namespace: v1beta1.Namespace(hrNN.Namespace), + } + for _, grant := range grants { + fromHTTPRoute := slices.ContainsFunc(grant.Spec.From, func(from v1beta1.ReferenceGrantFrom) bool { + return from == hr + }) + toService := slices.ContainsFunc(grant.Spec.To, func(to v1beta1.ReferenceGrantTo) bool { + return to.Group == corev1.GroupName && to.Kind == KindService && (to.Name == nil || string(*to.Name) == targetNN.Name) + }) + if fromHTTPRoute && toService { + return true + } + } + return false +} diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 5dd2e7f5..6572c0fe 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -47,6 +47,7 @@ const ( KindIngressClass = "IngressClass" KindGatewayProxy = "GatewayProxy" KindSecret = "Secret" + KindService = "Service" ) const defaultIngressClassAnnotation = "ingressclass.kubernetes.io/is-default-class" @@ -244,33 +245,22 @@ func SetRouteConditionAccepted(routeParentStatus *gatewayv1.RouteParentStatus, g // SetRouteConditionResolvedRefs sets the ResolvedRefs condition with proper reason based on error type func SetRouteConditionResolvedRefs(routeParentStatus *gatewayv1.RouteParentStatus, generation int64, err error) { - var ( - reason string - status = metav1.ConditionTrue - message = "backendRefs are resolved" - ) - - if err != nil { - status = metav1.ConditionFalse - message = err.Error() - reason = string(gatewayv1.RouteReasonResolvedRefs) - - if IsInvalidKindError(err) { - reason = string(gatewayv1.RouteReasonInvalidKind) - } else if IsBackendNotFoundError(err) { - reason = string(gatewayv1.RouteReasonBackendNotFound) - } - } else { - reason = string(gatewayv1.RouteReasonResolvedRefs) - } - - condition := metav1.Condition{ + var condition = metav1.Condition{ Type: string(gatewayv1.RouteConditionResolvedRefs), - Status: status, - Reason: reason, + Status: metav1.ConditionTrue, ObservedGeneration: generation, - Message: message, LastTransitionTime: metav1.Now(), + Reason: string(gatewayv1.RouteReasonResolvedRefs), + Message: "backendRefs are resolved", + } + + if err != nil { + condition.Status = metav1.ConditionFalse + condition.Message = err.Error() + + if re := new(ReasonError); errors.As(err, &re) { + condition.Reason = re.Reason + } } if !IsConditionPresentAndEqual(routeParentStatus.Conditions, condition) { @@ -914,50 +904,34 @@ func FullTypeName(a any) string { return path.Join(path.Dir(pkgPath), name) } -// InvalidKindError represents an error when backend reference kind is not supported -type InvalidKindError struct { - Kind string +type ReasonError struct { + Reason string + Message string } -// Error implements the error interface -func (e *InvalidKindError) Error() string { - return fmt.Sprintf("%s %s", string(gatewayv1.RouteReasonInvalidKind), e.Kind) +func (e ReasonError) Error() string { + return e.Message } -// NewInvalidKindError creates a new InvalidKindError -func NewInvalidKindError(kind string) *InvalidKindError { - return &InvalidKindError{Kind: kind} -} - -// IsInvalidKindError checks if the error is an InvalidKindError -func IsInvalidKindError(err error) bool { - _, ok := err.(*InvalidKindError) - return ok -} - -// BackendNotFoundError represents an error when a backend service is not found -type BackendNotFoundError struct { - Name string - Namespace string -} - -// Error implements the error interface -func (e *BackendNotFoundError) Error() string { - return fmt.Sprintf("Service %s/%s not found", e.Namespace, e.Name) -} - -// NewBackendNotFoundError creates a new BackendNotFoundError -func NewBackendNotFoundError(namespace, name string) *BackendNotFoundError { - return &BackendNotFoundError{ - Name: name, - Namespace: namespace, +func IsSomeReasonError[Reason ~string](err error, reasons ...Reason) bool { + if err == nil { + return false + } + var re = new(ReasonError) + if !errors.As(err, &re) { + return false } + if len(reasons) == 0 { + return true + } + return slices.Contains(reasons, Reason(re.Reason)) } -// IsBackendNotFoundError checks if the error is a BackendNotFoundError -func IsBackendNotFoundError(err error) bool { - _, ok := err.(*BackendNotFoundError) - return ok +func newInvalidKindError[Kind ~string](kind Kind) *ReasonError { + return &ReasonError{ + Reason: string(gatewayv1.RouteReasonInvalidKind), + Message: fmt.Sprintf("Invalid kind %s, only Service is supported", kind), + } } // filterHostnames accepts a list of gateways and an HTTPRoute, and returns a copy of the HTTPRoute with only the hostnames that match the listener hostnames of the gateways. diff --git a/internal/manager/run.go b/internal/manager/run.go index 769cd4f8..c8156ba9 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -19,7 +19,9 @@ import ( "time" "github.com/go-logr/logr" + "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/ptr" @@ -29,6 +31,7 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/apache/apisix-ingress-controller/api/v1alpha1" "github.com/apache/apisix-ingress-controller/internal/controller/config" @@ -48,6 +51,9 @@ func init() { if err := v1alpha1.AddToScheme(scheme); err != nil { panic(err) } + if err := v1beta1.Install(scheme); err != nil { + panic(err) + } // +kubebuilder:scaffold:scheme } @@ -177,6 +183,14 @@ func Run(ctx context.Context, logger logr.Logger) error { setupLog.Error(err, "unable to set up controllers") return err } + if _, err = mgr.GetRESTMapper().KindsFor(schema.GroupVersionResource{ + Group: v1beta1.GroupVersion.Group, + Version: v1beta1.GroupVersion.Version, + Resource: "referencegrants", + }); err != nil { + logger.Error(err, "CRD ReferenceGrants is not installed") + return errors.Wrap(err, "CRD ReferenceGrants is not installed") + } for _, c := range controllers { if err := c.SetupWithManager(mgr); err != nil { return err diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index bd6362fb..f4db5994 100644 --- a/test/conformance/conformance_test.go +++ b/test/conformance/conformance_test.go @@ -28,10 +28,6 @@ var skippedTestsForTraditionalRoutes = []string{ // TODO: Support ReferenceGrant resource tests.GatewaySecretInvalidReferenceGrant.ShortName, tests.GatewaySecretMissingReferenceGrant.ShortName, - tests.HTTPRouteInvalidCrossNamespaceBackendRef.ShortName, - tests.HTTPRouteInvalidReferenceGrant.ShortName, - tests.HTTPRoutePartiallyInvalidViaInvalidReferenceGrant.ShortName, - tests.HTTPRouteReferenceGrant.ShortName, // TODO: HTTPRoute hostname intersection and listener hostname matching } diff --git a/test/e2e/framework/manifests/ingress.yaml b/test/e2e/framework/manifests/ingress.yaml index fc63585f..4aeabb89 100644 --- a/test/e2e/framework/manifests/ingress.yaml +++ b/test/e2e/framework/manifests/ingress.yaml @@ -229,6 +229,20 @@ rules: verbs: - get - update +- apiGroups: + - gateway.networking.k8s.io + resources: + - referencegrants + verbs: + - get + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - referencegrants/status + verbs: + - get - apiGroups: - networking.k8s.io resources: From 37f55115e3d38ce6b264d28279a00f6bf4497981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 10:06:21 +0800 Subject: [PATCH 04/11] fix: --- internal/controller/httproute_controller.go | 23 +++++++++------------ internal/controller/utils.go | 9 ++++---- test/conformance/conformance_test.go | 7 +++++++ 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 757df05a..e9344fee 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -174,11 +174,12 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } } - var httpRouteErr error + var backendRefErr error if err := r.processHTTPRoute(tctx, hr); err != nil { - httpRouteErr = err // When encountering a backend reference error, it should not affect the acceptance status - if !IsSomeReasonError(err, gatewayv1.RouteReasonInvalidKind) { + if IsSomeReasonError(err, gatewayv1.RouteReasonInvalidKind) { + backendRefErr = err + } else { acceptStatus.status = false acceptStatus.msg = err.Error() } @@ -189,16 +190,12 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( acceptStatus.msg = err.Error() } - // Store the backend reference error for later use - var backendRefErr error - if err := r.processHTTPRouteBackendRefs(tctx, req.NamespacedName); err != nil { + // Store the backend reference error for later use. + // If the backend reference error is because of an invalid kind, use this error first + if err := r.processHTTPRouteBackendRefs(tctx, req.NamespacedName); err != nil && backendRefErr == nil { backendRefErr = err } - // If the backend reference error is because of an invalid kind, use this error first - if IsSomeReasonError(err, gatewayv1.RouteReasonInvalidKind) { - backendRefErr = httpRouteErr - } ProcessBackendTrafficPolicy(r.Client, r.Log, tctx) filteredHTTPRoute, err := filterHostnames(gateways, hr.DeepCopy()) @@ -436,7 +433,7 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla } if backend.Kind != nil && *backend.Kind != "Service" { - terr = &ReasonError{ + terr = ReasonError{ Reason: string(gatewayv1.RouteReasonInvalidKind), Message: fmt.Sprintf("invalid kind %s, only Service is supported", *backend.Kind), } @@ -452,7 +449,7 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla if err := r.Get(tctx, targetNN, &service); err != nil { terr = err if client.IgnoreNotFound(err) == nil { - terr = &ReasonError{ + terr = ReasonError{ Reason: string(gatewayv1.RouteReasonBackendNotFound), Message: fmt.Sprintf("Service %s not found", targetNN), } @@ -468,7 +465,7 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla return err } if !checkReferenceGrantBetweenHTTPRouteAndService(hrNN, targetNN, referenceGrantList.Items) { - terr = &ReasonError{ + terr = ReasonError{ Reason: string(v1beta1.RouteReasonRefNotPermitted), Message: fmt.Sprintf("%s is in a different namespace than the HTTPRoute %s and no ReferenceGrant allowing reference is configured", targetNN, hrNN), } diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 6572c0fe..f426b013 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -258,7 +258,8 @@ func SetRouteConditionResolvedRefs(routeParentStatus *gatewayv1.RouteParentStatu condition.Status = metav1.ConditionFalse condition.Message = err.Error() - if re := new(ReasonError); errors.As(err, &re) { + var re ReasonError + if errors.As(err, &re) { condition.Reason = re.Reason } } @@ -917,7 +918,7 @@ func IsSomeReasonError[Reason ~string](err error, reasons ...Reason) bool { if err == nil { return false } - var re = new(ReasonError) + var re ReasonError if !errors.As(err, &re) { return false } @@ -927,8 +928,8 @@ func IsSomeReasonError[Reason ~string](err error, reasons ...Reason) bool { return slices.Contains(reasons, Reason(re.Reason)) } -func newInvalidKindError[Kind ~string](kind Kind) *ReasonError { - return &ReasonError{ +func newInvalidKindError[Kind ~string](kind Kind) ReasonError { + return ReasonError{ Reason: string(gatewayv1.RouteReasonInvalidKind), Message: fmt.Sprintf("Invalid kind %s, only Service is supported", kind), } diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index f4db5994..86349078 100644 --- a/test/conformance/conformance_test.go +++ b/test/conformance/conformance_test.go @@ -63,6 +63,13 @@ func TestGatewayAPIConformance(t *testing.T) { require.NoError(t, err) t.Log("starting the gateway conformance test suite") + tests.ConformanceTests = []suite.ConformanceTest{ + tests.HTTPRouteInvalidBackendRefUnknownKind, + tests.HTTPRouteInvalidCrossNamespaceBackendRef, + tests.HTTPRouteInvalidReferenceGrant, + tests.HTTPRoutePartiallyInvalidViaInvalidReferenceGrant, + tests.HTTPRouteReferenceGrant, + } cSuite.Setup(t, tests.ConformanceTests) if err := cSuite.Run(t, tests.ConformanceTests); err != nil { From fa2ab4526d6eaf05f924868407ada97e9d2fe35c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 11:17:57 +0800 Subject: [PATCH 05/11] refactor: Centralize and streamline ReferenceGrant validation Reorganized and simplified the predicate logic for ReferenceGrant handling across gateway and HTTPRoute controllers. Consolidated duplicate code into reusable functions, reducing redundancy and improving maintainability. This centralization ensures consistent behavior and clearer code structure. --- internal/controller/gateway_controller.go | 41 ++-------- internal/controller/httproute_controller.go | 66 +++++----------- internal/controller/utils.go | 86 +++++++++++++-------- 3 files changed, 81 insertions(+), 112 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 4278e985..efc598bc 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -27,7 +27,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -87,20 +86,7 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&v1beta1.ReferenceGrant{}, handler.EnqueueRequestsFromMapFunc(r.listReferenceGrantsForGateway), - builder.WithPredicates(predicate.Funcs{ - CreateFunc: func(e event.CreateEvent) bool { - return referenceGrantHasGatewayFrom(e.Object) - }, - UpdateFunc: func(e event.UpdateEvent) bool { - return referenceGrantHasGatewayFrom(e.ObjectOld) || referenceGrantHasGatewayFrom(e.ObjectNew) - }, - DeleteFunc: func(e event.DeleteEvent) bool { - return referenceGrantHasGatewayFrom(e.Object) - }, - GenericFunc: func(e event.GenericEvent) bool { - return referenceGrantHasGatewayFrom(e.Object) - }, - }), + builder.WithPredicates(referenceGrantPredicates(KindGateway)), ). Complete(r) } @@ -190,7 +176,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } listenerStatuses, err := getListenerStatus(ctx, r.Client, gateway, referenceGrantList.Items) if err != nil { - r.Log.Error(err, "failed to get listener status", "gateway", types.NamespacedName{Namespace: gateway.GetNamespace(), Name: gateway.GetName()}) + r.Log.Error(err, "failed to get listener status", "gateway", types.NamespacedName{Namespace: gateway.GetNamespace()}) return ctrl.Result{}, err } @@ -390,12 +376,12 @@ func (r *GatewayReconciler) listReferenceGrantsForGateway(ctx context.Context, o } for _, gateway := range gatewayList.Items { + gw := v1beta1.ReferenceGrantFrom{ + Group: gatewayv1.GroupName, + Kind: KindGateway, + Namespace: v1beta1.Namespace(gateway.GetNamespace()), + } for _, from := range grant.Spec.From { - gw := v1beta1.ReferenceGrantFrom{ - Group: gatewayv1.GroupName, - Kind: KindGateway, - Namespace: v1beta1.Namespace(gateway.GetNamespace()), - } if from == gw { requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ @@ -409,19 +395,6 @@ func (r *GatewayReconciler) listReferenceGrantsForGateway(ctx context.Context, o return requests } -func referenceGrantHasGatewayFrom(obj client.Object) bool { - grant, ok := obj.(*v1beta1.ReferenceGrant) - if !ok { - return false - } - for _, from := range grant.Spec.From { - if from.Kind == KindGateway && string(from.Group) == gatewayv1.GroupName { - return true - } - } - return false -} - func (r *GatewayReconciler) processInfrastructure(tctx *provider.TranslateContext, gateway *gatewayv1.Gateway) error { rk := provider.ResourceKind{ Kind: gateway.Kind, diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index e9344fee..51cf51e2 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -104,7 +104,7 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&v1beta1.ReferenceGrant{}, handler.EnqueueRequestsFromMapFunc(r.lisHTTPRoutesForReferenceGrant), - builder.WithPredicates(httpRouteReferenceGrantPredicates()), + builder.WithPredicates(referenceGrantPredicates(KindHTTPRoute)), ). WatchesRawSource( source.Channel( @@ -464,7 +464,19 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla r.Log.Error(err, "failed to list ReferenceGrants", "namespace", targetNN.Namespace) return err } - if !checkReferenceGrantBetweenHTTPRouteAndService(hrNN, targetNN, referenceGrantList.Items) { + if !checkReferenceGrant( + v1beta1.ReferenceGrantFrom{ + Group: gatewayv1.GroupName, + Kind: KindHTTPRoute, + Namespace: v1beta1.Namespace(hrNN.Namespace), + }, + v1beta1.ReferenceGrantTo{ + Group: corev1.GroupName, + Kind: KindService, + Name: (*gatewayv1.ObjectName)(&targetNN.Name), + }, + referenceGrantList.Items, + ) { terr = ReasonError{ Reason: string(v1beta1.RouteReasonRefNotPermitted), Message: fmt.Sprintf("%s is in a different namespace than the HTTPRoute %s and no ReferenceGrant allowing reference is configured", targetNN, hrNN), @@ -639,12 +651,12 @@ func (r *HTTPRouteReconciler) lisHTTPRoutesForReferenceGrant(ctx context.Context } for _, httpRoute := range httpRouteList.Items { + hr := v1beta1.ReferenceGrantFrom{ + Group: gatewayv1.GroupName, + Kind: KindHTTPRoute, + Namespace: v1beta1.Namespace(httpRoute.GetNamespace()), + } for _, from := range grant.Spec.From { - hr := v1beta1.ReferenceGrantFrom{ - Group: gatewayv1.GroupName, - Kind: KindHTTPRoute, - Namespace: v1beta1.Namespace(httpRoute.GetNamespace()), - } if from == hr { requests = append(requests, reconcile.Request{ NamespacedName: client.ObjectKey{ @@ -657,43 +669,3 @@ func (r *HTTPRouteReconciler) lisHTTPRoutesForReferenceGrant(ctx context.Context } return requests } - -func httpRouteReferenceGrantPredicates() predicate.Funcs { - var filter = func(obj client.Object) bool { - grant, ok := obj.(*v1beta1.ReferenceGrant) - if !ok { - return false - } - for _, from := range grant.Spec.From { - if from.Kind == KindHTTPRoute && string(from.Group) == gatewayv1.GroupName { - return true - } - } - return false - } - predicates := predicate.NewPredicateFuncs(filter) - predicates.UpdateFunc = func(e event.UpdateEvent) bool { - return filter(e.ObjectOld) || filter(e.ObjectNew) - } - return predicates -} - -func checkReferenceGrantBetweenHTTPRouteAndService(hrNN, targetNN types.NamespacedName, grants []v1beta1.ReferenceGrant) bool { - hr := v1beta1.ReferenceGrantFrom{ - Group: gatewayv1.GroupName, - Kind: KindHTTPRoute, - Namespace: v1beta1.Namespace(hrNN.Namespace), - } - for _, grant := range grants { - fromHTTPRoute := slices.ContainsFunc(grant.Spec.From, func(from v1beta1.ReferenceGrantFrom) bool { - return from == hr - }) - toService := slices.ContainsFunc(grant.Spec.To, func(to v1beta1.ReferenceGrantTo) bool { - return to.Group == corev1.GroupName && to.Kind == KindService && (to.Name == nil || string(*to.Name) == targetNN.Name) - }) - if fromHTTPRoute && toService { - return true - } - } - return false -} diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 105d4136..6ad20d7d 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -13,6 +13,7 @@ package controller import ( + "cmp" "context" "encoding/pem" "errors" @@ -31,6 +32,8 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -736,19 +739,32 @@ func getListenerStatus( conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) break } - if ok := checkReferenceGrantBetweenGatewayAndSecret(gateway.Namespace, ref, grants); !ok { - conditionResolvedRefs.Status = metav1.ConditionFalse - conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonRefNotPermitted) - conditionResolvedRefs.Message = "certificateRefs cross namespaces is not permitted" - conditionProgrammed.Status = metav1.ConditionFalse - conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) - break - } - ns := gateway.Namespace - if ref.Namespace != nil { - ns = string(*ref.Namespace) + // if cross namespaces, check if the Gateway has the permission to access the Secret + if ref.Namespace != nil && string(*ref.Namespace) != gateway.Namespace { + if ok := checkReferenceGrant( + v1beta1.ReferenceGrantFrom{ + Group: gatewayv1.GroupName, + Kind: KindGateway, + Namespace: v1beta1.Namespace(gateway.Namespace), + }, + v1beta1.ReferenceGrantTo{ + Group: corev1.GroupName, + Kind: KindSecret, + Name: &ref.Name, + }, + grants, + ); !ok { + conditionResolvedRefs.Status = metav1.ConditionFalse + conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonRefNotPermitted) + conditionResolvedRefs.Message = "certificateRefs cross namespaces is not permitted" + conditionProgrammed.Status = metav1.ConditionFalse + conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) + break + } } - if err := mrgc.Get(ctx, client.ObjectKey{Namespace: ns, Name: string(ref.Name)}, &secret); err != nil { + + ns := cmp.Or(ref.Namespace, (*gatewayv1.Namespace)(&gateway.Namespace)) + if err := mrgc.Get(ctx, client.ObjectKey{Namespace: string(*ns), Name: string(ref.Name)}, &secret); err != nil { conditionResolvedRefs.Status = metav1.ConditionFalse conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonInvalidCertificateRef) conditionResolvedRefs.Message = err.Error() @@ -1070,28 +1086,36 @@ func isTLSSecretValid(secret *corev1.Secret) (string, bool) { return "", true } -func checkReferenceGrantBetweenGatewayAndSecret(gwNamespace string, certRef gatewayv1.SecretObjectReference, grants []v1beta1.ReferenceGrant) bool { - // if not cross namespaces - if certRef.Namespace == nil || string(*certRef.Namespace) == gwNamespace { - return true +func referenceGrantPredicates(kind gatewayv1.Kind) predicate.Funcs { + var filter = func(obj client.Object) bool { + grant, ok := obj.(*v1beta1.ReferenceGrant) + if !ok { + return false + } + for _, from := range grant.Spec.From { + if from.Kind == kind && string(from.Group) == gatewayv1.GroupName { + return true + } + } + return false + } + predicates := predicate.NewPredicateFuncs(filter) + predicates.UpdateFunc = func(e event.UpdateEvent) bool { + return filter(e.ObjectOld) || filter(e.ObjectNew) } + return predicates +} +func checkReferenceGrant(from v1beta1.ReferenceGrantFrom, to v1beta1.ReferenceGrantTo, grants []v1beta1.ReferenceGrant) bool { for _, grant := range grants { - if grant.Namespace == string(*certRef.Namespace) { - for _, from := range grant.Spec.From { - gw := v1beta1.ReferenceGrantFrom{ - Group: gatewayv1.GroupName, - Kind: KindGateway, - Namespace: v1beta1.Namespace(gwNamespace), - } - if from == gw { - for _, to := range grant.Spec.To { - if to.Group == corev1.GroupName && to.Kind == KindSecret && (to.Name == nil || *to.Name == certRef.Name) { - return true - } - } - } - } + grantFrom := slices.ContainsFunc(grant.Spec.From, func(item v1beta1.ReferenceGrantFrom) bool { + return item == from + }) + grantTo := slices.ContainsFunc(grant.Spec.To, func(item v1beta1.ReferenceGrantTo) bool { + return item.Group == to.Group && item.Kind == to.Kind && to.Name != nil && (item.Name == nil || *item.Name == *to.Name) + }) + if grantFrom && grantTo { + return true } } return false From de81779f23dc685a772c583fdb3ce796f49f9234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 11:49:14 +0800 Subject: [PATCH 06/11] refactor: Simplify permission checking in ReferenceGrant validation --- internal/controller/httproute_controller.go | 4 ++-- internal/controller/utils.go | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 51cf51e2..ed89b0ef 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -464,7 +464,7 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla r.Log.Error(err, "failed to list ReferenceGrants", "namespace", targetNN.Namespace) return err } - if !checkReferenceGrant( + if permitted := checkReferenceGrant( v1beta1.ReferenceGrantFrom{ Group: gatewayv1.GroupName, Kind: KindHTTPRoute, @@ -476,7 +476,7 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla Name: (*gatewayv1.ObjectName)(&targetNN.Name), }, referenceGrantList.Items, - ) { + ); !permitted { terr = ReasonError{ Reason: string(v1beta1.RouteReasonRefNotPermitted), Message: fmt.Sprintf("%s is in a different namespace than the HTTPRoute %s and no ReferenceGrant allowing reference is configured", targetNN, hrNN), diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 6ad20d7d..637eec7a 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -741,7 +741,7 @@ func getListenerStatus( } // if cross namespaces, check if the Gateway has the permission to access the Secret if ref.Namespace != nil && string(*ref.Namespace) != gateway.Namespace { - if ok := checkReferenceGrant( + if permitted := checkReferenceGrant( v1beta1.ReferenceGrantFrom{ Group: gatewayv1.GroupName, Kind: KindGateway, @@ -753,7 +753,7 @@ func getListenerStatus( Name: &ref.Name, }, grants, - ); !ok { + ); !permitted { conditionResolvedRefs.Status = metav1.ConditionFalse conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonRefNotPermitted) conditionResolvedRefs.Message = "certificateRefs cross namespaces is not permitted" @@ -763,8 +763,11 @@ func getListenerStatus( } } - ns := cmp.Or(ref.Namespace, (*gatewayv1.Namespace)(&gateway.Namespace)) - if err := mrgc.Get(ctx, client.ObjectKey{Namespace: string(*ns), Name: string(ref.Name)}, &secret); err != nil { + secretNN := types.NamespacedName{ + Namespace: string(*cmp.Or(ref.Namespace, (*gatewayv1.Namespace)(&gateway.Namespace))), + Name: string(ref.Name), + } + if err := mrgc.Get(ctx, secretNN, &secret); err != nil { conditionResolvedRefs.Status = metav1.ConditionFalse conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonInvalidCertificateRef) conditionResolvedRefs.Message = err.Error() From 7c9925b93ffe144a5dd184bc38226a5c5891a00f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 14:40:53 +0800 Subject: [PATCH 07/11] refactor: Update ReferenceGrant validation to use ObjectReference for cross-namespace checks --- internal/controller/httproute_controller.go | 9 +-- internal/controller/utils.go | 65 +++++++++++---------- 2 files changed, 39 insertions(+), 35 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index ed89b0ef..d80b00b3 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -470,10 +470,11 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla Kind: KindHTTPRoute, Namespace: v1beta1.Namespace(hrNN.Namespace), }, - v1beta1.ReferenceGrantTo{ - Group: corev1.GroupName, - Kind: KindService, - Name: (*gatewayv1.ObjectName)(&targetNN.Name), + gatewayv1.ObjectReference{ + Group: corev1.GroupName, + Kind: KindService, + Name: gatewayv1.ObjectName(targetNN.Name), + Namespace: (*gatewayv1.Namespace)(&targetNN.Namespace), }, referenceGrantList.Items, ); !permitted { diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 637eec7a..859a5680 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -739,28 +739,26 @@ func getListenerStatus( conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) break } - // if cross namespaces, check if the Gateway has the permission to access the Secret - if ref.Namespace != nil && string(*ref.Namespace) != gateway.Namespace { - if permitted := checkReferenceGrant( - v1beta1.ReferenceGrantFrom{ - Group: gatewayv1.GroupName, - Kind: KindGateway, - Namespace: v1beta1.Namespace(gateway.Namespace), - }, - v1beta1.ReferenceGrantTo{ - Group: corev1.GroupName, - Kind: KindSecret, - Name: &ref.Name, - }, - grants, - ); !permitted { - conditionResolvedRefs.Status = metav1.ConditionFalse - conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonRefNotPermitted) - conditionResolvedRefs.Message = "certificateRefs cross namespaces is not permitted" - conditionProgrammed.Status = metav1.ConditionFalse - conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) - break - } + if permitted := checkReferenceGrant( + v1beta1.ReferenceGrantFrom{ + Group: gatewayv1.GroupName, + Kind: KindGateway, + Namespace: v1beta1.Namespace(gateway.Namespace), + }, + gatewayv1.ObjectReference{ + Group: corev1.GroupName, + Kind: KindSecret, + Name: ref.Name, + Namespace: ref.Namespace, + }, + grants, + ); !permitted { + conditionResolvedRefs.Status = metav1.ConditionFalse + conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonRefNotPermitted) + conditionResolvedRefs.Message = "certificateRefs cross namespaces is not permitted" + conditionProgrammed.Status = metav1.ConditionFalse + conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) + break } secretNN := types.NamespacedName{ @@ -1109,16 +1107,21 @@ func referenceGrantPredicates(kind gatewayv1.Kind) predicate.Funcs { return predicates } -func checkReferenceGrant(from v1beta1.ReferenceGrantFrom, to v1beta1.ReferenceGrantTo, grants []v1beta1.ReferenceGrant) bool { +func checkReferenceGrant(obj v1beta1.ReferenceGrantFrom, ref gatewayv1.ObjectReference, grants []v1beta1.ReferenceGrant) bool { + if ref.Namespace == nil || *ref.Namespace == obj.Namespace { + return true + } for _, grant := range grants { - grantFrom := slices.ContainsFunc(grant.Spec.From, func(item v1beta1.ReferenceGrantFrom) bool { - return item == from - }) - grantTo := slices.ContainsFunc(grant.Spec.To, func(item v1beta1.ReferenceGrantTo) bool { - return item.Group == to.Group && item.Kind == to.Kind && to.Name != nil && (item.Name == nil || *item.Name == *to.Name) - }) - if grantFrom && grantTo { - return true + if grant.Namespace == string(*ref.Namespace) { + for _, from := range grant.Spec.From { + if obj == from { + for _, to := range grant.Spec.To { + if to.Group == ref.Group && to.Kind == ref.Kind && (to.Name == nil || *to.Name == ref.Name) { + return true + } + } + } + } } } return false From b8d945035072465f612da14ad3e116fed20bfa87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 16:30:55 +0800 Subject: [PATCH 08/11] feat: Enable ReferenceGrant checks and streamline validation logic --- internal/controller/gateway_controller.go | 7 +---- internal/controller/httproute_controller.go | 9 ++---- internal/controller/utils.go | 31 +++++++++++++++++---- internal/manager/run.go | 22 +++++++++------ 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index efc598bc..a7683f7e 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -169,12 +169,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } } - var referenceGrantList v1beta1.ReferenceGrantList - if err := r.List(ctx, &referenceGrantList); err != nil { - r.Log.Error(err, "failed to list reference grants") - return ctrl.Result{}, err - } - listenerStatuses, err := getListenerStatus(ctx, r.Client, gateway, referenceGrantList.Items) + listenerStatuses, err := getListenerStatus(ctx, r.Client, gateway) if err != nil { r.Log.Error(err, "failed to get listener status", "gateway", types.NamespacedName{Namespace: gateway.GetNamespace()}) return ctrl.Result{}, err diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index d80b00b3..da8b5062 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -459,12 +459,8 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla // if cross namespaces between HTTPRoute and referenced Service, check ReferenceGrant if hrNN.Namespace != targetNN.Namespace { - var referenceGrantList v1beta1.ReferenceGrantList - if err := r.List(tctx, &referenceGrantList, client.InNamespace(targetNN.Namespace)); err != nil { - r.Log.Error(err, "failed to list ReferenceGrants", "namespace", targetNN.Namespace) - return err - } - if permitted := checkReferenceGrant( + if permitted := checkReferenceGrant(tctx, + r.Client, v1beta1.ReferenceGrantFrom{ Group: gatewayv1.GroupName, Kind: KindHTTPRoute, @@ -476,7 +472,6 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla Name: gatewayv1.ObjectName(targetNN.Name), Namespace: (*gatewayv1.Namespace)(&targetNN.Namespace), }, - referenceGrantList.Items, ); !permitted { terr = ReasonError{ Reason: string(v1beta1.RouteReasonRefNotPermitted), diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 859a5680..4c5de816 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -60,6 +60,18 @@ var ( ErrNoMatchingListenerHostname = errors.New("no matching hostnames in listener") ) +var ( + enableReferenceGrant bool +) + +func SetEnableReferenceGrant(enable bool) { + enableReferenceGrant = enable +} + +func GetEnableReferenceGrant() bool { + return enableReferenceGrant +} + // IsDefaultIngressClass returns whether an IngressClass is the default IngressClass. func IsDefaultIngressClass(obj client.Object) bool { if ingressClass, ok := obj.(*networkingv1.IngressClass); ok { @@ -651,7 +663,6 @@ func getListenerStatus( ctx context.Context, mrgc client.Client, gateway *gatewayv1.Gateway, - grants []v1beta1.ReferenceGrant, ) ([]gatewayv1.ListenerStatus, error) { statuses := make(map[gatewayv1.SectionName]gatewayv1.ListenerStatus, len(gateway.Spec.Listeners)) @@ -739,7 +750,8 @@ func getListenerStatus( conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid) break } - if permitted := checkReferenceGrant( + if permitted := checkReferenceGrant(ctx, + mrgc, v1beta1.ReferenceGrantFrom{ Group: gatewayv1.GroupName, Kind: KindGateway, @@ -751,7 +763,6 @@ func getListenerStatus( Name: ref.Name, Namespace: ref.Namespace, }, - grants, ); !permitted { conditionResolvedRefs.Status = metav1.ConditionFalse conditionResolvedRefs.Reason = string(gatewayv1.ListenerReasonRefNotPermitted) @@ -1107,11 +1118,21 @@ func referenceGrantPredicates(kind gatewayv1.Kind) predicate.Funcs { return predicates } -func checkReferenceGrant(obj v1beta1.ReferenceGrantFrom, ref gatewayv1.ObjectReference, grants []v1beta1.ReferenceGrant) bool { +func checkReferenceGrant(ctx context.Context, cli client.Client, obj v1beta1.ReferenceGrantFrom, ref gatewayv1.ObjectReference) bool { if ref.Namespace == nil || *ref.Namespace == obj.Namespace { return true } - for _, grant := range grants { + + if !GetEnableReferenceGrant() { + return false + } + + var grantList v1beta1.ReferenceGrantList + if err := cli.List(ctx, &grantList, client.InNamespace(*ref.Namespace)); err != nil { + return false + } + + for _, grant := range grantList.Items { if grant.Namespace == string(*ref.Namespace) { for _, from := range grant.Spec.From { if obj == from { diff --git a/internal/manager/run.go b/internal/manager/run.go index c8156ba9..91d2a00f 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -19,7 +19,6 @@ import ( "time" "github.com/go-logr/logr" - "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -34,6 +33,7 @@ import ( "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/apache/apisix-ingress-controller/api/v1alpha1" + "github.com/apache/apisix-ingress-controller/internal/controller" "github.com/apache/apisix-ingress-controller/internal/controller/config" "github.com/apache/apisix-ingress-controller/internal/provider/adc" ) @@ -177,20 +177,24 @@ func Run(ctx context.Context, logger logr.Logger) error { } }() + setupLog.Info("check ReferenceGrants is enabled") + _, err = mgr.GetRESTMapper().KindsFor(schema.GroupVersionResource{ + Group: v1beta1.GroupVersion.Group, + Version: v1beta1.GroupVersion.Version, + Resource: "referencegrants", + }) + if err != nil { + logger.Error(err, "CRD ReferenceGrants is not installed") + } + controller.SetEnableReferenceGrant(err == nil) + setupLog.Info("setting up controllers") controllers, err := setupControllers(ctx, mgr, provider) if err != nil { setupLog.Error(err, "unable to set up controllers") return err } - if _, err = mgr.GetRESTMapper().KindsFor(schema.GroupVersionResource{ - Group: v1beta1.GroupVersion.Group, - Version: v1beta1.GroupVersion.Version, - Resource: "referencegrants", - }); err != nil { - logger.Error(err, "CRD ReferenceGrants is not installed") - return errors.Wrap(err, "CRD ReferenceGrants is not installed") - } + for _, c := range controllers { if err := c.SetupWithManager(mgr); err != nil { return err From c5062fbff32cbc783a92b27e008905af76944052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 17:27:51 +0800 Subject: [PATCH 09/11] fix: Correct function name for listing HTTP routes in ReferenceGrant handling --- internal/controller/gateway_controller.go | 2 +- internal/controller/httproute_controller.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index a7683f7e..6011d7a8 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -171,7 +171,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct listenerStatuses, err := getListenerStatus(ctx, r.Client, gateway) if err != nil { - r.Log.Error(err, "failed to get listener status", "gateway", types.NamespacedName{Namespace: gateway.GetNamespace()}) + r.Log.Error(err, "failed to get listener status", "gateway", req.NamespacedName) return ctrl.Result{}, err } diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index da8b5062..9cb2aec2 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -103,7 +103,7 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { handler.EnqueueRequestsFromMapFunc(r.listHTTPRoutesForGatewayProxy), ). Watches(&v1beta1.ReferenceGrant{}, - handler.EnqueueRequestsFromMapFunc(r.lisHTTPRoutesForReferenceGrant), + handler.EnqueueRequestsFromMapFunc(r.listHTTPRoutesForReferenceGrant), builder.WithPredicates(referenceGrantPredicates(KindHTTPRoute)), ). WatchesRawSource( @@ -633,7 +633,7 @@ func (r *HTTPRouteReconciler) listHTTPRoutesForGatewayProxy(ctx context.Context, return requests } -func (r *HTTPRouteReconciler) lisHTTPRoutesForReferenceGrant(ctx context.Context, obj client.Object) (requests []reconcile.Request) { +func (r *HTTPRouteReconciler) listHTTPRoutesForReferenceGrant(ctx context.Context, obj client.Object) (requests []reconcile.Request) { grant, ok := obj.(*v1beta1.ReferenceGrant) if !ok { r.Log.Error(fmt.Errorf("unexpected object type"), "failed to convert object to ReferenceGrant") From bdb20588ce9497fc1fd4bdd9dc6261935a28ded6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 18:25:54 +0800 Subject: [PATCH 10/11] resolve comments --- internal/controller/httproute_controller.go | 5 +---- internal/manager/run.go | 2 +- test/conformance/conformance_test.go | 6 +++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 9cb2aec2..ee66879c 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -433,10 +433,7 @@ func (r *HTTPRouteReconciler) processHTTPRouteBackendRefs(tctx *provider.Transla } if backend.Kind != nil && *backend.Kind != "Service" { - terr = ReasonError{ - Reason: string(gatewayv1.RouteReasonInvalidKind), - Message: fmt.Sprintf("invalid kind %s, only Service is supported", *backend.Kind), - } + terr = newInvalidKindError(*backend.Kind) continue } diff --git a/internal/manager/run.go b/internal/manager/run.go index 91d2a00f..2a1daf99 100644 --- a/internal/manager/run.go +++ b/internal/manager/run.go @@ -184,7 +184,7 @@ func Run(ctx context.Context, logger logr.Logger) error { Resource: "referencegrants", }) if err != nil { - logger.Error(err, "CRD ReferenceGrants is not installed") + setupLog.Info("CRD ReferenceGrants is not installed", "err", err) } controller.SetEnableReferenceGrant(err == nil) diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index aefba6af..2e77998e 100644 --- a/test/conformance/conformance_test.go +++ b/test/conformance/conformance_test.go @@ -24,7 +24,7 @@ var skippedTestsForSSL = []string{ tests.HTTPRouteRedirectPortAndScheme.ShortName, } -var skippedTestsForTraditionalRoutes []string // TODO: HTTPRoute hostname intersection and listener hostname matching +// TODO: HTTPRoute hostname intersection and listener hostname matching var gatewaySupportedFeatures = []features.FeatureName{ features.SupportGateway, @@ -44,7 +44,7 @@ func TestGatewayAPIConformance(t *testing.T) { opts.CleanupBaseResources = true opts.GatewayClassName = gatewayClassName opts.SupportedFeatures = sets.New(gatewaySupportedFeatures...) - opts.SkipTests = append(skippedTestsForSSL, skippedTestsForTraditionalRoutes...) + opts.SkipTests = skippedTestsForSSL opts.Implementation = conformancev1.Implementation{ Organization: "APISIX", Project: "apisix-ingress-controller", @@ -73,6 +73,6 @@ func TestGatewayAPIConformance(t *testing.T) { if err != nil { t.Fatalf("failed to marshal the gateway conformance test report: %v", err) } - // Save report in root of the repository, file name is in .gitignore. + // Save report in the root of the repository, file name is in .gitignore. require.NoError(t, os.WriteFile("../../"+reportFileName, rawReport, 0o600)) } From e8da0caecad27b0f54a8a6f9a3543646178b7e06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=82=9F=E7=A9=BA?= Date: Wed, 21 May 2025 18:33:03 +0800 Subject: [PATCH 11/11] resolve comments --- internal/controller/gateway_controller.go | 14 +++++++++----- internal/controller/httproute_controller.go | 18 +++++++++++------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/internal/controller/gateway_controller.go b/internal/controller/gateway_controller.go index 6011d7a8..acf3e9a8 100644 --- a/internal/controller/gateway_controller.go +++ b/internal/controller/gateway_controller.go @@ -49,7 +49,7 @@ type GatewayReconciler struct { //nolint:revive // SetupWithManager sets up the controller with the Manager. func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). + bdr := ctrl.NewControllerManagedBy(mgr). For( &gatewayv1.Gateway{}, builder.WithPredicates( @@ -83,12 +83,16 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches( &corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.listGatewaysForSecret), - ). - Watches(&v1beta1.ReferenceGrant{}, + ) + + if GetEnableReferenceGrant() { + bdr.Watches(&v1beta1.ReferenceGrant{}, handler.EnqueueRequestsFromMapFunc(r.listReferenceGrantsForGateway), builder.WithPredicates(referenceGrantPredicates(KindGateway)), - ). - Complete(r) + ) + } + + return bdr.Complete(r) } func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index ee66879c..ab6814c6 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -61,7 +61,7 @@ type HTTPRouteReconciler struct { //nolint:revive func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { r.genericEvent = make(chan event.GenericEvent, 100) - return ctrl.NewControllerManagedBy(mgr). + bdr := ctrl.NewControllerManagedBy(mgr). For(&gatewayv1.HTTPRoute{}). WithEventFilter(predicate.GenerationChangedPredicate{}). Watches(&discoveryv1.EndpointSlice{}, @@ -102,17 +102,21 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches(&v1alpha1.GatewayProxy{}, handler.EnqueueRequestsFromMapFunc(r.listHTTPRoutesForGatewayProxy), ). - Watches(&v1beta1.ReferenceGrant{}, - handler.EnqueueRequestsFromMapFunc(r.listHTTPRoutesForReferenceGrant), - builder.WithPredicates(referenceGrantPredicates(KindHTTPRoute)), - ). WatchesRawSource( source.Channel( r.genericEvent, handler.EnqueueRequestsFromMapFunc(r.listHTTPRouteForGenericEvent), ), - ). - Complete(r) + ) + + if GetEnableReferenceGrant() { + bdr.Watches(&v1beta1.ReferenceGrant{}, + handler.EnqueueRequestsFromMapFunc(r.listHTTPRoutesForReferenceGrant), + builder.WithPredicates(referenceGrantPredicates(KindHTTPRoute)), + ) + } + + return bdr.Complete(r) } func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {