diff --git a/cmd/root/root.go b/cmd/root/root.go index 577b05677..8252778be 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -35,6 +35,7 @@ import ( // +kubebuilder:scaffold:imports + "github.com/apache/apisix-ingress-controller/internal/controller" "github.com/apache/apisix-ingress-controller/internal/controller/config" "github.com/apache/apisix-ingress-controller/internal/manager" "github.com/apache/apisix-ingress-controller/internal/version" @@ -108,6 +109,9 @@ func newAPISIXIngressController() *cobra.Command { if err := cfg.Validate(); err != nil { return err } + if err := controller.SetNamespaceSelector(cfg.NamespaceSelector); err != nil { + return err + } logLevel, err := zapcore.ParseLevel(cfg.LogLevel) if err != nil { diff --git a/config/samples/config.yaml b/config/samples/config.yaml index 6f37a0a54..6779ef98b 100644 --- a/config/samples/config.yaml +++ b/config/samples/config.yaml @@ -47,6 +47,23 @@ listener_port_match_mode: "off" # Mode for injecting server_port route v # "auto"/"explicit" when those two coincide, otherwise routes bound to a # listener via sectionName/port will never match. +namespace_selector: [] # Label selectors of the namespaces whose resources are handled by the controller. + # A namespace is selected when its labels match all entries. Equality and "in" + # requirements on the same key are merged, so the example below selects namespaces + # labeled team=a or team=b that are also labeled env=prod: + # namespace_selector: + # - "team=a" + # - "team=b" + # - "env=prod" + # Only separate entries are merged. Within one entry, comma-separated requirements + # follow the Kubernetes label selector syntax, so "team=a,team=b" matches nothing. + # It applies to Ingress and apisix.apache.org/v2 resources. Gateway API resources are + # not filtered, use the allowedRoutes of the Gateway listeners instead. Resources they + # reference, such as Services, Secrets and GatewayProxies, are read from any namespace. + # When a namespace stops matching, the configuration of its resources is removed + # from the data plane. + # The default value is empty, which selects all namespaces. Empty entries are ignored. + provider: type: "api7ee" diff --git a/docs/en/latest/reference/configuration-file.md b/docs/en/latest/reference/configuration-file.md index 588d945ae..f83d73f22 100644 --- a/docs/en/latest/reference/configuration-file.md +++ b/docs/en/latest/reference/configuration-file.md @@ -65,6 +65,23 @@ secure_metrics: false # The secure metrics configuration. exec_adc_timeout: 15s # The timeout for the ADC to execute. # The default value is 15 seconds. +namespace_selector: [] # Label selectors of the namespaces whose resources are handled by the controller. + # A namespace is selected when its labels match all entries. Equality and "in" + # requirements on the same key are merged, so the example below selects namespaces + # labeled team=a or team=b that are also labeled env=prod: + # namespace_selector: + # - "team=a" + # - "team=b" + # - "env=prod" + # Only separate entries are merged. Within one entry, comma-separated requirements + # follow the Kubernetes label selector syntax, so "team=a,team=b" matches nothing. + # It applies to Ingress and apisix.apache.org/v2 resources. Gateway API resources are + # not filtered, use the allowedRoutes of the Gateway listeners instead. Resources they + # reference, such as Services, Secrets and GatewayProxies, are read from any namespace. + # When a namespace stops matching, the configuration of its resources is removed + # from the data plane. + # The default value is empty, which selects all namespaces. Empty entries are ignored. + provider: type: "api7ee" # Provider type. diff --git a/docs/en/latest/upgrade-guide.md b/docs/en/latest/upgrade-guide.md index 848ed6a33..a87039ff6 100644 --- a/docs/en/latest/upgrade-guide.md +++ b/docs/en/latest/upgrade-guide.md @@ -90,6 +90,15 @@ Because the Admin API fills in default values, the submitted content may differ | `apisix.*` | Static Admin API configuration | | `etcdserver.*` | Configuration for mock-etcd (deprecated) | +#### Namespace Selector + +`kubernetes.namespace_selector` is replaced by the top-level `namespace_selector`. Entries written for 1.x keep their meaning: every entry must match, and the values given for the same key are ORed. Each entry also accepts the full Kubernetes label selector syntax, such as `env in (prod,staging)` or `!legacy`. The command line flag `--namespace-selector` is not available, set the option in the configuration file. + +It behaves differently from 1.x in the following ways: + +- When a namespace stops matching, 2.x removes the configuration of its resources from the data plane, while 1.x left the synced routes in place. Before upgrading, check for namespaces that were unlabeled in 1.x but still have routes in service, since those routes disappear after the upgrade. +- Only Ingress and `apisix.apache.org/v2` resources are filtered. Gateway API resources, which 1.x also filtered, are not; use the `allowedRoutes` of the Gateway listeners to limit their namespaces. + #### Example: Legacy Configuration Removed in 2.0.0 ```yaml diff --git a/internal/controller/apisixconsumer_controller.go b/internal/controller/apisixconsumer_controller.go index 39b3c638a..7e86a2289 100644 --- a/internal/controller/apisixconsumer_controller.go +++ b/internal/controller/apisixconsumer_controller.go @@ -132,7 +132,7 @@ func (r *ApisixConsumerReconciler) SetupWithManager(mgr ctrl.Manager) error { icWatch = &networkingv1.IngressClass{} } - return ctrl.NewControllerManagedBy(mgr). + bdr := ctrl.NewControllerManagedBy(mgr). For(&apiv2.ApisixConsumer{}, builder.WithPredicates( MatchesIngressClassPredicate(r.Client, r.Log, r.ICGV.String()), @@ -142,6 +142,7 @@ func (r *ApisixConsumerReconciler) SetupWithManager(mgr ctrl.Manager) error { predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.NewPredicateFuncs(TypePredicate[*corev1.Secret]()), + predicate.NewPredicateFuncs(TypePredicate[*corev1.Namespace]()), ), ). Watches( @@ -156,7 +157,8 @@ func (r *ApisixConsumerReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.listApisixConsumerForSecret), - ). + ) + return watchNamespaceSelector(bdr, r.Client, r.Log, func() client.ObjectList { return &apiv2.ApisixConsumerList{} }). Named("apisixconsumer"). Complete(r) } diff --git a/internal/controller/apisixglobalrule_controller.go b/internal/controller/apisixglobalrule_controller.go index d50d33677..7ebdefa72 100644 --- a/internal/controller/apisixglobalrule_controller.go +++ b/internal/controller/apisixglobalrule_controller.go @@ -95,6 +95,9 @@ func (r *ApisixGlobalRuleReconciler) Reconcile(ctx context.Context, req ctrl.Req r.Log.V(1).Info("no matching IngressClass available", "ingressClassName", globalRule.Spec.IngressClassName, "error", err.Error()) + if !isIngressClassSelectionAbsent(err) { + return ctrl.Result{}, err + } if err := r.Provider.Delete(ctx, &globalRule); err != nil { r.Log.Error(err, "failed to delete global rule from provider") return ctrl.Result{}, err @@ -159,7 +162,7 @@ func (r *ApisixGlobalRuleReconciler) SetupWithManager(mgr ctrl.Manager) error { } else { icWatch = &networkingv1.IngressClass{} } - return ctrl.NewControllerManagedBy(mgr). + bdr := ctrl.NewControllerManagedBy(mgr). For(&apiv2.ApisixGlobalRule{}, builder.WithPredicates( MatchesIngressClassPredicate(r.Client, r.Log, r.ICGV.String()), @@ -170,6 +173,7 @@ func (r *ApisixGlobalRuleReconciler) SetupWithManager(mgr ctrl.Manager) error { predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.NewPredicateFuncs(TypePredicate[*corev1.Secret]()), + predicate.NewPredicateFuncs(TypePredicate[*corev1.Namespace]()), ), ). Watches( @@ -184,7 +188,8 @@ func (r *ApisixGlobalRuleReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.listGlobalRulesForSecret), - ). + ) + return watchNamespaceSelector(bdr, r.Client, r.Log, func() client.ObjectList { return &apiv2.ApisixGlobalRuleList{} }). Named("apisixglobalrule"). Complete(r) } diff --git a/internal/controller/apisixroute_controller.go b/internal/controller/apisixroute_controller.go index fb9e1c3dd..ddd6fa782 100644 --- a/internal/controller/apisixroute_controller.go +++ b/internal/controller/apisixroute_controller.go @@ -88,6 +88,7 @@ func (r *ApisixRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.NewPredicateFuncs(TypePredicate[*corev1.Secret]()), + predicate.NewPredicateFuncs(TypePredicate[*corev1.Namespace]()), } if !r.supportsEndpointSlice { @@ -117,6 +118,7 @@ func (r *ApisixRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { r.listApisixRoutesForService, r.listApisixRoutesForEndpoints, r.Log) + bdr = watchNamespaceSelector(bdr, r.Client, r.Log, func() client.ObjectList { return &apiv2.ApisixRouteList{} }) return bdr. Watches(&corev1.Secret{}, @@ -163,6 +165,9 @@ func (r *ApisixRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) r.Log.V(1).Info("no matching IngressClass available", "ingressClassName", ar.Spec.IngressClassName, "error", err.Error()) + if !isIngressClassSelectionAbsent(err) { + return ctrl.Result{}, err + } if err := r.Provider.Delete(ctx, &ar); err != nil { r.Log.Error(err, "failed to delete apisixroute", "apisixroute", utils.NamespacedName(&ar)) return ctrl.Result{}, err diff --git a/internal/controller/apisixtls_controller.go b/internal/controller/apisixtls_controller.go index eeda370de..472a66d43 100644 --- a/internal/controller/apisixtls_controller.go +++ b/internal/controller/apisixtls_controller.go @@ -19,6 +19,7 @@ package controller import ( "context" + "errors" "fmt" "github.com/go-logr/logr" @@ -67,7 +68,7 @@ func (r *ApisixTlsReconciler) SetupWithManager(mgr ctrl.Manager) error { default: icWatch = &networkingv1.IngressClass{} } - return ctrl.NewControllerManagedBy(mgr). + bdr := ctrl.NewControllerManagedBy(mgr). For(&apiv2.ApisixTls{}, builder.WithPredicates( MatchesIngressClassPredicate(r.Client, r.Log, r.ICGV.String()), @@ -78,6 +79,7 @@ func (r *ApisixTlsReconciler) SetupWithManager(mgr ctrl.Manager) error { predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.NewPredicateFuncs(TypePredicate[*corev1.Secret]()), + predicate.NewPredicateFuncs(TypePredicate[*corev1.Namespace]()), ), ). Watches( @@ -92,7 +94,8 @@ func (r *ApisixTlsReconciler) SetupWithManager(mgr ctrl.Manager) error { ). Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.listApisixTlsForSecret), - ). + ) + return watchNamespaceSelector(bdr, r.Client, r.Log, func() client.ObjectList { return &apiv2.ApisixTlsList{} }). Complete(r) } @@ -131,6 +134,13 @@ func (r *ApisixTlsReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( r.Log.V(1).Info("no matching IngressClass available, skip processing", "ingressClassName", tls.Spec.IngressClassName, "error", err.Error()) + // Retract what was synced before the namespace stopped being watched. + if errors.Is(err, ErrNamespaceNotWatched) { + if err := r.Provider.Delete(ctx, &tls); err != nil { + r.Log.Error(err, "failed to delete TLS from provider") + return ctrl.Result{}, err + } + } return ctrl.Result{}, nil } diff --git a/internal/controller/config/config.go b/internal/controller/config/config.go index 0aa777d3a..e0829f8c2 100644 --- a/internal/controller/config/config.go +++ b/internal/controller/config/config.go @@ -27,6 +27,8 @@ import ( "time" "gopkg.in/yaml.v3" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" "github.com/apache/apisix-ingress-controller/internal/types" ) @@ -132,6 +134,10 @@ func (c *Config) Validate() error { } } + if _, err := ParseNamespaceSelector(c.NamespaceSelector); err != nil { + return err + } + if err := validateProvider(c.ProviderConfig); err != nil { return err } @@ -155,6 +161,52 @@ func validateProvider(config ProviderConfig) error { } } +// ParseNamespaceSelector combines the namespace_selector entries into one +// selector, keeping the semantics of 1.x: every entry must match, and entries +// holding a single equality or "in" requirement on the same key are merged, so +// ["team=a", "team=b"] selects "team in (a,b)". An entry with several +// requirements keeps the standard label selector semantics, so "team=a,team=b" +// matches nothing. Empty entries are ignored, as 1.x used [""] to disable the +// selector. It returns nil when no entry is left. +func ParseNamespaceSelector(entries []string) (labels.Selector, error) { + var ( + selector labels.Selector + keys []string + values = map[string][]string{} + ) + for _, entry := range entries { + if strings.TrimSpace(entry) == "" { + continue + } + reqs, err := labels.ParseToRequirements(entry) + if err != nil { + return nil, fmt.Errorf("invalid namespace_selector %q: %w", entry, err) + } + if selector == nil { + selector = labels.NewSelector() + } + if len(reqs) == 1 { + switch req := reqs[0]; req.Operator() { + case selection.Equals, selection.DoubleEquals, selection.In: + if _, ok := values[req.Key()]; !ok { + keys = append(keys, req.Key()) + } + values[req.Key()] = append(values[req.Key()], req.ValuesUnsorted()...) + continue + } + } + selector = selector.Add(reqs...) + } + for _, key := range keys { + req, err := labels.NewRequirement(key, selection.In, values[key]) + if err != nil { + return nil, fmt.Errorf("invalid namespace_selector on key %q: %w", key, err) + } + selector = selector.Add(*req) + } + return selector, nil +} + func GetControllerName() string { return ControllerConfig.ControllerName } diff --git a/internal/controller/config/config_test.go b/internal/controller/config/config_test.go index b31b10862..6a8b9e8bc 100644 --- a/internal/controller/config/config_test.go +++ b/internal/controller/config/config_test.go @@ -5,6 +5,9 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/utils/ptr" ) func TestNewDefaultConfig(t *testing.T) { @@ -65,12 +68,85 @@ func TestConfigValidateListenerPortMatchMode(t *testing.T) { } } +func TestConfigValidateNamespaceSelector(t *testing.T) { + tests := []struct { + name string + selector []string + expectErr bool + }{ + {name: "unset", selector: nil}, + {name: "1.x default", selector: []string{""}}, + {name: "equality", selector: []string{"team=a"}}, + {name: "set based", selector: []string{"env in (prod,staging),!legacy", "team=a"}}, + {name: "invalid", selector: []string{"team in a"}, expectErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := NewDefaultConfig() + cfg.NamespaceSelector = tt.selector + + err := cfg.Validate() + if tt.expectErr { + assert.ErrorContains(t, err, "invalid namespace_selector") + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestParseNamespaceSelector(t *testing.T) { + nsLabels := labels.Set{"version": "v1", "env": "prod"} + + tests := []struct { + name string + entries []string + // nil means the selector is disabled. + matches *bool + }{ + // Cases ported from TestMultiValueLabelsIsSubsetOf of 1.x. + {name: "no entry", entries: nil}, + {name: "1.x default", entries: []string{""}}, + {name: "single value", entries: []string{"env=prod"}, matches: ptr.To(true)}, + {name: "values on one key are ORed", entries: []string{"env=qa", "env=prod"}, matches: ptr.To(true)}, + {name: "value mismatch", entries: []string{"env=qa"}, matches: ptr.To(false)}, + {name: "missing key", entries: []string{"env3=not"}, matches: ptr.To(false)}, + // Entries on different keys are ANDed. + {name: "all keys match", entries: []string{"env=prod", "version=v1"}, matches: ptr.To(true)}, + {name: "one key mismatches", entries: []string{"env=prod", "version=v2"}, matches: ptr.To(false)}, + {name: "empty entry is ignored", entries: []string{"env=qa", ""}, matches: ptr.To(false)}, + // Full selector syntax on top of 1.x. + {name: "in merges with equality", entries: []string{"env in (qa)", "env==prod"}, matches: ptr.To(true)}, + {name: "not equal", entries: []string{"env=prod", "version!=v1"}, matches: ptr.To(false)}, + {name: "does not exist", entries: []string{"!legacy"}, matches: ptr.To(true)}, + // Only separate entries are merged, one entry keeps the standard semantics. + {name: "one entry is not merged", entries: []string{"env=qa,env=prod"}, matches: ptr.To(false)}, + {name: "one entry with several keys", entries: []string{"env=prod,version=v1"}, matches: ptr.To(true)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + selector, err := ParseNamespaceSelector(tt.entries) + require.NoError(t, err) + if tt.matches == nil { + assert.Nil(t, selector) + return + } + require.NotNil(t, selector) + assert.Equal(t, *tt.matches, selector.Matches(nsLabels), selector.String()) + }) + } +} + func TestNewConfigFromFile(t *testing.T) { // Create a temporary config file fileContent := ` log_level: debug controller_name: test-controller disable_gateway_api: true +namespace_selector: +- "team=a" ` tempFile, err := os.CreateTemp("", "config-*.yaml") assert.NoError(t, err) @@ -87,4 +163,5 @@ disable_gateway_api: true assert.Equal(t, "debug", cfg.LogLevel) assert.Equal(t, "test-controller", cfg.ControllerName) assert.Equal(t, true, cfg.DisableGatewayAPI) + assert.Equal(t, []string{"team=a"}, cfg.NamespaceSelector) } diff --git a/internal/controller/config/types.go b/internal/controller/config/types.go index d39af96bd..2b7c3de9e 100644 --- a/internal/controller/config/types.go +++ b/internal/controller/config/types.go @@ -79,6 +79,7 @@ type Config struct { Webhook *WebhookConfig `json:"webhook" yaml:"webhook"` DisableGatewayAPI bool `json:"disable_gateway_api" yaml:"disable_gateway_api"` ListenerPortMatchMode ListenerPortMatchMode `json:"listener_port_match_mode" yaml:"listener_port_match_mode"` + NamespaceSelector []string `json:"namespace_selector" yaml:"namespace_selector"` } type GatewayConfig struct { diff --git a/internal/controller/ingress_controller.go b/internal/controller/ingress_controller.go index 402f27704..6748fef6d 100644 --- a/internal/controller/ingress_controller.go +++ b/internal/controller/ingress_controller.go @@ -84,6 +84,7 @@ func (r *IngressReconciler) SetupWithManager(mgr ctrl.Manager) error { predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.NewPredicateFuncs(TypePredicate[*corev1.Secret]()), + predicate.NewPredicateFuncs(TypePredicate[*corev1.Namespace]()), } if !r.supportsEndpointSlice { @@ -110,6 +111,7 @@ func (r *IngressReconciler) SetupWithManager(mgr ctrl.Manager) error { r.listIngressesByService, r.listIngressesByEndpoints, r.Log) + bdr = watchNamespaceSelector(bdr, r.Client, r.Log, func() client.ObjectList { return &networkingv1.IngressList{} }) return bdr. Watches( @@ -177,6 +179,9 @@ func (r *IngressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct ingressClass, err := FindMatchingIngressClassByObject(tctx, r.Client, r.Log, ingress, "") if err != nil { + if !isIngressClassSelectionAbsent(err) { + return ctrl.Result{}, err + } if err := r.Provider.Delete(ctx, ingress); err != nil { r.Log.Error(err, "failed to delete ingress resources", "ingress", ingress.Name) return ctrl.Result{}, nil diff --git a/internal/controller/namespace_selector.go b/internal/controller/namespace_selector.go new file mode 100644 index 000000000..3536b6deb --- /dev/null +++ b/internal/controller/namespace_selector.go @@ -0,0 +1,137 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 controller + +import ( + "context" + "errors" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/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" + + "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +// ErrNamespaceNotWatched is returned for an object whose namespace does not +// match the configured namespace selector. +var ErrNamespaceNotWatched = errors.New("namespace is not watched by the namespace selector") + +var namespaceSelector labels.Selector + +// SetNamespaceSelector limits the IngressClass scoped resources (Ingress and +// apisix.apache.org/v2 resources) handled by the controller to the namespaces +// matching the namespace_selector entries, see config.ParseNamespaceSelector. +// Without an entry every namespace is watched. +func SetNamespaceSelector(entries []string) error { + selector, err := config.ParseNamespaceSelector(entries) + if err != nil { + return err + } + namespaceSelector = selector + return nil +} + +func namespaceSelectorEnabled() bool { + return namespaceSelector != nil +} + +func namespaceLabelsMatch(nsLabels map[string]string) bool { + return !namespaceSelectorEnabled() || namespaceSelector.Matches(labels.Set(nsLabels)) +} + +// IsWatchedNamespace reports whether objects in the namespace are handled by +// the controller under the configured namespace selector. +func IsWatchedNamespace(ctx context.Context, c client.Client, namespace string) (bool, error) { + if !namespaceSelectorEnabled() || namespace == "" { + return true, nil + } + var ns corev1.Namespace + if err := c.Get(ctx, client.ObjectKey{Name: namespace}, &ns); err != nil { + if k8serrors.IsNotFound(err) { + return false, nil + } + return false, err + } + return namespaceLabelsMatch(ns.Labels), nil +} + +func checkWatchedNamespace(ctx context.Context, c client.Client, obj client.Object) error { + watched, err := IsWatchedNamespace(ctx, c, obj.GetNamespace()) + if err != nil { + return err + } + if !watched { + return ErrNamespaceNotWatched + } + return nil +} + +// namespaceSelectorChangedPredicate passes a Namespace update only when the +// namespace moves into or out of the watched set. A new namespace holds no +// objects yet, and the objects of a deleted namespace are deleted one by one. +func namespaceSelectorChangedPredicate() predicate.Funcs { + return predicate.Funcs{ + CreateFunc: func(event.CreateEvent) bool { return false }, + DeleteFunc: func(event.DeleteEvent) bool { return false }, + GenericFunc: func(event.GenericEvent) bool { return false }, + UpdateFunc: func(e event.UpdateEvent) bool { + return namespaceLabelsMatch(e.ObjectOld.GetLabels()) != namespaceLabelsMatch(e.ObjectNew.GetLabels()) + }, + } +} + +// watchNamespaceSelector requeues every object listed by newList in a namespace +// whose labels start or stop matching the namespace selector, so that the +// objects are synced or retracted accordingly. The event filter of the +// controller must let Namespace events through. +func watchNamespaceSelector(bdr *builder.Builder, c client.Client, log logr.Logger, newList func() client.ObjectList) *builder.Builder { + if !namespaceSelectorEnabled() { + return bdr + } + return bdr.Watches(&corev1.Namespace{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []reconcile.Request { + list := newList() + if err := c.List(ctx, list, client.InNamespace(obj.GetName())); err != nil { + log.Error(err, "failed to list objects for namespace", "namespace", obj.GetName()) + return nil + } + var requests []reconcile.Request + if err := meta.EachListItem(list, func(item runtime.Object) error { + if o, ok := item.(client.Object); ok { + requests = append(requests, reconcile.Request{NamespacedName: utils.NamespacedName(o)}) + } + return nil + }); err != nil { + log.Error(err, "failed to iterate objects for namespace", "namespace", obj.GetName()) + } + return requests + }), + builder.WithPredicates(namespaceSelectorChangedPredicate()), + ) +} diff --git a/internal/controller/namespace_selector_test.go b/internal/controller/namespace_selector_test.go new file mode 100644 index 000000000..31e8db857 --- /dev/null +++ b/internal/controller/namespace_selector_test.go @@ -0,0 +1,207 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 controller + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8stypes "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/event" + + apiv2 "github.com/apache/apisix-ingress-controller/api/v2" +) + +const ( + watchedNamespace = "watched" + unwatchedNamespace = "unwatched" +) + +func setNamespaceSelector(t *testing.T, entries ...string) { + t.Helper() + require.NoError(t, SetNamespaceSelector(entries)) + t.Cleanup(func() { namespaceSelector = nil }) +} + +func selectorNamespaces() []client.Object { + return []client.Object{ + &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{ + Name: watchedNamespace, + Labels: map[string]string{"team": "a"}, + }}, + &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{ + Name: unwatchedNamespace, + Labels: map[string]string{"team": "b"}, + }}, + } +} + +func TestSetNamespaceSelector(t *testing.T) { + t.Cleanup(func() { namespaceSelector = nil }) + + require.Error(t, SetNamespaceSelector([]string{"team in a"})) + + require.NoError(t, SetNamespaceSelector([]string{""})) + assert.False(t, namespaceSelectorEnabled(), "[\"\"] disables the selector as in 1.x") + assert.True(t, namespaceLabelsMatch(nil)) + + require.NoError(t, SetNamespaceSelector([]string{"team=a", "team=b", "env=prod"})) + assert.True(t, namespaceSelectorEnabled()) + assert.True(t, namespaceLabelsMatch(map[string]string{"team": "a", "env": "prod"})) + assert.True(t, namespaceLabelsMatch(map[string]string{"team": "b", "env": "prod"})) + assert.False(t, namespaceLabelsMatch(map[string]string{"team": "a"}), "entries on different keys are ANDed") + assert.False(t, namespaceLabelsMatch(map[string]string{"team": "c", "env": "prod"})) + assert.False(t, namespaceLabelsMatch(nil)) +} + +func TestIsWatchedNamespace(t *testing.T) { + cli := fake.NewClientBuilder().WithScheme(retractPluginConfigScheme(t)). + WithObjects(selectorNamespaces()...).Build() + ctx := context.Background() + + watched, err := IsWatchedNamespace(ctx, cli, unwatchedNamespace) + require.NoError(t, err) + assert.True(t, watched, "every namespace is watched without a selector") + + setNamespaceSelector(t, "team=a") + + for ns, want := range map[string]bool{ + watchedNamespace: true, + unwatchedNamespace: false, + "missing": false, + "": true, + } { + watched, err := IsWatchedNamespace(ctx, cli, ns) + require.NoError(t, err, ns) + assert.Equal(t, want, watched, ns) + } +} + +func TestFindMatchingIngressClassByObject_NamespaceSelector(t *testing.T) { + cli := fake.NewClientBuilder().WithScheme(retractPluginConfigScheme(t)). + WithObjects(append(selectorNamespaces(), retractIngressClass())...).Build() + setNamespaceSelector(t, "team=a") + + route := func(ns string) *apiv2.ApisixRoute { + return &apiv2.ApisixRoute{ + ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: "route"}, + Spec: apiv2.ApisixRouteSpec{IngressClassName: "apisix"}, + } + } + + ic, err := FindMatchingIngressClassByObject(context.Background(), cli, logr.Discard(), route(watchedNamespace), "") + require.NoError(t, err) + assert.Equal(t, "apisix", ic.Name) + + _, err = FindMatchingIngressClassByObject(context.Background(), cli, logr.Discard(), route(unwatchedNamespace), "") + require.ErrorIs(t, err, ErrNamespaceNotWatched) + assert.True(t, isIngressClassSelectionAbsent(err)) + assert.False(t, MatchesIngressClass(cli, logr.Discard(), route(unwatchedNamespace), "")) +} + +func TestNamespaceSelectorChangedPredicate(t *testing.T) { + setNamespaceSelector(t, "team=a") + pred := namespaceSelectorChangedPredicate() + + ns := func(labels map[string]string) *corev1.Namespace { + return &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns", Labels: labels}} + } + watching := map[string]string{"team": "a"} + + assert.True(t, pred.Update(event.UpdateEvent{ObjectOld: ns(nil), ObjectNew: ns(watching)})) + assert.True(t, pred.Update(event.UpdateEvent{ObjectOld: ns(watching), ObjectNew: ns(nil)})) + assert.False(t, pred.Update(event.UpdateEvent{ + ObjectOld: ns(watching), + ObjectNew: ns(map[string]string{"team": "a", "other": "x"}), + }), "a label change that keeps the match result must not requeue") + assert.False(t, pred.Create(event.CreateEvent{Object: ns(watching)})) + assert.False(t, pred.Delete(event.DeleteEvent{Object: ns(watching)})) +} + +// An ApisixRoute in a namespace that stops matching the selector must be +// retracted, just like one whose IngressClass is no longer ours. +func TestApisixRouteReconcile_RetractsOutsideWatchedNamespace(t *testing.T) { + scheme := retractPluginConfigScheme(t) + route := &apiv2.ApisixRoute{ + ObjectMeta: metav1.ObjectMeta{Namespace: unwatchedNamespace, Name: "route"}, + Spec: apiv2.ApisixRouteSpec{IngressClassName: "apisix"}, + } + cli := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(append(selectorNamespaces(), retractIngressClass(), route)...). + WithStatusSubresource(route). + Build() + setNamespaceSelector(t, "team=a") + + prov := &pluginConfigProvider{} + updater := &pluginConfigUpdater{} + r := &ApisixRouteReconciler{ + Client: cli, + Scheme: scheme, + Log: logr.Discard(), + Provider: prov, + Updater: updater, + Readier: newRetractReadier(t, cli), + } + + key := k8stypes.NamespacedName{Namespace: unwatchedNamespace, Name: "route"} + result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: key}) + + require.NoError(t, err) + assert.Equal(t, ctrl.Result{}, result) + assert.Equal(t, []k8stypes.NamespacedName{key}, prov.deleted) + assert.Zero(t, prov.updated) + assert.Empty(t, updater.updates, "the status of an unwatched object belongs to another controller") +} + +func TestApisixTlsReconcile_RetractsOutsideWatchedNamespace(t *testing.T) { + scheme := retractPluginConfigScheme(t) + tls := &apiv2.ApisixTls{ + ObjectMeta: metav1.ObjectMeta{Namespace: unwatchedNamespace, Name: "tls"}, + Spec: apiv2.ApisixTlsSpec{IngressClassName: "apisix"}, + } + cli := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(append(selectorNamespaces(), retractIngressClass(), tls)...). + WithStatusSubresource(tls). + Build() + setNamespaceSelector(t, "team=a") + + prov := &pluginConfigProvider{} + r := &ApisixTlsReconciler{ + Client: cli, + Scheme: scheme, + Log: logr.Discard(), + Provider: prov, + Updater: &pluginConfigUpdater{}, + Readier: newRetractReadier(t, cli), + } + + key := k8stypes.NamespacedName{Namespace: unwatchedNamespace, Name: "tls"} + _, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: key}) + + require.NoError(t, err) + assert.Equal(t, []k8stypes.NamespacedName{key}, prov.deleted) + assert.Zero(t, prov.updated) +} diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 3b7e31fdc..13550995c 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -1927,10 +1927,16 @@ func GetIngressClassV1(ctx context.Context, c client.Client, log logr.Logger, in func isIngressClassSelectionAbsent(err error) bool { return k8serrors.IsNotFound(err) || errors.Is(err, errNoDefaultIngressClass) || - errors.Is(err, errIngressClassNotControlled) + errors.Is(err, errIngressClassNotControlled) || + errors.Is(err, ErrNamespaceNotWatched) } func FindMatchingIngressClassByObject(ctx context.Context, c client.Client, log logr.Logger, obj client.Object, apiVersion string) (*networkingv1.IngressClass, error) { + // An object outside the watched namespaces is not ours, just like one bound + // to an IngressClass of another controller. + if err := checkWatchedNamespace(ctx, c, obj); err != nil { + return nil, err + } ingressClassName := ExtractIngressClass(obj) switch apiVersion { case networkingv1beta1.SchemeGroupVersion.String(): diff --git a/internal/manager/controllers.go b/internal/manager/controllers.go index 134eb96ea..1a42d8d6f 100644 --- a/internal/manager/controllers.go +++ b/internal/manager/controllers.go @@ -349,6 +349,16 @@ func registerV2ForReadinessGVK(mgr manager.Manager, readier readiness.ReadinessM readier.RegisterGVK(readiness.GVKConfig{ GVKs: gvks, Filter: readiness.GVKFilter(func(obj *unstructured.Unstructured) bool { + watched, err := controller.IsWatchedNamespace(context.Background(), c, obj.GetNamespace()) + if err != nil { + // Keep waiting for the object rather than skipping a selected one. If + // the lookup keeps failing, readiness falls back to its timeout. + log.Error(err, "failed to evaluate namespace selector", "namespace", obj.GetNamespace()) + return true + } + if !watched { + return false + } icName, _, _ := unstructured.NestedString(obj.Object, "spec", "ingressClassName") ingressClass, _ := controller.FindMatchingIngressClassByName(context.Background(), c, log, icName, icgv.String()) return ingressClass != nil diff --git a/internal/webhook/v1/ssl/conflict_detector.go b/internal/webhook/v1/ssl/conflict_detector.go index fe5983ea2..9db0a115c 100644 --- a/internal/webhook/v1/ssl/conflict_detector.go +++ b/internal/webhook/v1/ssl/conflict_detector.go @@ -17,6 +17,7 @@ package ssl import ( "context" + "errors" "fmt" "sort" "strings" @@ -303,6 +304,10 @@ func (d *ConflictDetector) resolveGatewayProxy(ctx context.Context, obj client.O return controller.GetGatewayProxyByGateway(ctx, d.client, resource) case *networkingv1.Ingress: ingressClass, err := controller.FindMatchingIngressClassByObject(ctx, d.client, logger, resource, "") + if errors.Is(err, controller.ErrNamespaceNotWatched) { + // Handled by another controller, so it cannot conflict. + return nil, nil + } if err != nil { return nil, err } @@ -312,6 +317,10 @@ func (d *ConflictDetector) resolveGatewayProxy(ctx context.Context, obj client.O return controller.GetGatewayProxyByIngressClass(ctx, d.client, ingressClass) case *apiv2.ApisixTls: ingressClass, err := controller.FindMatchingIngressClassByObject(ctx, d.client, logger, resource, "") + if errors.Is(err, controller.ErrNamespaceNotWatched) { + // Handled by another controller, so it cannot conflict. + return nil, nil + } if err != nil { return nil, err } diff --git a/test/e2e/crds/v2/namespace_selector.go b/test/e2e/crds/v2/namespace_selector.go new file mode 100644 index 000000000..f4e7c6f90 --- /dev/null +++ b/test/e2e/crds/v2/namespace_selector.go @@ -0,0 +1,143 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 v2 + +import ( + "fmt" + "net/http" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" +) + +var _ = Describe("Test Namespace Selector", Label("apisix.apache.org", "v2", "apisixroute"), func() { + // Entries on different keys are ANDed, a namespace needs both labels. + const ( + teamLabel = "apisix.apache.org/e2e-namespace-team" + envLabel = "apisix.apache.org/e2e-namespace-env" + ) + + var ( + s = scaffold.NewScaffold(scaffold.Options{ + NamespaceSelector: []string{teamLabel + "=a", envLabel + "=prod"}, + }) + otherNamespace string + ) + + const ( + externalServiceSpec = ` +apiVersion: v1 +kind: Service +metadata: + name: httpbin-external +spec: + type: ExternalName + externalName: httpbin-service-e2e-test.%s.svc +` + apisixRouteSpec = ` +apiVersion: apisix.apache.org/v2 +kind: ApisixRoute +metadata: + name: default + namespace: %s +spec: + ingressClassName: %s + http: + - name: rule0 + match: + hosts: + - %s + paths: + - /get + backends: + - serviceName: httpbin-external + servicePort: 80 +` + ) + + labelNamespace := func(ns string, labels ...string) { + args := append([]string{"label", "namespace", ns, "--overwrite"}, labels...) + _, err := s.RunKubectlAndGetOutput(args...) + Expect(err).NotTo(HaveOccurred(), "labeling namespace %s", ns) + } + selectNamespace := func(ns string) { labelNamespace(ns, teamLabel+"=a", envLabel+"=prod") } + + request := func(host string) int { + return s.NewAPISIXClient().GET("/get").WithHost(host).Expect().Raw().StatusCode + } + + BeforeEach(func() { + By("create GatewayProxy") + Expect(s.CreateResourceFromString(s.GetGatewayProxySpec())).NotTo(HaveOccurred(), "creating GatewayProxy") + + By("create IngressClass") + err := s.CreateResourceFromStringWithNamespace(s.GetIngressClassYaml(), "") + Expect(err).NotTo(HaveOccurred(), "creating IngressClass") + + otherNamespace = s.Namespace() + "-other" + s.CreateNamespace(otherNamespace) + selectNamespace(s.Namespace()) + + for _, ns := range []string{s.Namespace(), otherNamespace} { + err := s.CreateResourceFromStringWithNamespace(fmt.Sprintf(externalServiceSpec, s.Namespace()), ns) + Expect(err).NotTo(HaveOccurred(), "creating ExternalName Service in %s", ns) + } + }) + + AfterEach(func() { + s.DeleteNamespace(otherNamespace) + }) + + It("syncs only the resources in the selected namespaces", func() { + By("create an ApisixRoute in the selected and in the unselected namespace") + for ns, host := range map[string]string{s.Namespace(): "watched", otherNamespace: "unwatched"} { + err := s.CreateResourceFromStringWithNamespace(fmt.Sprintf(apisixRouteSpec, ns, s.Namespace(), host), ns) + Expect(err).NotTo(HaveOccurred(), "creating ApisixRoute in %s", ns) + } + + Eventually(request).WithArguments("watched").WithTimeout(30 * time.Second).ProbeEvery(time.Second). + Should(Equal(http.StatusOK)) + Consistently(request).WithArguments("unwatched").WithTimeout(10 * time.Second).ProbeEvery(time.Second). + Should(Equal(http.StatusNotFound)) + + By("label the other namespace with only one of the selected labels") + labelNamespace(otherNamespace, teamLabel+"=a") + Consistently(request).WithArguments("unwatched").WithTimeout(10 * time.Second).ProbeEvery(time.Second). + Should(Equal(http.StatusNotFound)) + + By("select the other namespace") + selectNamespace(otherNamespace) + Eventually(request).WithArguments("unwatched").WithTimeout(30 * time.Second).ProbeEvery(time.Second). + Should(Equal(http.StatusOK)) + + By("unselect the namespace, its configuration is retracted") + labelNamespace(s.Namespace(), envLabel+"-") + Eventually(request).WithArguments("watched").WithTimeout(30 * time.Second).ProbeEvery(time.Second). + Should(Equal(http.StatusNotFound)) + Consistently(request).WithArguments("unwatched").WithTimeout(5 * time.Second).ProbeEvery(time.Second). + Should(Equal(http.StatusOK)) + + By("select the namespace again") + selectNamespace(s.Namespace()) + Eventually(request).WithArguments("watched").WithTimeout(30 * time.Second).ProbeEvery(time.Second). + Should(Equal(http.StatusOK)) + }) +}) diff --git a/test/e2e/framework/ingress.go b/test/e2e/framework/ingress.go index 9c2e71998..8b85192cd 100644 --- a/test/e2e/framework/ingress.go +++ b/test/e2e/framework/ingress.go @@ -53,6 +53,7 @@ type IngressDeployOpts struct { InitSyncDelay time.Duration WebhookEnable bool WebhookPort int + NamespaceSelector []string } func (f *Framework) DeployIngress(opts IngressDeployOpts) { diff --git a/test/e2e/framework/manifests/ingress.yaml b/test/e2e/framework/manifests/ingress.yaml index a4e244a4c..5e7273705 100644 --- a/test/e2e/framework/manifests/ingress.yaml +++ b/test/e2e/framework/manifests/ingress.yaml @@ -308,6 +308,12 @@ data: # The default value is 0 seconds, which means the controller will not sync. # If you want to enable the sync, set it to a positive value. init_sync_delay: {{ .InitSyncDelay | default "20m" }} + {{- if .NamespaceSelector }} + namespace_selector: + {{- range .NamespaceSelector }} + - {{ . | quote }} + {{- end }} + {{- end }} webhook: enable: {{ .WebhookEnable | default false }} port: {{ .WebhookPort | default 9443 }} diff --git a/test/e2e/scaffold/api7_deployer.go b/test/e2e/scaffold/api7_deployer.go index 7744372a8..b1d3dbc2e 100644 --- a/test/e2e/scaffold/api7_deployer.go +++ b/test/e2e/scaffold/api7_deployer.go @@ -197,6 +197,7 @@ func (s *API7Deployer) DeployIngress() { ControllerName: s.runtimeOpts.ControllerName, ProviderSyncPeriod: 1 * time.Hour, Namespace: s.namespace, + NamespaceSelector: s.runtimeOpts.NamespaceSelector, Replicas: ptr.To(1), WebhookEnable: s.runtimeOpts.EnableWebhook, }) @@ -208,6 +209,7 @@ func (s *API7Deployer) ScaleIngress(replicas int) { ControllerName: s.runtimeOpts.ControllerName, ProviderSyncPeriod: 1 * time.Hour, Namespace: s.namespace, + NamespaceSelector: s.runtimeOpts.NamespaceSelector, Replicas: ptr.To(replicas), }) } diff --git a/test/e2e/scaffold/apisix_deployer.go b/test/e2e/scaffold/apisix_deployer.go index aa6d8fad3..1edc65367 100644 --- a/test/e2e/scaffold/apisix_deployer.go +++ b/test/e2e/scaffold/apisix_deployer.go @@ -313,6 +313,7 @@ func (s *APISIXDeployer) DeployIngress() { ProviderType: framework.ProviderType, ProviderSyncPeriod: syncPeriod, Namespace: s.namespace, + NamespaceSelector: s.runtimeOpts.NamespaceSelector, Replicas: ptr.To(1), WebhookEnable: s.runtimeOpts.EnableWebhook, }) @@ -328,6 +329,7 @@ func (s *APISIXDeployer) ScaleIngress(replicas int) { ProviderType: framework.ProviderType, ProviderSyncPeriod: syncPeriod, Namespace: s.namespace, + NamespaceSelector: s.runtimeOpts.NamespaceSelector, Replicas: &replicas, }) } diff --git a/test/e2e/scaffold/apisix_prewarm.go b/test/e2e/scaffold/apisix_prewarm.go index 030917365..af913b640 100644 --- a/test/e2e/scaffold/apisix_prewarm.go +++ b/test/e2e/scaffold/apisix_prewarm.go @@ -75,7 +75,8 @@ func isPoolable(o Options) bool { return !o.SkipHooks && !o.EnableWebhook && o.ControllerName == "" && - o.APISIXAdminAPIKey == "" + o.APISIXAdminAPIKey == "" && + len(o.NamespaceSelector) == 0 } // profileKey identifies the pool an environment belongs to. Within a process diff --git a/test/e2e/scaffold/scaffold.go b/test/e2e/scaffold/scaffold.go index 41ac85c74..db765d8f8 100644 --- a/test/e2e/scaffold/scaffold.go +++ b/test/e2e/scaffold/scaffold.go @@ -58,6 +58,10 @@ type Options struct { SkipHooks bool EnableWebhook bool + + // NamespaceSelector is rendered into the namespace_selector of the + // controller configuration. + NamespaceSelector []string } type Scaffold struct {