diff --git a/go.mod b/go.mod index 114977559bf9..0d1fb80239e7 100644 --- a/go.mod +++ b/go.mod @@ -235,3 +235,6 @@ require ( sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.1 // indirect ) + +// Local development: Use local istio/api for exportToSelectors feature +replace istio.io/api => ../istio-api diff --git a/pilot/pkg/bootstrap/server.go b/pilot/pkg/bootstrap/server.go index ff3f1c4abe3a..07485f225480 100644 --- a/pilot/pkg/bootstrap/server.go +++ b/pilot/pkg/bootstrap/server.go @@ -52,6 +52,7 @@ import ( sec_model "istio.io/istio/pilot/pkg/security/model" "istio.io/istio/pilot/pkg/server" "istio.io/istio/pilot/pkg/serviceregistry/aggregate" + kubecontroller "istio.io/istio/pilot/pkg/serviceregistry/kube/controller" "istio.io/istio/pilot/pkg/serviceregistry/provider" "istio.io/istio/pilot/pkg/serviceregistry/serviceentry" "istio.io/istio/pilot/pkg/status" @@ -293,6 +294,9 @@ func NewServer(args *PilotArgs, initFuncs ...func(*Server)) (*Server, error) { namespaces := kclient.New[*corev1.Namespace](s.kubeClient) filter := namespace.NewDiscoveryNamespacesFilter(namespaces, s.environment.Watcher, s.internalStop) s.kubeClient = kubelib.SetObjectFilter(s.kubeClient, filter) + + // Set up namespace labels getter for dynamic exportTo visibility based on label selectors + s.environment.NamespaceLabelsGetter = kubecontroller.NewKubeNamespaceLabelsGetter(namespaces) } s.initMeshNetworks(args, s.fileWatcher) diff --git a/pilot/pkg/model/context.go b/pilot/pkg/model/context.go index db3e5de34974..69c3a2de7c5d 100644 --- a/pilot/pkg/model/context.go +++ b/pilot/pkg/model/context.go @@ -148,6 +148,17 @@ type Environment struct { // Cache for XDS resources. Cache XdsCache + + // NamespaceLabelsGetter provides a way to look up namespace labels for dynamic exportTo visibility checks + NamespaceLabelsGetter NamespaceLabelsGetter +} + +// NamespaceLabelsGetter is an interface for looking up namespace labels. +// This is used to support dynamic exportTo visibility based on namespace label selectors. +type NamespaceLabelsGetter interface { + // GetNamespaceLabels returns the labels for a given namespace. + // Returns nil if the namespace is not found or labels cannot be retrieved. + GetNamespaceLabels(namespace string) map[string]string } func (e *Environment) Mesh() *meshconfig.MeshConfig { @@ -232,6 +243,15 @@ func (e *Environment) ClusterLocal() ClusterLocalProvider { return e.clusterLocalServices } +// GetNamespaceLabels returns the labels for a given namespace. +// Returns nil if the namespace is not found or if no NamespaceLabelsGetter is configured. +func (e *Environment) GetNamespaceLabels(namespace string) map[string]string { + if e == nil || e.NamespaceLabelsGetter == nil { + return nil + } + return e.NamespaceLabelsGetter.GetNamespaceLabels(namespace) +} + func (e *Environment) GetProxyConfigOrDefault(ns string, labels, annotations map[string]string, meshConfig *meshconfig.MeshConfig) *meshconfig.ProxyConfig { push := e.PushContext() if push != nil && push.ProxyConfigs != nil { diff --git a/pilot/pkg/model/destination_rule.go b/pilot/pkg/model/destination_rule.go index fb82cd438964..0de8399eea2c 100644 --- a/pilot/pkg/model/destination_rule.go +++ b/pilot/pkg/model/destination_rule.go @@ -24,7 +24,6 @@ import ( "istio.io/istio/pkg/config" "istio.io/istio/pkg/config/host" "istio.io/istio/pkg/config/labels" - "istio.io/istio/pkg/config/visibility" "istio.io/istio/pkg/util/sets" ) @@ -38,7 +37,7 @@ import ( // 2. If the original rule did not have any top level traffic policy, traffic policies from the new rule will be // used. // 3. If the original rule did not have any exportTo, exportTo settings from the new rule will be used. -func (ps *PushContext) mergeDestinationRule(p *consolidatedDestRules, destRuleConfig config.Config, exportToSet sets.Set[visibility.Instance]) { +func (ps *PushContext) mergeDestinationRule(p *consolidatedDestRules, destRuleConfig config.Config, exportToSet *ExportToTarget) { rule := destRuleConfig.Spec.(*networking.DestinationRule) resolvedHost := host.Name(rule.Host) @@ -57,7 +56,7 @@ func (ps *PushContext) mergeDestinationRule(p *consolidatedDestRules, destRuleCo if features.EnableEnhancedDestinationRuleMerge { if exportToSet.Equals(mdr.exportTo) { appendSeparately = false - } else if len(mdr.exportTo) > 0 && exportToSet.SupersetOf(mdr.exportTo) { + } else if mdr.exportTo.Len() > 0 && exportToSet.IsSuperset(mdr.exportTo) { // If the new exportTo is superset of existing, merge and also append as a standalone one appendSeparately = true } else { @@ -126,7 +125,7 @@ func (ps *PushContext) mergeDestinationRule(p *consolidatedDestRules, destRuleCo destRules[resolvedHost] = append(destRules[resolvedHost], ConvertConsolidatedDestRule(&destRuleConfig, exportToSet)) } -func ConvertConsolidatedDestRule(cfg *config.Config, exportToSet sets.Set[visibility.Instance]) *ConsolidatedDestRule { +func ConvertConsolidatedDestRule(cfg *config.Config, exportToSet *ExportToTarget) *ConsolidatedDestRule { return &ConsolidatedDestRule{ exportTo: exportToSet, rule: cfg, diff --git a/pilot/pkg/model/exportto.go b/pilot/pkg/model/exportto.go new file mode 100644 index 000000000000..17f0ffff5371 --- /dev/null +++ b/pilot/pkg/model/exportto.go @@ -0,0 +1,275 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package model + +import ( + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + + typev1beta1 "istio.io/api/type/v1beta1" + "istio.io/istio/pkg/config/visibility" + "istio.io/istio/pkg/util/sets" +) + +// ExportToTarget represents the visibility configuration for a service or config object. +// It can contain both static namespace names (including visibility constants like ".", "*", "~") +// and label selectors for dynamic namespace matching. +type ExportToTarget struct { + // StaticNamespaces contains explicit namespace names and visibility constants (., *, ~) + StaticNamespaces sets.Set[visibility.Instance] + + // Selectors contains label selectors for dynamic namespace matching + Selectors []labels.Selector +} + +// IsEmpty returns true if the ExportToTarget has no static namespaces and no selectors +func (e *ExportToTarget) IsEmpty() bool { + if e == nil { + return true + } + return len(e.StaticNamespaces) == 0 && len(e.Selectors) == 0 +} + +// IsPublic returns true if the ExportToTarget contains the public visibility constant "*" +func (e *ExportToTarget) IsPublic() bool { + if e == nil { + return false + } + return e.StaticNamespaces.Contains(visibility.Public) +} + +// IsPrivate returns true if the ExportToTarget contains the private visibility constant "." +func (e *ExportToTarget) IsPrivate() bool { + if e == nil { + return false + } + return e.StaticNamespaces.Contains(visibility.Private) +} + +// IsNone returns true if the ExportToTarget contains the none visibility constant "~" +func (e *ExportToTarget) IsNone() bool { + if e == nil { + return false + } + return e.StaticNamespaces.Contains(visibility.None) +} + +// Matches evaluates if a namespace matches the export target based on: +// 1. Static namespace names (including visibility constants) +// 2. Label selectors against the namespace labels +// +// Parameters: +// - namespace: the namespace name to check +// - namespaceLabels: the labels of the namespace +// +// Returns true if the namespace matches any of the export targets +func (e *ExportToTarget) Matches(namespace string, namespaceLabels map[string]string) bool { + if e == nil { + return false + } + + // Check if public visibility is enabled + if e.StaticNamespaces.Contains(visibility.Public) { + return true + } + + // Check if the specific namespace is in the static list + if e.StaticNamespaces.Contains(visibility.Instance(namespace)) { + return true + } + + // Check label selectors + if len(e.Selectors) > 0 && namespaceLabels != nil { + labelSet := labels.Set(namespaceLabels) + for _, selector := range e.Selectors { + if selector.Matches(labelSet) { + return true + } + } + } + + return false +} + +// ParseExportTo converts a list of exportTo strings and label selectors to an ExportToTarget. +// This function is used to convert API types to internal types. +// +// Parameters: +// - exportToList: list of static namespace names or visibility constants +// - exportToSelectors: list of label selectors from the API +// +// Returns an ExportToTarget with both static namespaces and selectors populated +func ParseExportTo(exportToList []string, exportToSelectors []*typev1beta1.LabelSelector) (*ExportToTarget, error) { + target := &ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{}, + } + + // Parse static namespace names and visibility constants + for _, ns := range exportToList { + target.StaticNamespaces.Insert(visibility.Instance(ns)) + } + + // Parse label selectors + for _, ls := range exportToSelectors { + selector, err := LabelSelectorAsSelector(ls) + if err != nil { + return nil, fmt.Errorf("invalid label selector: %v", err) + } + target.Selectors = append(target.Selectors, selector) + } + + return target, nil +} + +// Contains checks if the given visibility instance is in the static namespaces set. +// This provides backward compatibility with code that used sets.Set[visibility.Instance].Contains(). +func (e *ExportToTarget) Contains(v visibility.Instance) bool { + if e == nil { + return false + } + return e.StaticNamespaces.Contains(v) +} + +// Copy creates a deep copy of ExportToTarget. +// Note: labels.Selector is typically immutable once created, so we can share the selector references. +func (e *ExportToTarget) Copy() *ExportToTarget { + if e == nil { + return nil + } + out := &ExportToTarget{} + + // Preserve nil for StaticNamespaces + if e.StaticNamespaces != nil { + out.StaticNamespaces = e.StaticNamespaces.Copy() + } + + // Preserve nil for Selectors + if e.Selectors != nil { + out.Selectors = make([]labels.Selector, len(e.Selectors)) + copy(out.Selectors, e.Selectors) + } + + return out +} + +// Equals checks if two ExportToTarget objects are equal. +// Note: Label selectors are compared by their string representation. +func (e *ExportToTarget) Equals(other *ExportToTarget) bool { + if e == nil && other == nil { + return true + } + if e == nil || other == nil { + return false + } + // Compare static namespaces + if !e.StaticNamespaces.Equals(other.StaticNamespaces) { + return false + } + // Compare selectors by count and string representation + if len(e.Selectors) != len(other.Selectors) { + return false + } + // Compare each selector's string representation + // Note: Order matters for this comparison + for i, sel := range e.Selectors { + if sel.String() != other.Selectors[i].String() { + return false + } + } + return true +} + +// StaticNamespacesList returns the static namespaces as a list for iteration. +// This is useful for code that needs to iterate over the namespaces. +func (e *ExportToTarget) StaticNamespacesList() []visibility.Instance { + if e == nil { + return nil + } + return e.StaticNamespaces.UnsortedList() +} + +// HasSelectors returns true if there are any label selectors defined. +func (e *ExportToTarget) HasSelectors() bool { + return e != nil && len(e.Selectors) > 0 +} + +// IsSuperset checks if this ExportToTarget is a superset of another. +// For static namespaces, checks set containment. +// If either has selectors, this is conservatively false (cannot determine superset relationship with dynamic selectors). +func (e *ExportToTarget) IsSuperset(other *ExportToTarget) bool { + if e == nil || other == nil { + return false + } + // If either has selectors, we cannot determine superset relationship + if e.HasSelectors() || other.HasSelectors() { + return false + } + // Check if all namespaces in 'other' are in 'e' + return e.StaticNamespaces.SupersetOf(other.StaticNamespaces) +} + +// Len returns the number of static namespaces (for backward compatibility). +func (e *ExportToTarget) Len() int { + if e == nil { + return 0 + } + return len(e.StaticNamespaces) +} + +// LabelSelectorAsSelector converts a type v1beta1 LabelSelector to a labels.Selector. +// This follows the same pattern used in the ambient controller. +func LabelSelectorAsSelector(ps *typev1beta1.LabelSelector) (labels.Selector, error) { + if ps == nil { + return labels.Nothing(), nil + } + if len(ps.MatchLabels)+len(ps.MatchExpressions) == 0 { + return labels.Everything(), nil + } + requirements := make([]labels.Requirement, 0, len(ps.MatchLabels)+len(ps.MatchExpressions)) + for k, v := range ps.MatchLabels { + r, err := labels.NewRequirement(k, selection.Equals, []string{v}) + if err != nil { + return nil, err + } + requirements = append(requirements, *r) + } + for _, expr := range ps.MatchExpressions { + var op selection.Operator + switch metav1.LabelSelectorOperator(expr.Operator) { + case metav1.LabelSelectorOpIn: + op = selection.In + case metav1.LabelSelectorOpNotIn: + op = selection.NotIn + case metav1.LabelSelectorOpExists: + op = selection.Exists + case metav1.LabelSelectorOpDoesNotExist: + op = selection.DoesNotExist + default: + return nil, fmt.Errorf("%q is not a valid label selector operator", expr.Operator) + } + r, err := labels.NewRequirement(expr.Key, op, append([]string(nil), expr.Values...)) + if err != nil { + return nil, err + } + requirements = append(requirements, *r) + } + selector := labels.NewSelector() + selector = selector.Add(requirements...) + return selector, nil +} diff --git a/pilot/pkg/model/exportto_test.go b/pilot/pkg/model/exportto_test.go new file mode 100644 index 000000000000..ddb19ae7bdaa --- /dev/null +++ b/pilot/pkg/model/exportto_test.go @@ -0,0 +1,731 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package model_test + +import ( + "testing" + + "k8s.io/apimachinery/pkg/labels" + + typev1beta1 "istio.io/api/type/v1beta1" + "istio.io/istio/pilot/pkg/model" + "istio.io/istio/pkg/config/visibility" + "istio.io/istio/pkg/test/util/assert" + "istio.io/istio/pkg/util/sets" +) + +func TestExportToTarget_IsEmpty(t *testing.T) { + tests := []struct { + name string + target *model.ExportToTarget + expected bool + }{ + { + name: "nil target", + target: nil, + expected: true, + }, + { + name: "empty target", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{}, + }, + expected: true, + }, + { + name: "target with static namespace", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](visibility.Public), + Selectors: []labels.Selector{}, + }, + expected: false, + }, + { + name: "target with selector", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{labels.Everything()}, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.target.IsEmpty() + if got != tt.expected { + t.Errorf("IsEmpty() = %v, want %v", got, tt.expected) + } + }) + } +} + +func TestExportToTarget_Matches(t *testing.T) { + // Create label selectors for testing + envProdSelector, _ := model.LabelSelectorAsSelector(&typev1beta1.LabelSelector{ + MatchLabels: map[string]string{"env": "prod"}, + }) + envTestSelector, _ := model.LabelSelectorAsSelector(&typev1beta1.LabelSelector{ + MatchLabels: map[string]string{"env": "test"}, + }) + matchExpressionSelector, _ := model.LabelSelectorAsSelector(&typev1beta1.LabelSelector{ + MatchExpressions: []*typev1beta1.LabelSelectorRequirement{ + { + Key: "tier", + Operator: "In", + Values: []string{"frontend", "backend"}, + }, + }, + }) + + tests := []struct { + name string + target *model.ExportToTarget + namespace string + namespaceLabels map[string]string + expected bool + }{ + { + name: "nil target", + target: nil, + namespace: "ns1", + namespaceLabels: map[string]string{}, + expected: false, + }, + { + name: "public visibility", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](visibility.Public), + Selectors: []labels.Selector{}, + }, + namespace: "any-namespace", + namespaceLabels: map[string]string{}, + expected: true, + }, + { + name: "specific static namespace match", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1", "ns2"), + Selectors: []labels.Selector{}, + }, + namespace: "ns1", + namespaceLabels: map[string]string{}, + expected: true, + }, + { + name: "specific static namespace no match", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1", "ns2"), + Selectors: []labels.Selector{}, + }, + namespace: "ns3", + namespaceLabels: map[string]string{}, + expected: false, + }, + { + name: "label selector match", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{envProdSelector}, + }, + namespace: "prod-ns", + namespaceLabels: map[string]string{ + "env": "prod", + }, + expected: true, + }, + { + name: "label selector no match", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{envProdSelector}, + }, + namespace: "test-ns", + namespaceLabels: map[string]string{ + "env": "test", + }, + expected: false, + }, + { + name: "multiple selectors - one matches", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{envProdSelector, envTestSelector}, + }, + namespace: "test-ns", + namespaceLabels: map[string]string{ + "env": "test", + }, + expected: true, + }, + { + name: "combined static and selector - static matches", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("special-ns"), + Selectors: []labels.Selector{envProdSelector}, + }, + namespace: "special-ns", + namespaceLabels: map[string]string{ + "env": "dev", + }, + expected: true, + }, + { + name: "combined static and selector - selector matches", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("special-ns"), + Selectors: []labels.Selector{envProdSelector}, + }, + namespace: "prod-ns", + namespaceLabels: map[string]string{ + "env": "prod", + }, + expected: true, + }, + { + name: "combined static and selector - neither matches", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("special-ns"), + Selectors: []labels.Selector{envProdSelector}, + }, + namespace: "other-ns", + namespaceLabels: map[string]string{ + "env": "dev", + }, + expected: false, + }, + { + name: "match expression selector - matches", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{matchExpressionSelector}, + }, + namespace: "app-ns", + namespaceLabels: map[string]string{ + "tier": "frontend", + }, + expected: true, + }, + { + name: "match expression selector - no match", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{matchExpressionSelector}, + }, + namespace: "db-ns", + namespaceLabels: map[string]string{ + "tier": "database", + }, + expected: false, + }, + { + name: "selector with nil labels", + target: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{envProdSelector}, + }, + namespace: "ns1", + namespaceLabels: nil, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.target.Matches(tt.namespace, tt.namespaceLabels) + if got != tt.expected { + t.Errorf("Matches() = %v, want %v", got, tt.expected) + } + }) + } +} + +func TestParseExportTo(t *testing.T) { + tests := []struct { + name string + exportToList []string + exportToSelectors []*typev1beta1.LabelSelector + expectError bool + validateResult func(*testing.T, *model.ExportToTarget) + }{ + { + name: "empty inputs", + exportToList: []string{}, + exportToSelectors: []*typev1beta1.LabelSelector{}, + expectError: false, + validateResult: func(t *testing.T, target *model.ExportToTarget) { + assert.Equal(t, true, target.IsEmpty()) + }, + }, + { + name: "static namespaces only", + exportToList: []string{"ns1", "ns2", "*"}, + exportToSelectors: []*typev1beta1.LabelSelector{}, + expectError: false, + validateResult: func(t *testing.T, target *model.ExportToTarget) { + assert.Equal(t, 3, target.Len()) + assert.Equal(t, true, target.Contains(visibility.Instance("ns1"))) + assert.Equal(t, true, target.Contains(visibility.Instance("ns2"))) + assert.Equal(t, true, target.Contains(visibility.Public)) + }, + }, + { + name: "selectors only", + exportToList: []string{}, + exportToSelectors: []*typev1beta1.LabelSelector{ + { + MatchLabels: map[string]string{"env": "prod"}, + }, + }, + expectError: false, + validateResult: func(t *testing.T, target *model.ExportToTarget) { + assert.Equal(t, true, target.HasSelectors()) + assert.Equal(t, 1, len(target.Selectors)) + }, + }, + { + name: "combined static and selectors", + exportToList: []string{"ns1", "."}, + exportToSelectors: []*typev1beta1.LabelSelector{ + { + MatchLabels: map[string]string{"env": "prod"}, + }, + }, + expectError: false, + validateResult: func(t *testing.T, target *model.ExportToTarget) { + assert.Equal(t, 2, target.Len()) + assert.Equal(t, true, target.HasSelectors()) + assert.Equal(t, true, target.Contains(visibility.Private)) + assert.Equal(t, true, target.Contains(visibility.Instance("ns1"))) + }, + }, + { + name: "match expressions", + exportToList: []string{}, + exportToSelectors: []*typev1beta1.LabelSelector{ + { + MatchExpressions: []*typev1beta1.LabelSelectorRequirement{ + { + Key: "tier", + Operator: "In", + Values: []string{"frontend", "backend"}, + }, + }, + }, + }, + expectError: false, + validateResult: func(t *testing.T, target *model.ExportToTarget) { + assert.Equal(t, true, target.HasSelectors()) + assert.Equal(t, 1, len(target.Selectors)) + }, + }, + { + name: "invalid selector operator", + exportToList: []string{}, + exportToSelectors: []*typev1beta1.LabelSelector{ + { + MatchExpressions: []*typev1beta1.LabelSelectorRequirement{ + { + Key: "tier", + Operator: "InvalidOp", + Values: []string{"value"}, + }, + }, + }, + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := model.ParseExportTo(tt.exportToList, tt.exportToSelectors) + if tt.expectError { + if err == nil { + t.Errorf("ParseExportTo() expected error but got none") + } + return + } + if err != nil { + t.Errorf("ParseExportTo() unexpected error: %v", err) + return + } + if tt.validateResult != nil { + tt.validateResult(t, result) + } + }) + } +} + +func TestExportToTarget_Copy(t *testing.T) { + selector, _ := model.LabelSelectorAsSelector(&typev1beta1.LabelSelector{ + MatchLabels: map[string]string{"env": "prod"}, + }) + + original := &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1", "ns2"), + Selectors: []labels.Selector{selector}, + } + + // Test copy + copied := original.Copy() + + // Verify deep copy of static namespaces + if !original.StaticNamespaces.Equals(copied.StaticNamespaces) { + t.Errorf("Copy() static namespaces not equal") + } + + // Modify original and verify copied is not affected + original.StaticNamespaces.Insert("ns3") + if copied.StaticNamespaces.Contains("ns3") { + t.Errorf("Copy() is not a deep copy - modifications affect the copy") + } + + // Test nil copy + var nilTarget *model.ExportToTarget + if nilTarget.Copy() != nil { + t.Errorf("Copy() of nil should return nil") + } +} + +func TestExportToTarget_Equals(t *testing.T) { + selector1, _ := model.LabelSelectorAsSelector(&typev1beta1.LabelSelector{ + MatchLabels: map[string]string{"env": "prod"}, + }) + selector2, _ := model.LabelSelectorAsSelector(&typev1beta1.LabelSelector{ + MatchLabels: map[string]string{"env": "test"}, + }) + + tests := []struct { + name string + target1 *model.ExportToTarget + target2 *model.ExportToTarget + expected bool + }{ + { + name: "both nil", + target1: nil, + target2: nil, + expected: true, + }, + { + name: "one nil", + target1: nil, + target2: &model.ExportToTarget{StaticNamespaces: sets.New[visibility.Instance]()}, + expected: false, + }, + { + name: "same static namespaces, no selectors", + target1: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1", "ns2"), + Selectors: []labels.Selector{}, + }, + target2: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1", "ns2"), + Selectors: []labels.Selector{}, + }, + expected: true, + }, + { + name: "different static namespaces", + target1: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1"), + Selectors: []labels.Selector{}, + }, + target2: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns2"), + Selectors: []labels.Selector{}, + }, + expected: false, + }, + { + name: "same selectors", + target1: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{selector1}, + }, + target2: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{selector1}, + }, + expected: true, + }, + { + name: "different selectors", + target1: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{selector1}, + }, + target2: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{selector2}, + }, + expected: false, + }, + { + name: "different selector count", + target1: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{selector1}, + }, + target2: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance](), + Selectors: []labels.Selector{selector1, selector2}, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.target1.Equals(tt.target2) + if got != tt.expected { + t.Errorf("Equals() = %v, want %v", got, tt.expected) + } + }) + } +} + +func TestExportToTarget_IsSuperset(t *testing.T) { + selector, _ := model.LabelSelectorAsSelector(&typev1beta1.LabelSelector{ + MatchLabels: map[string]string{"env": "prod"}, + }) + + tests := []struct { + name string + target1 *model.ExportToTarget + target2 *model.ExportToTarget + expected bool + }{ + { + name: "superset static namespaces", + target1: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1", "ns2", "ns3"), + Selectors: []labels.Selector{}, + }, + target2: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1", "ns2"), + Selectors: []labels.Selector{}, + }, + expected: true, + }, + { + name: "not a superset", + target1: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1"), + Selectors: []labels.Selector{}, + }, + target2: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1", "ns2"), + Selectors: []labels.Selector{}, + }, + expected: false, + }, + { + name: "with selectors - returns false", + target1: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1", "ns2"), + Selectors: []labels.Selector{selector}, + }, + target2: &model.ExportToTarget{ + StaticNamespaces: sets.New[visibility.Instance]("ns1"), + Selectors: []labels.Selector{}, + }, + expected: false, + }, + { + name: "nil targets", + target1: nil, + target2: nil, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.target1.IsSuperset(tt.target2) + if got != tt.expected { + t.Errorf("IsSuperset() = %v, want %v", got, tt.expected) + } + }) + } +} + +func TestLabelSelectorAsSelector(t *testing.T) { + tests := []struct { + name string + selector *typev1beta1.LabelSelector + expectError bool + testMatch func(*testing.T, labels.Selector) + }{ + { + name: "nil selector", + selector: nil, + expectError: false, + testMatch: func(t *testing.T, sel labels.Selector) { + // Nothing() selector matches nothing + if sel.Matches(labels.Set{"any": "label"}) { + t.Error("nil selector should match nothing") + } + }, + }, + { + name: "empty selector", + selector: &typev1beta1.LabelSelector{}, + expectError: false, + testMatch: func(t *testing.T, sel labels.Selector) { + // Everything() selector matches everything + if !sel.Matches(labels.Set{"any": "label"}) { + t.Error("empty selector should match everything") + } + }, + }, + { + name: "match labels", + selector: &typev1beta1.LabelSelector{ + MatchLabels: map[string]string{ + "env": "prod", + "tier": "frontend", + }, + }, + expectError: false, + testMatch: func(t *testing.T, sel labels.Selector) { + // Should match when all labels present + if !sel.Matches(labels.Set{"env": "prod", "tier": "frontend", "extra": "label"}) { + t.Error("should match when all required labels present") + } + // Should not match when missing a label + if sel.Matches(labels.Set{"env": "prod"}) { + t.Error("should not match when missing required labels") + } + }, + }, + { + name: "match expressions - In", + selector: &typev1beta1.LabelSelector{ + MatchExpressions: []*typev1beta1.LabelSelectorRequirement{ + { + Key: "env", + Operator: "In", + Values: []string{"prod", "staging"}, + }, + }, + }, + expectError: false, + testMatch: func(t *testing.T, sel labels.Selector) { + if !sel.Matches(labels.Set{"env": "prod"}) { + t.Error("should match when value is in list") + } + if sel.Matches(labels.Set{"env": "dev"}) { + t.Error("should not match when value not in list") + } + }, + }, + { + name: "match expressions - NotIn", + selector: &typev1beta1.LabelSelector{ + MatchExpressions: []*typev1beta1.LabelSelectorRequirement{ + { + Key: "env", + Operator: "NotIn", + Values: []string{"dev"}, + }, + }, + }, + expectError: false, + testMatch: func(t *testing.T, sel labels.Selector) { + if !sel.Matches(labels.Set{"env": "prod"}) { + t.Error("should match when value not in excluded list") + } + if sel.Matches(labels.Set{"env": "dev"}) { + t.Error("should not match when value in excluded list") + } + }, + }, + { + name: "match expressions - Exists", + selector: &typev1beta1.LabelSelector{ + MatchExpressions: []*typev1beta1.LabelSelectorRequirement{ + { + Key: "env", + Operator: "Exists", + }, + }, + }, + expectError: false, + testMatch: func(t *testing.T, sel labels.Selector) { + if !sel.Matches(labels.Set{"env": "any-value"}) { + t.Error("should match when label exists") + } + if sel.Matches(labels.Set{"other": "label"}) { + t.Error("should not match when label doesn't exist") + } + }, + }, + { + name: "match expressions - DoesNotExist", + selector: &typev1beta1.LabelSelector{ + MatchExpressions: []*typev1beta1.LabelSelectorRequirement{ + { + Key: "env", + Operator: "DoesNotExist", + }, + }, + }, + expectError: false, + testMatch: func(t *testing.T, sel labels.Selector) { + if !sel.Matches(labels.Set{"other": "label"}) { + t.Error("should match when label doesn't exist") + } + if sel.Matches(labels.Set{"env": "prod"}) { + t.Error("should not match when label exists") + } + }, + }, + { + name: "invalid operator", + selector: &typev1beta1.LabelSelector{ + MatchExpressions: []*typev1beta1.LabelSelectorRequirement{ + { + Key: "env", + Operator: "InvalidOp", + Values: []string{"value"}, + }, + }, + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + selector, err := model.LabelSelectorAsSelector(tt.selector) + if tt.expectError { + if err == nil { + t.Error("expected error but got none") + } + return + } + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + if tt.testMatch != nil { + tt.testMatch(t, selector) + } + }) + } +} diff --git a/pilot/pkg/model/push_context.go b/pilot/pkg/model/push_context.go index efbe035b3706..4ae0fe927873 100644 --- a/pilot/pkg/model/push_context.go +++ b/pilot/pkg/model/push_context.go @@ -291,7 +291,8 @@ type consolidatedDestRules struct { // ConsolidatedDestRule represents a dr and from which it is consolidated. type ConsolidatedDestRule struct { // the list of namespaces to which this destination rule has been exported to - exportTo sets.Set[visibility.Instance] + // It supports both static namespace names and label selectors for dynamic matching. + exportTo *ExportToTarget // rule is merged from the following destinationRules. rule *config.Config // the original dest rules from which above rule is merged. @@ -1077,13 +1078,26 @@ func (ps *PushContext) ServiceForHostname(proxy *Proxy, hostname host.Name) *Ser } // IsServiceVisible returns true if the input service is visible to the given namespace. +// This function checks both static namespace names and label selectors in exportTo. +// For label selector matching, it uses the provided NamespaceLabelsGetter to look up namespace labels. func (ps *PushContext) IsServiceVisible(service *Service, namespace string) bool { + return ps.IsServiceVisibleWithLabels(service, namespace, nil) +} + +// IsServiceVisibleWithLabels returns true if the input service is visible to the given namespace. +// This function checks both static namespace names and label selectors in exportTo. +// If env is provided, it will use it to look up namespace labels for dynamic selector matching. +// If env is nil or doesn't have a NamespaceLabelsGetter, only static namespace checks are performed. +func (ps *PushContext) IsServiceVisibleWithLabels(service *Service, namespace string, env *Environment) bool { if service == nil { return false } ns := service.Attributes.Namespace - if service.Attributes.ExportTo.IsEmpty() { + exportTo := service.Attributes.ExportTo + + // Handle empty exportTo - use defaults + if exportTo.IsEmpty() { if ps.exportToDefaults.service.Contains(visibility.Private) { return ns == namespace } else if ps.exportToDefaults.service.Contains(visibility.Public) { @@ -1091,9 +1105,24 @@ func (ps *PushContext) IsServiceVisible(service *Service, namespace string) bool } } - return service.Attributes.ExportTo.Contains(visibility.Public) || - (service.Attributes.ExportTo.Contains(visibility.Private) && ns == namespace) || - service.Attributes.ExportTo.Contains(visibility.Instance(namespace)) + // Check if service has label selectors and we have a way to look up namespace labels + if exportTo.HasSelectors() && env != nil { + namespaceLabels := env.GetNamespaceLabels(namespace) + // Use the Matches method which handles both static namespaces and label selectors + if exportTo.Matches(namespace, namespaceLabels) { + return true + } + // Fall through to check private visibility + if exportTo.Contains(visibility.Private) && ns == namespace { + return true + } + return false + } + + // Static namespace visibility check (legacy path when no selectors or no env) + return exportTo.Contains(visibility.Public) || + (exportTo.Contains(visibility.Private) && ns == namespace) || + exportTo.Contains(visibility.Instance(namespace)) } // VirtualServicesForGateway lists all virtual services bound to the specified gateways @@ -1579,7 +1608,7 @@ func (ps *PushContext) initServiceRegistry(env *Environment, configsUpdate sets. continue } // . or other namespaces - for exportTo := range s.Attributes.ExportTo { + for _, exportTo := range s.Attributes.ExportTo.StaticNamespacesList() { if exportTo == visibility.Private || string(exportTo) == ns { // exportTo with same namespace is effectively private ps.ServiceIndex.privateByNamespace[ns] = append(ps.ServiceIndex.privateByNamespace[ns], s) @@ -1588,6 +1617,8 @@ func (ps *PushContext) initServiceRegistry(env *Environment, configsUpdate sets. ps.ServiceIndex.exportedToNamespace[string(exportTo)] = append(ps.ServiceIndex.exportedToNamespace[string(exportTo)], s) } } + // Note: Services with label selectors in exportTo are not currently indexed + // They will be handled dynamically through IsServiceVisible with namespace labels } } @@ -1775,7 +1806,21 @@ func (ps *PushContext) initVirtualServices(env *Environment) { ns := virtualService.Namespace rule := virtualService.Spec.(*networking.VirtualService) gwNames := getGatewayNames(rule) - if len(rule.ExportTo) == 0 { + + var exportToSet *ExportToTarget + var err error + if len(rule.ExportTo) == 0 && len(rule.ExportToSelectors) == 0 { + // No exportTo in virtualService. Use empty set to trigger default behavior + exportToSet = &ExportToTarget{StaticNamespaces: sets.New[visibility.Instance]()} + } else { + exportToSet, err = ParseExportTo(rule.ExportTo, rule.ExportToSelectors) + if err != nil { + log.Warnf("Failed to parse exportTo for VirtualService %s/%s: %v", virtualService.Namespace, virtualService.Name, err) + exportToSet = &ExportToTarget{StaticNamespaces: sets.New[visibility.Instance]()} + } + } + + if exportToSet.IsEmpty() { // No exportTo in virtualService. Use the global default // We only honor ., * if ps.exportToDefaults.virtualService.Contains(visibility.Private) { @@ -1791,10 +1836,6 @@ func (ps *PushContext) initVirtualServices(env *Environment) { } } } else { - exportToSet := sets.NewWithLength[visibility.Instance](len(rule.ExportTo)) - for _, e := range rule.ExportTo { - exportToSet.Insert(visibility.Instance(e)) - } // if vs has exportTo ~ - i.e. not visible to anyone, ignore all exportTos // if vs has exportTo *, make public and ignore all other exportTos // if vs has exportTo ., replace with current namespace @@ -1804,7 +1845,7 @@ func (ps *PushContext) initVirtualServices(env *Environment) { } } else if !exportToSet.Contains(visibility.None) { // . or other namespaces - for exportTo := range exportToSet { + for _, exportTo := range exportToSet.StaticNamespacesList() { if exportTo == visibility.Private || string(exportTo) == ns { // add to local namespace only for _, gw := range gwNames { @@ -2069,16 +2110,18 @@ func (ps *PushContext) setDestinationRules(configs []config.Config) { rule := configs[i].Spec.(*networking.DestinationRule) rule.Host = string(ResolveShortnameToFQDN(rule.Host, configs[i].Meta)) - var exportToSet sets.Set[visibility.Instance] + var exportToSet *ExportToTarget // destination rules with workloadSelector should not be exported to other namespaces if rule.GetWorkloadSelector() == nil { - exportToSet = sets.NewWithLength[visibility.Instance](len(rule.ExportTo)) - for _, e := range rule.ExportTo { - exportToSet.Insert(visibility.Instance(e)) + var err error + exportToSet, err = ParseExportTo(rule.ExportTo, rule.ExportToSelectors) + if err != nil { + log.Warnf("Failed to parse exportTo for DestinationRule %s/%s: %v", configs[i].Namespace, configs[i].Name, err) + exportToSet = &ExportToTarget{StaticNamespaces: sets.New[visibility.Instance]()} } } else { - exportToSet = sets.New[visibility.Instance](visibility.Private) + exportToSet = &ExportToTarget{StaticNamespaces: sets.New[visibility.Instance](visibility.Private)} } // add only if the dest rule is exported with . or * or explicit exportTo containing this namespace diff --git a/pilot/pkg/model/push_context_test.go b/pilot/pkg/model/push_context_test.go index 7116f9db1a87..d0b17de00459 100644 --- a/pilot/pkg/model/push_context_test.go +++ b/pilot/pkg/model/push_context_test.go @@ -58,6 +58,14 @@ import ( "istio.io/istio/pkg/util/sets" ) +// makeExportTo creates an ExportToTarget from visibility instances. +// This is a test helper function to make it easier to create ExportToTarget in tests. +func makeExportTo(vis ...visibility.Instance) *ExportToTarget { + return &ExportToTarget{ + StaticNamespaces: sets.New(vis...), + } +} + func TestMergeUpdateRequest(t *testing.T) { push0 := &PushContext{} // trivially different push contexts just for testing @@ -1446,7 +1454,7 @@ func TestServiceIndex(t *testing.T) { Ports: allPorts, Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.Public), + ExportTo: makeExportTo(visibility.Public), }, }, { @@ -1454,7 +1462,7 @@ func TestServiceIndex(t *testing.T) { Ports: allPorts, Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.Private), + ExportTo: makeExportTo(visibility.Private), }, }, { @@ -1462,7 +1470,7 @@ func TestServiceIndex(t *testing.T) { Ports: allPorts, Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.None), + ExportTo: makeExportTo(visibility.None), }, }, { @@ -1470,7 +1478,7 @@ func TestServiceIndex(t *testing.T) { Ports: allPorts, Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.Instance("namespace")), + ExportTo: makeExportTo(visibility.Instance("namespace")), }, }, }, @@ -1571,7 +1579,7 @@ func TestIsServiceVisible(t *testing.T) { service: &Service{ Attributes: ServiceAttributes{ Namespace: "foo", - ExportTo: sets.New(visibility.Private), + ExportTo: makeExportTo(visibility.Private), }, }, expect: true, @@ -1582,7 +1590,7 @@ func TestIsServiceVisible(t *testing.T) { service: &Service{ Attributes: ServiceAttributes{ Namespace: "bar", - ExportTo: sets.New(visibility.Private), + ExportTo: makeExportTo(visibility.Private), }, }, expect: false, @@ -1593,7 +1601,7 @@ func TestIsServiceVisible(t *testing.T) { service: &Service{ Attributes: ServiceAttributes{ Namespace: "bar", - ExportTo: sets.New(visibility.Public), + ExportTo: makeExportTo(visibility.Public), }, }, expect: true, @@ -1604,7 +1612,7 @@ func TestIsServiceVisible(t *testing.T) { service: &Service{ Attributes: ServiceAttributes{ Namespace: "bar", - ExportTo: sets.New(visibility.Instance("foo")), + ExportTo: makeExportTo(visibility.Instance("foo")), }, }, expect: true, @@ -1615,7 +1623,7 @@ func TestIsServiceVisible(t *testing.T) { service: &Service{ Attributes: ServiceAttributes{ Namespace: "bar", - ExportTo: sets.New(visibility.Instance("baz")), + ExportTo: makeExportTo(visibility.Instance("baz")), }, }, expect: false, @@ -1626,7 +1634,7 @@ func TestIsServiceVisible(t *testing.T) { service: &Service{ Attributes: ServiceAttributes{ Namespace: "bar", - ExportTo: sets.New(visibility.None), + ExportTo: makeExportTo(visibility.None), }, }, expect: false, @@ -1637,7 +1645,7 @@ func TestIsServiceVisible(t *testing.T) { service: &Service{ Attributes: ServiceAttributes{ Namespace: "bar", - ExportTo: sets.New( + ExportTo: makeExportTo( visibility.Public, visibility.None, ), @@ -1651,7 +1659,7 @@ func TestIsServiceVisible(t *testing.T) { service: &Service{ Attributes: ServiceAttributes{ Namespace: "bar", - ExportTo: sets.New( + ExportTo: makeExportTo( visibility.Private, visibility.None, ), @@ -1732,7 +1740,7 @@ func TestInitPushContext(t *testing.T) { Ports: allPorts, Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.Public), + ExportTo: makeExportTo(visibility.Public), }, }, }, @@ -3194,14 +3202,14 @@ func TestServiceWithExportTo(t *testing.T) { Hostname: "svc1", Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.Private, visibility.Instance("ns1")), + ExportTo: makeExportTo(visibility.Private, visibility.Instance("ns1")), }, } svc2 := &Service{ Hostname: "svc2", Attributes: ServiceAttributes{ Namespace: "test2", - ExportTo: sets.New( + ExportTo: makeExportTo( visibility.Instance("test1"), visibility.Instance("ns1"), visibility.Instance("test2"), @@ -3212,7 +3220,7 @@ func TestServiceWithExportTo(t *testing.T) { Hostname: "svc3", Attributes: ServiceAttributes{ Namespace: "test3", - ExportTo: sets.New( + ExportTo: makeExportTo( visibility.Instance("test1"), visibility.Public, visibility.Instance("test2"), @@ -3293,7 +3301,7 @@ func TestInstancesByPort(t *testing.T) { Attributes: ServiceAttributes{ Namespace: "test5", ServiceRegistry: provider.External, - ExportTo: sets.New( + ExportTo: makeExportTo( visibility.Instance("test5"), ), }, @@ -3305,7 +3313,7 @@ func TestInstancesByPort(t *testing.T) { Attributes: ServiceAttributes{ Namespace: "test5", ServiceRegistry: provider.External, - ExportTo: sets.New( + ExportTo: makeExportTo( visibility.Instance("test5"), ), }, @@ -3692,7 +3700,7 @@ func BenchmarkInitServiceAccounts(b *testing.B) { Ports: allPorts, Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.Public), + ExportTo: makeExportTo(visibility.Public), }, }, { @@ -3700,7 +3708,7 @@ func BenchmarkInitServiceAccounts(b *testing.B) { Ports: allPorts, Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.Private), + ExportTo: makeExportTo(visibility.Private), }, }, { @@ -3708,7 +3716,7 @@ func BenchmarkInitServiceAccounts(b *testing.B) { Ports: allPorts, Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.None), + ExportTo: makeExportTo(visibility.None), }, }, { @@ -3716,7 +3724,7 @@ func BenchmarkInitServiceAccounts(b *testing.B) { Ports: allPorts, Attributes: ServiceAttributes{ Namespace: "test1", - ExportTo: sets.New(visibility.Instance("namespace")), + ExportTo: makeExportTo(visibility.Instance("namespace")), }, }, } diff --git a/pilot/pkg/model/service.go b/pilot/pkg/model/service.go index 6b18e41a0d44..3962f13ef759 100644 --- a/pilot/pkg/model/service.go +++ b/pilot/pkg/model/service.go @@ -49,7 +49,6 @@ import ( "istio.io/istio/pkg/config/labels" "istio.io/istio/pkg/config/protocol" "istio.io/istio/pkg/config/schema/kind" - "istio.io/istio/pkg/config/visibility" "istio.io/istio/pkg/maps" pm "istio.io/istio/pkg/model" "istio.io/istio/pkg/network" @@ -727,7 +726,8 @@ type ServiceAttributes struct { Labels map[string]string // ExportTo defines the visibility of Service in // a namespace when the namespace is imported. - ExportTo sets.Set[visibility.Instance] + // It supports both static namespace names and label selectors for dynamic matching. + ExportTo *ExportToTarget // LabelSelectors are the labels used by the service to select workloads. // Applicable to both Kubernetes and ServiceEntries. @@ -829,9 +829,7 @@ func (s *ServiceAttributes) DeepCopy() ServiceAttributes { out := *s out.Labels = maps.Clone(s.Labels) - if s.ExportTo != nil { - out.ExportTo = s.ExportTo.Copy() - } + out.ExportTo = s.ExportTo.Copy() out.LabelSelectors = maps.Clone(s.LabelSelectors) out.ClusterExternalAddresses = s.ClusterExternalAddresses.DeepCopy() @@ -868,7 +866,12 @@ func (s *ServiceAttributes) Equals(other *ServiceAttributes) bool { return false } - if !maps.Equal(s.ExportTo, other.ExportTo) { + if s.ExportTo != nil && other.ExportTo != nil { + if !s.ExportTo.Equals(other.ExportTo) { + return false + } + } else if s.ExportTo != other.ExportTo { + // One is nil and the other is not return false } diff --git a/pilot/pkg/model/service_test.go b/pilot/pkg/model/service_test.go index 48e074804a8f..ccb24bc3e73a 100644 --- a/pilot/pkg/model/service_test.go +++ b/pilot/pkg/model/service_test.go @@ -21,6 +21,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" fuzz "github.com/google/gofuzz" + v1beta1 "istio.io/api/type/v1beta1" "istio.io/istio/pilot/pkg/features" "istio.io/istio/pkg/cluster" "istio.io/istio/pkg/config/constants" @@ -561,7 +562,23 @@ func BenchmarkServiceDeepCopy(b *testing.B) { } func TestFuzzServiceDeepCopy(t *testing.T) { - fuzzer := fuzz.New() + fuzzer := fuzz.New().NilChance(0.5).Funcs( + func(t *ExportToTarget, c fuzz.Continue) { + // Provide a simple valid ExportToTarget to avoid protobuf nil issues + *t = ExportToTarget{ + StaticNamespaces: nil, + Selectors: nil, + } + }, + func(s *v1beta1.LabelSelector, c fuzz.Continue) { + // Provide an empty LabelSelector to avoid protobuf nil issues + *s = v1beta1.LabelSelector{} + }, + func(s *v1beta1.LabelSelectorRequirement, c fuzz.Continue) { + // Provide an empty LabelSelectorRequirement to avoid protobuf nil issues + *s = v1beta1.LabelSelectorRequirement{} + }, + ) originalSvc := &Service{} fuzzer.Fuzz(originalSvc) copied := originalSvc.DeepCopy() diff --git a/pilot/pkg/model/sidecar.go b/pilot/pkg/model/sidecar.go index ba473a19960f..aa06f4e71208 100644 --- a/pilot/pkg/model/sidecar.go +++ b/pilot/pkg/model/sidecar.go @@ -983,7 +983,12 @@ func canMergeServices(s1, s2 *Service) bool { return false } - if !maps.Equal(s1.Attributes.ExportTo, s2.Attributes.ExportTo) { + if s1.Attributes.ExportTo != nil && s2.Attributes.ExportTo != nil { + if !s1.Attributes.ExportTo.Equals(s2.Attributes.ExportTo) { + return false + } + } else if s1.Attributes.ExportTo != s2.Attributes.ExportTo { + // One is nil and the other is not return false } diff --git a/pilot/pkg/model/sidecar_test.go b/pilot/pkg/model/sidecar_test.go index ed9f292a26b0..43f6c2291f18 100644 --- a/pilot/pkg/model/sidecar_test.go +++ b/pilot/pkg/model/sidecar_test.go @@ -44,6 +44,14 @@ import ( "istio.io/istio/pkg/util/sets" ) +// makeExportTo creates an ExportToTarget from visibility instances. +// This is a test helper function to make it easier to create ExportToTarget in tests. +func makeExportToFromInstances(vis ...visibility.Instance) *ExportToTarget { + return &ExportToTarget{ + StaticNamespaces: sets.New(vis...), + } +} + var ( port9999 = []*Port{ { @@ -919,7 +927,7 @@ var ( Attributes: ServiceAttributes{ Name: "foo", Namespace: "ns1", - ExportTo: sets.New(visibility.Private), + ExportTo: makeExportToFromInstances(visibility.Private), }, }, { @@ -2693,7 +2701,7 @@ func TestCreateSidecarScope(t *testing.T) { Attributes: ServiceAttributes{ Name: "foo", Namespace: "ns1", - ExportTo: sets.New(visibility.Public), + ExportTo: makeExportToFromInstances(visibility.Public), }, }, { @@ -2703,7 +2711,7 @@ func TestCreateSidecarScope(t *testing.T) { Attributes: ServiceAttributes{ Name: "bar", Namespace: "ns1", - ExportTo: sets.New(visibility.Private), + ExportTo: makeExportToFromInstances(visibility.Private), }, }, }, @@ -2714,7 +2722,7 @@ func TestCreateSidecarScope(t *testing.T) { Attributes: ServiceAttributes{ Name: "foo", Namespace: "ns1", - ExportTo: sets.New(visibility.Public), + ExportTo: makeExportToFromInstances(visibility.Public), }, }, }, diff --git a/pilot/pkg/model/virtualservice_test.go b/pilot/pkg/model/virtualservice_test.go index ec897770a294..6b3fb52cc075 100644 --- a/pilot/pkg/model/virtualservice_test.go +++ b/pilot/pkg/model/virtualservice_test.go @@ -35,6 +35,14 @@ import ( "istio.io/istio/pkg/util/sets" ) +// makeExportToSimple creates an ExportToTarget from a single visibility instance. +// This is a test helper function. +func makeExportToSimple(vis visibility.Instance) *ExportToTarget { + return &ExportToTarget{ + StaticNamespaces: sets.New(vis), + } +} + const wildcardIP = "0.0.0.0" func TestMergeVirtualServices(t *testing.T) { @@ -2359,7 +2367,7 @@ func buildHTTPService(hostname string, v visibility.Instance, ip, namespace stri Attributes: ServiceAttributes{ ServiceRegistry: provider.Kubernetes, Namespace: namespace, - ExportTo: sets.New(v), + ExportTo: makeExportToSimple(v), }, } if ip == wildcardIP { diff --git a/pilot/pkg/serviceregistry/kube/controller/namespace_labels.go b/pilot/pkg/serviceregistry/kube/controller/namespace_labels.go new file mode 100644 index 000000000000..eda8d2892dc7 --- /dev/null +++ b/pilot/pkg/serviceregistry/kube/controller/namespace_labels.go @@ -0,0 +1,41 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controller + +import ( + corev1 "k8s.io/api/core/v1" + + "istio.io/istio/pilot/pkg/model" + "istio.io/istio/pkg/kube/kclient" +) + +// KubeNamespaceLabelsGetter implements model.NamespaceLabelsGetter using a Kubernetes client. +type KubeNamespaceLabelsGetter struct { + client kclient.Client[*corev1.Namespace] +} + +// NewKubeNamespaceLabelsGetter creates a new KubeNamespaceLabelsGetter. +func NewKubeNamespaceLabelsGetter(client kclient.Client[*corev1.Namespace]) model.NamespaceLabelsGetter { + return &KubeNamespaceLabelsGetter{client: client} +} + +// GetNamespaceLabels returns the labels for the given namespace. +func (g *KubeNamespaceLabelsGetter) GetNamespaceLabels(namespace string) map[string]string { + ns := g.client.Get(namespace, "") + if ns == nil { + return nil + } + return ns.Labels +} diff --git a/pilot/pkg/serviceregistry/kube/controller/namespace_labels_test.go b/pilot/pkg/serviceregistry/kube/controller/namespace_labels_test.go new file mode 100644 index 000000000000..2abba4abfd3e --- /dev/null +++ b/pilot/pkg/serviceregistry/kube/controller/namespace_labels_test.go @@ -0,0 +1,144 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controller + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "istio.io/istio/pkg/kube" + "istio.io/istio/pkg/kube/kclient" + "istio.io/istio/pkg/test" +) + +func TestKubeNamespaceLabelsGetter(t *testing.T) { + client := kube.NewFakeClient() + stop := test.NewStop(t) + + namespaces := kclient.New[*corev1.Namespace](client) + + // Create some test namespaces + prodNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "prod", + Labels: map[string]string{ + "env": "production", + "tier": "frontend", + }, + }, + } + testNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{ + "env": "testing", + }, + }, + } + noLabelsNs := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "no-labels", + Labels: map[string]string{}, + }, + } + + client.Kube().CoreV1().Namespaces().Create(nil, prodNs, metav1.CreateOptions{}) + client.Kube().CoreV1().Namespaces().Create(nil, testNs, metav1.CreateOptions{}) + client.Kube().CoreV1().Namespaces().Create(nil, noLabelsNs, metav1.CreateOptions{}) + + // Wait for namespaces to be synced + client.RunAndWait(stop) + + // Create the namespace labels getter + getter := NewKubeNamespaceLabelsGetter(namespaces) + + tests := []struct { + name string + namespace string + expectedLabels map[string]string + }{ + { + name: "namespace with labels", + namespace: "prod", + expectedLabels: map[string]string{ + "env": "production", + "tier": "frontend", + }, + }, + { + name: "namespace with one label", + namespace: "test", + expectedLabels: map[string]string{ + "env": "testing", + }, + }, + { + name: "namespace with no labels", + namespace: "no-labels", + expectedLabels: map[string]string{}, + }, + { + name: "non-existent namespace", + namespace: "non-existent", + expectedLabels: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + labels := getter.GetNamespaceLabels(tt.namespace) + if !mapsEqual(labels, tt.expectedLabels) { + t.Errorf("GetNamespaceLabels(%s) = %v, expected %v", tt.namespace, labels, tt.expectedLabels) + } + }) + } +} + +func TestKubeNamespaceLabelsGetter_EmptyNamespace(t *testing.T) { + client := kube.NewFakeClient() + stop := test.NewStop(t) + + namespaces := kclient.New[*corev1.Namespace](client) + + client.RunAndWait(stop) + + getter := NewKubeNamespaceLabelsGetter(namespaces) + + // Test with empty namespace name + labels := getter.GetNamespaceLabels("") + if labels != nil { + t.Errorf("GetNamespaceLabels(\"\") = %v, expected nil", labels) + } +} + +func mapsEqual(a, b map[string]string) bool { + if len(a) != len(b) { + return false + } + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } + for k, v := range a { + if b[k] != v { + return false + } + } + return true +} diff --git a/pilot/pkg/serviceregistry/kube/conversion.go b/pilot/pkg/serviceregistry/kube/conversion.go index a0efe3544669..747f3514612f 100644 --- a/pilot/pkg/serviceregistry/kube/conversion.go +++ b/pilot/pkg/serviceregistry/kube/conversion.go @@ -72,7 +72,24 @@ func ConvertService(svc corev1.Service, domainSuffix string, clusterID cluster.I ports = append(ports, convertPort(port)) } - var exportTo sets.Set[visibility.Instance] + // Parse exportTo annotation + var exportTo *model.ExportToTarget + if exportToAnnotation, ok := svc.Annotations[annotation.NetworkingExportTo.Name]; ok { + if exportToAnnotation != "" { + namespaces := strings.Split(exportToAnnotation, ",") + // Trim whitespace from each namespace + for i := range namespaces { + namespaces[i] = strings.TrimSpace(namespaces[i]) + } + // Parse using ParseExportTo (no selectors from annotations) + exportTo, _ = model.ParseExportTo(namespaces, nil) + // Ignore errors - invalid exportTo will result in nil which is acceptable + } else { + // Empty annotation value - return empty ExportToTarget + exportTo = &model.ExportToTarget{StaticNamespaces: sets.New[visibility.Instance]()} + } + } + serviceaccounts := make([]string, 0) if svc.Annotations[annotation.AlphaCanonicalServiceAccounts.Name] != "" { serviceaccounts = append(serviceaccounts, strings.Split(svc.Annotations[annotation.AlphaCanonicalServiceAccounts.Name], ",")...) @@ -82,14 +99,6 @@ func ConvertService(svc corev1.Service, domainSuffix string, clusterID cluster.I serviceaccounts = append(serviceaccounts, kubeToIstioServiceAccount(ksa, svc.Namespace, trustDomain)) } } - if svc.Annotations[annotation.NetworkingExportTo.Name] != "" { - namespaces := strings.Split(svc.Annotations[annotation.NetworkingExportTo.Name], ",") - exportTo = sets.NewWithLength[visibility.Instance](len(namespaces)) - for _, ns := range namespaces { - ns = strings.TrimSpace(ns) - exportTo.Insert(visibility.Instance(ns)) - } - } istioService := &model.Service{ Hostname: ServiceHostname(svc.Name, svc.Namespace, domainSuffix), diff --git a/pilot/pkg/serviceregistry/kube/conversion_test.go b/pilot/pkg/serviceregistry/kube/conversion_test.go index f1ede23813db..73d1e69a6095 100644 --- a/pilot/pkg/serviceregistry/kube/conversion_test.go +++ b/pilot/pkg/serviceregistry/kube/conversion_test.go @@ -25,6 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "istio.io/api/annotation" + "istio.io/istio/pilot/pkg/model" "istio.io/istio/pkg/cluster" "istio.io/istio/pkg/config/kube" "istio.io/istio/pkg/config/protocol" @@ -296,16 +297,16 @@ func TestServiceConversionWithExportToAnnotation(t *testing.T) { tests := []struct { Annotation string - Want sets.Set[visibility.Instance] + Want *model.ExportToTarget }{ - {"", sets.Set[visibility.Instance]{}}, - {".", sets.New(visibility.Private)}, - {"*", sets.New(visibility.Public)}, - {"~", sets.New(visibility.None)}, - {"ns", sets.New(visibility.Instance("ns"))}, - {"ns1,ns2", sets.New(visibility.Instance("ns1"), visibility.Instance("ns2"))}, - {"ns1, ns2", sets.New(visibility.Instance("ns1"), visibility.Instance("ns2"))}, - {"ns1 ,ns2", sets.New(visibility.Instance("ns1"), visibility.Instance("ns2"))}, + {"", &model.ExportToTarget{StaticNamespaces: sets.Set[visibility.Instance]{}}}, + {".", &model.ExportToTarget{StaticNamespaces: sets.New(visibility.Private)}}, + {"*", &model.ExportToTarget{StaticNamespaces: sets.New(visibility.Public)}}, + {"~", &model.ExportToTarget{StaticNamespaces: sets.New(visibility.None)}}, + {"ns", &model.ExportToTarget{StaticNamespaces: sets.New(visibility.Instance("ns"))}}, + {"ns1,ns2", &model.ExportToTarget{StaticNamespaces: sets.New(visibility.Instance("ns1"), visibility.Instance("ns2"))}}, + {"ns1, ns2", &model.ExportToTarget{StaticNamespaces: sets.New(visibility.Instance("ns1"), visibility.Instance("ns2"))}}, + {"ns1 ,ns2", &model.ExportToTarget{StaticNamespaces: sets.New(visibility.Instance("ns1"), visibility.Instance("ns2"))}}, } for _, test := range tests { localSvc.Annotations[annotation.NetworkingExportTo.Name] = test.Annotation diff --git a/pilot/pkg/serviceregistry/serviceentry/conversion.go b/pilot/pkg/serviceregistry/serviceentry/conversion.go index d0e76facbdc5..dbfe5372affb 100644 --- a/pilot/pkg/serviceregistry/serviceentry/conversion.go +++ b/pilot/pkg/serviceregistry/serviceentry/conversion.go @@ -31,13 +31,11 @@ import ( "istio.io/istio/pkg/config/host" "istio.io/istio/pkg/config/protocol" "istio.io/istio/pkg/config/schema/gvk" - "istio.io/istio/pkg/config/visibility" "istio.io/istio/pkg/kube/labels" pm "istio.io/istio/pkg/model" "istio.io/istio/pkg/network" "istio.io/istio/pkg/spiffe" netutil "istio.io/istio/pkg/util/net" - "istio.io/istio/pkg/util/sets" ) func convertPort(port *networking.ServicePort) *model.Port { @@ -94,9 +92,13 @@ func ServiceToServiceEntry(svc *model.Service, proxy *model.Proxy) *config.Confi } // Based on networking.istio.io/exportTo annotation - for k := range svc.Attributes.ExportTo { - // k is Private or Public - se.ExportTo = append(se.ExportTo, string(k)) + if svc.Attributes.ExportTo != nil { + for _, k := range svc.Attributes.ExportTo.StaticNamespacesList() { + // k is Private or Public + se.ExportTo = append(se.ExportTo, string(k)) + } + // Note: exportToSelectors are not currently converted back to ServiceEntry + // as this reverse conversion is typically used for debugging/display purposes only } if svc.MeshExternal { @@ -197,12 +199,12 @@ func convertServices(cfg config.Config) []*model.Service { } } - var exportTo sets.Set[visibility.Instance] - if len(serviceEntry.ExportTo) > 0 { - exportTo = sets.NewWithLength[visibility.Instance](len(serviceEntry.ExportTo)) - for _, e := range serviceEntry.ExportTo { - exportTo.Insert(visibility.Instance(e)) - } + // Parse exportTo and exportToSelectors into ExportToTarget + exportTo, err := model.ParseExportTo(serviceEntry.ExportTo, serviceEntry.ExportToSelectors) + if err != nil { + // Log error but continue with empty export target + log.Warnf("Failed to parse exportTo for ServiceEntry %s/%s: %v", cfg.Namespace, cfg.Name, err) + exportTo = nil } var labelSelectors map[string]string diff --git a/pkg/config/validation/validation.go b/pkg/config/validation/validation.go index 645c5b276a26..83d477cbe86f 100644 --- a/pkg/config/validation/validation.go +++ b/pkg/config/validation/validation.go @@ -679,6 +679,8 @@ var ValidateDestinationRule = RegisterValidateFunc("ValidateDestinationRule", } v = AppendValidation(v, validateExportTo(cfg.Namespace, rule.ExportTo, false, rule.GetWorkloadSelector() != nil)) + v = AppendValidation(v, + validateExportToSelectors(cfg.Namespace, rule.ExportTo, rule.ExportToSelectors, rule.GetWorkloadSelector() != nil)) v = AppendValidation(v, validateWorkloadSelector(rule.GetWorkloadSelector())) @@ -749,6 +751,78 @@ func validateExportTo(namespace string, exportTo []string, isServiceEntry bool, return errs } +func validateExportToSelectors(namespace string, exportTo []string, exportToSelectors []*type_beta.LabelSelector, isDestinationRuleWithSelector bool) (errs error) { + if len(exportToSelectors) == 0 { + return nil + } + + // DestinationRule with workload selector cannot use export_to_selectors + if isDestinationRuleWithSelector { + return fmt.Errorf("destination rule with workload selector cannot use export_to_selectors") + } + + // Cannot mix "*" in exportTo with export_to_selectors + if len(exportTo) > 0 { + exportToSet := sets.New[string]() + for _, e := range exportTo { + exportToSet.Insert(e) + } + if exportToSet.Contains(string(visibility.Public)) { + return fmt.Errorf("cannot use both public (*) in exportTo and export_to_selectors") + } + } + + // Validate each label selector + for i, selector := range exportToSelectors { + if selector == nil { + errs = appendErrors(errs, fmt.Errorf("export_to_selectors[%d] cannot be nil", i)) + continue + } + + // Validate matchLabels + for key, value := range selector.MatchLabels { + if key == "" { + errs = appendErrors(errs, fmt.Errorf("export_to_selectors[%d]: label key cannot be empty", i)) + } + if value == "" { + errs = appendErrors(errs, fmt.Errorf("export_to_selectors[%d]: label value for key %q cannot be empty", i, key)) + } + } + + // Validate matchExpressions + for j, expr := range selector.MatchExpressions { + if expr == nil { + errs = appendErrors(errs, fmt.Errorf("export_to_selectors[%d].matchExpressions[%d] cannot be nil", i, j)) + continue + } + + if expr.Key == "" { + errs = appendErrors(errs, fmt.Errorf("export_to_selectors[%d].matchExpressions[%d]: key cannot be empty", i, j)) + } + + // Validate operator + validOps := sets.New("In", "NotIn", "Exists", "DoesNotExist") + if !validOps.Contains(expr.Operator) { + errs = appendErrors(errs, fmt.Errorf("export_to_selectors[%d].matchExpressions[%d]: invalid operator %q, must be one of: In, NotIn, Exists, DoesNotExist", i, j, expr.Operator)) + continue + } + + // Validate values based on operator + if expr.Operator == "In" || expr.Operator == "NotIn" { + if len(expr.Values) == 0 { + errs = appendErrors(errs, fmt.Errorf("export_to_selectors[%d].matchExpressions[%d]: values array must be non-empty for operator %q", i, j, expr.Operator)) + } + } else if expr.Operator == "Exists" || expr.Operator == "DoesNotExist" { + if len(expr.Values) > 0 { + errs = appendErrors(errs, fmt.Errorf("export_to_selectors[%d].matchExpressions[%d]: values array must be empty for operator %q", i, j, expr.Operator)) + } + } + } + } + + return errs +} + func ValidateAlphaWorkloadSelector(selector *networking.WorkloadSelector) (Warning, error) { var errs error var warning Warning @@ -1815,6 +1889,7 @@ var ValidateVirtualService = RegisterValidateFunc("ValidateVirtualService", } errs = AppendValidation(errs, validateExportTo(cfg.Namespace, virtualService.ExportTo, false, false)) + errs = AppendValidation(errs, validateExportToSelectors(cfg.Namespace, virtualService.ExportTo, virtualService.ExportToSelectors, false)) warnUnused := func(ruleno, reason string) { errs = AppendValidation(errs, WrapWarning(&AnalysisAwareError{ @@ -3095,6 +3170,7 @@ var ValidateServiceEntry = RegisterValidateFunc("ValidateServiceEntry", } errs = AppendValidation(errs, validateExportTo(cfg.Namespace, serviceEntry.ExportTo, true, false)) + errs = AppendValidation(errs, validateExportToSelectors(cfg.Namespace, serviceEntry.ExportTo, serviceEntry.ExportToSelectors, false)) return errs.Unwrap() }) diff --git a/pkg/kube/namespace/exportto_filter.go b/pkg/kube/namespace/exportto_filter.go new file mode 100644 index 000000000000..ba03d83b7a02 --- /dev/null +++ b/pkg/kube/namespace/exportto_filter.go @@ -0,0 +1,195 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package namespace + +import ( + "sync" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + + "istio.io/istio/pkg/kube" + "istio.io/istio/pkg/kube/controllers" + "istio.io/istio/pkg/kube/kclient" + "istio.io/istio/pkg/slices" +) + +// ExportToFilter tracks namespace membership for exportTo selectors. +// It watches namespace changes and caches namespace labels to support +// efficient selector evaluation for exportTo visibility. +type ExportToFilter struct { + mu sync.RWMutex + + // namespaceLabels caches labels for all namespaces + namespaceLabels map[string]map[string]string + + // handlers to notify when namespace labels change + handlers []func(namespaceName string) + + namespaces kclient.Client[*corev1.Namespace] +} + +// NewExportToFilter creates a new ExportToFilter that watches namespace changes +// and caches namespace labels for efficient exportTo selector evaluation. +func NewExportToFilter(namespaces kclient.Client[*corev1.Namespace], stop <-chan struct{}) *ExportToFilter { + f := &ExportToFilter{ + namespaceLabels: make(map[string]map[string]string), + namespaces: namespaces, + } + + // Register event handlers for namespace changes + namespaces.AddEventHandler(controllers.EventHandler[*corev1.Namespace]{ + AddFunc: func(ns *corev1.Namespace) { + f.mu.Lock() + f.namespaceCreatedLocked(ns) + f.mu.Unlock() + f.notifyHandlers(ns.Name) + }, + UpdateFunc: func(oldObj, newObj *corev1.Namespace) { + f.mu.Lock() + labelsChanged := f.namespaceUpdatedLocked(oldObj, newObj) + f.mu.Unlock() + if labelsChanged { + f.notifyHandlers(newObj.Name) + } + }, + DeleteFunc: func(ns *corev1.Namespace) { + f.mu.Lock() + f.namespaceDeletedLocked(ns) + f.mu.Unlock() + f.notifyHandlers(ns.Name) + }, + }) + + // Start namespaces and wait for it to be ready + namespaces.Start(stop) + kube.WaitForCacheSync("exportto filter", stop, namespaces.HasSynced) + + // Initialize the cache with existing namespaces + f.initializeCache() + + return f +} + +// initializeCache populates the namespace labels cache with all existing namespaces +func (f *ExportToFilter) initializeCache() { + f.mu.Lock() + defer f.mu.Unlock() + + namespaceList := f.namespaces.List("", labels.Everything()) + for _, ns := range namespaceList { + f.namespaceLabels[ns.Name] = copyLabels(ns.Labels) + } +} + +// GetNamespaceLabels returns cached labels for a namespace. +// Returns nil if the namespace is not found in the cache. +func (f *ExportToFilter) GetNamespaceLabels(namespace string) map[string]string { + f.mu.RLock() + defer f.mu.RUnlock() + + labels, exists := f.namespaceLabels[namespace] + if !exists { + return nil + } + // Return a copy to prevent external modifications + return copyLabels(labels) +} + +// MatchesSelector checks if a namespace matches a given selector. +// Returns false if the namespace is not found in the cache. +func (f *ExportToFilter) MatchesSelector(namespace string, selector labels.Selector) bool { + f.mu.RLock() + defer f.mu.RUnlock() + + nsLabels, exists := f.namespaceLabels[namespace] + if !exists { + return false + } + + return selector.Matches(labels.Set(nsLabels)) +} + +// AddHandler registers a callback for namespace label changes. +// The handler will be called whenever a namespace is created, updated (if labels changed), or deleted. +func (f *ExportToFilter) AddHandler(handler func(namespaceName string)) { + f.mu.Lock() + defer f.mu.Unlock() + f.handlers = append(f.handlers, handler) +} + +// notifyHandlers calls all registered handlers with the namespace name that changed. +// Handlers are called without holding the lock to prevent deadlocks. +func (f *ExportToFilter) notifyHandlers(namespaceName string) { + // Clone handlers; we handle dynamic handlers so they can change after the filter has started. + // Important: handlers are not called under the lock to prevent deadlocks. + f.mu.RLock() + handlers := slices.Clone(f.handlers) + f.mu.RUnlock() + + for _, h := range handlers { + h(namespaceName) + } +} + +// namespaceCreatedLocked handles a newly created namespace by caching its labels. +// Must be called with lock held. +func (f *ExportToFilter) namespaceCreatedLocked(ns *corev1.Namespace) { + f.namespaceLabels[ns.Name] = copyLabels(ns.Labels) +} + +// namespaceUpdatedLocked handles a namespace update by updating cached labels. +// Returns true if the labels changed. +// Must be called with lock held. +func (f *ExportToFilter) namespaceUpdatedLocked(oldNs, newNs *corev1.Namespace) bool { + // Check if labels actually changed + if labelsEqual(oldNs.Labels, newNs.Labels) { + return false + } + + f.namespaceLabels[newNs.Name] = copyLabels(newNs.Labels) + return true +} + +// namespaceDeletedLocked handles a namespace deletion by removing it from the cache. +// Must be called with lock held. +func (f *ExportToFilter) namespaceDeletedLocked(ns *corev1.Namespace) { + delete(f.namespaceLabels, ns.Name) +} + +// copyLabels creates a deep copy of a label map +func copyLabels(labels map[string]string) map[string]string { + if labels == nil { + return nil + } + result := make(map[string]string, len(labels)) + for k, v := range labels { + result[k] = v + } + return result +} + +// labelsEqual compares two label maps for equality +func labelsEqual(a, b map[string]string) bool { + if len(a) != len(b) { + return false + } + for k, v := range a { + if b[k] != v { + return false + } + } + return true +} diff --git a/pkg/kube/namespace/exportto_filter_example.go b/pkg/kube/namespace/exportto_filter_example.go new file mode 100644 index 000000000000..f11004a9a704 --- /dev/null +++ b/pkg/kube/namespace/exportto_filter_example.go @@ -0,0 +1,91 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package namespace + +// Example usage of ExportToFilter for exportTo label selector support. +// +// This example demonstrates how to use ExportToFilter to track namespace +// label changes and evaluate exportTo selectors for resource visibility. +// +// Basic usage: +// +// // Create the filter +// filter := NewExportToFilter(namespaceClient, stop) +// +// // Register a handler to be notified of namespace changes +// filter.AddHandler(func(namespaceName string) { +// log.Infof("Namespace %s labels changed", namespaceName) +// // Trigger re-evaluation of exportTo visibility for resources +// // that reference this namespace +// }) +// +// // Check if a namespace matches an exportTo selector +// selector, _ := labels.Parse("env=prod,team=platform") +// if filter.MatchesSelector("my-namespace", selector) { +// // Resource is visible in my-namespace +// } +// +// // Get namespace labels for custom evaluation +// labels := filter.GetNamespaceLabels("my-namespace") +// if labels["env"] == "prod" { +// // Do something based on label value +// } +// +// Integration with exportTo: +// +// When a VirtualService, DestinationRule, or ServiceEntry has an exportToSelector, +// the ExportToFilter can be used to determine which namespaces match the selector: +// +// // Example: VirtualService with exportToSelectors +// vs := &v1alpha3.VirtualService{ +// Spec: v1alpha3.VirtualServiceSpec{ +// ExportToSelectors: []*v1alpha1.LabelSelector{ +// { +// MatchLabels: map[string]string{ +// "env": "prod", +// }, +// }, +// }, +// }, +// } +// +// // Convert to k8s label selector +// selector, _ := LabelSelectorAsSelector(vs.Spec.ExportToSelectors[0]) +// +// // Check which namespaces match +// for _, ns := range allNamespaces { +// if filter.MatchesSelector(ns, selector) { +// // VirtualService should be visible in namespace ns +// } +// } +// +// Handling namespace changes: +// +// The filter automatically watches for namespace create/update/delete events +// and triggers registered handlers when namespace labels change. This allows +// the control plane to reactively update resource visibility: +// +// filter.AddHandler(func(namespaceName string) { +// // Re-evaluate all exportTo selectors that might match this namespace +// for _, resource := range resourcesWithExportToSelectors { +// for _, selector := range resource.ExportToSelectors { +// sel, _ := LabelSelectorAsSelector(selector) +// if filter.MatchesSelector(namespaceName, sel) { +// // Push config update to proxies in namespaceName +// pushConfigUpdate(namespaceName, resource) +// } +// } +// } +// }) diff --git a/pkg/kube/namespace/exportto_filter_test.go b/pkg/kube/namespace/exportto_filter_test.go new file mode 100644 index 000000000000..741f7005aa6f --- /dev/null +++ b/pkg/kube/namespace/exportto_filter_test.go @@ -0,0 +1,337 @@ +// Copyright Istio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package namespace + +import ( + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + + "istio.io/istio/pkg/kube" + "istio.io/istio/pkg/kube/kclient" + "istio.io/istio/pkg/test" + "istio.io/istio/pkg/test/util/retry" +) + +func TestExportToFilter(t *testing.T) { + stop := test.NewStop(t) + client := kube.NewFakeClient() + client.RunAndWait(stop) + + // Create the filter + namespaces := kclient.New[*corev1.Namespace](client) + filter := NewExportToFilter(namespaces, stop) + + // Create test namespaces + ns1 := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1", + Labels: map[string]string{ + "env": "prod", + "app": "web", + }, + }, + } + + ns2 := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns2", + Labels: map[string]string{ + "env": "dev", + "app": "api", + }, + }, + } + + // Add namespace 1 + _, err := client.Kube().CoreV1().Namespaces().Create(nil, ns1, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) // Give time for event to propagate + + // Test GetNamespaceLabels + t.Run("GetNamespaceLabels", func(t *testing.T) { + labels := filter.GetNamespaceLabels("ns1") + if labels == nil { + t.Fatal("expected labels for ns1, got nil") + } + if labels["env"] != "prod" { + t.Errorf("expected env=prod, got %s", labels["env"]) + } + if labels["app"] != "web" { + t.Errorf("expected app=web, got %s", labels["app"]) + } + + // Non-existent namespace should return nil + labels = filter.GetNamespaceLabels("nonexistent") + if labels != nil { + t.Errorf("expected nil for nonexistent namespace, got %v", labels) + } + }) + + // Test MatchesSelector + t.Run("MatchesSelector", func(t *testing.T) { + // Create a selector that matches env=prod + selector, err := labels.Parse("env=prod") + if err != nil { + t.Fatal(err) + } + + matches := filter.MatchesSelector("ns1", selector) + if !matches { + t.Error("expected ns1 to match selector env=prod") + } + + // Selector that doesn't match + selector, err = labels.Parse("env=dev") + if err != nil { + t.Fatal(err) + } + + matches = filter.MatchesSelector("ns1", selector) + if matches { + t.Error("expected ns1 to not match selector env=dev") + } + + // Non-existent namespace should not match + matches = filter.MatchesSelector("nonexistent", selector) + if matches { + t.Error("expected nonexistent namespace to not match") + } + }) + + // Test namespace updates + t.Run("NamespaceUpdate", func(t *testing.T) { + handlerCalled := make(chan string, 10) + + filter.AddHandler(func(ns string) { + handlerCalled <- ns + }) + + // Update namespace labels + ns1Updated := ns1.DeepCopy() + ns1Updated.Labels["env"] = "staging" + + _, err := client.Kube().CoreV1().Namespaces().Update(nil, ns1Updated, metav1.UpdateOptions{}) + if err != nil { + t.Fatal(err) + } + + // Handler should have been called + select { + case ns := <-handlerCalled: + if ns != "ns1" { + t.Errorf("expected handler to be called with ns1, got %s", ns) + } + case <-time.After(2 * time.Second): + t.Error("timeout waiting for handler to be called") + } + + // Verify labels were updated + retry.UntilOrFail(t, func() bool { + labels := filter.GetNamespaceLabels("ns1") + return labels != nil && labels["env"] == "staging" + }, retry.Timeout(2*time.Second)) + }) + + // Test namespace creation + t.Run("NamespaceCreation", func(t *testing.T) { + handlerCalled := make(chan string, 10) + + filter.AddHandler(func(ns string) { + handlerCalled <- ns + }) + + // Create namespace 2 + _, err := client.Kube().CoreV1().Namespaces().Create(nil, ns2, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + // Handler should have been called + select { + case ns := <-handlerCalled: + if ns != "ns2" { + t.Errorf("expected handler to be called with ns2, got %s", ns) + } + case <-time.After(2 * time.Second): + t.Error("timeout waiting for handler to be called") + } + + // Verify labels were cached + retry.UntilOrFail(t, func() bool { + labels := filter.GetNamespaceLabels("ns2") + return labels != nil && labels["env"] == "dev" && labels["app"] == "api" + }, retry.Timeout(2*time.Second)) + }) + + // Test namespace deletion + t.Run("NamespaceDeletion", func(t *testing.T) { + handlerCalled := make(chan string, 10) + + filter.AddHandler(func(ns string) { + handlerCalled <- ns + }) + + // Delete namespace 2 + err := client.Kube().CoreV1().Namespaces().Delete(nil, "ns2", metav1.DeleteOptions{}) + if err != nil { + t.Fatal(err) + } + + // Handler should have been called + select { + case ns := <-handlerCalled: + if ns != "ns2" { + t.Errorf("expected handler to be called with ns2, got %s", ns) + } + case <-time.After(2 * time.Second): + t.Error("timeout waiting for handler to be called") + } + + // Verify labels were removed from cache + retry.UntilOrFail(t, func() bool { + labels := filter.GetNamespaceLabels("ns2") + return labels == nil + }, retry.Timeout(2*time.Second)) + }) +} + +func TestExportToFilterConcurrency(t *testing.T) { + stop := test.NewStop(t) + client := kube.NewFakeClient() + client.RunAndWait(stop) + + namespaces := kclient.New[*corev1.Namespace](client) + filter := NewExportToFilter(namespaces, stop) + + // Create a namespace + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ns", + Labels: map[string]string{ + "env": "test", + }, + }, + } + _, err := client.Kube().CoreV1().Namespaces().Create(nil, ns, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + time.Sleep(100 * time.Millisecond) + + // Test concurrent reads and writes + t.Run("ConcurrentAccess", func(t *testing.T) { + done := make(chan struct{}) + + // Concurrent reads + for i := 0; i < 10; i++ { + go func() { + defer func() { done <- struct{}{} }() + for j := 0; j < 100; j++ { + filter.GetNamespaceLabels("test-ns") + selector, _ := labels.Parse("env=test") + filter.MatchesSelector("test-ns", selector) + } + }() + } + + // Concurrent handler registration + for i := 0; i < 5; i++ { + go func() { + defer func() { done <- struct{}{} }() + filter.AddHandler(func(ns string) { + // Empty handler + }) + }() + } + + // Wait for all goroutines + for i := 0; i < 15; i++ { + <-done + } + }) +} + +func TestLabelsCopyAndEquality(t *testing.T) { + t.Run("copyLabels", func(t *testing.T) { + original := map[string]string{ + "key1": "value1", + "key2": "value2", + } + + copy := copyLabels(original) + if copy["key1"] != "value1" { + t.Errorf("expected key1=value1, got %s", copy["key1"]) + } + if copy["key2"] != "value2" { + t.Errorf("expected key2=value2, got %s", copy["key2"]) + } + + // Modify copy should not affect original + copy["key3"] = "value3" + _, exists := original["key3"] + if exists { + t.Error("modifying copy should not affect original") + } + + // Nil input should return nil + nilCopy := copyLabels(nil) + if nilCopy != nil { + t.Errorf("expected nil, got %v", nilCopy) + } + }) + + t.Run("labelsEqual", func(t *testing.T) { + a := map[string]string{ + "key1": "value1", + "key2": "value2", + } + + b := map[string]string{ + "key1": "value1", + "key2": "value2", + } + + if !labelsEqual(a, b) { + t.Error("expected labels to be equal") + } + + // Different values + b["key2"] = "different" + if labelsEqual(a, b) { + t.Error("expected labels to be different") + } + + // Different keys + c := map[string]string{ + "key1": "value1", + } + if labelsEqual(a, c) { + t.Error("expected labels to be different (different keys)") + } + + // Empty maps + d := map[string]string{} + e := map[string]string{} + if !labelsEqual(d, e) { + t.Error("expected empty labels to be equal") + } + }) +}