Allow configuring whether to allow cross namespaces Brokers configura…

…tion references (knative#7455) * Allow configuring disallow cross namespaces Brokers configuration ref Instead of always allowing to specify cross namespace configuration references for Broker allow users to configure whether to disallow such references as it might be problematic in multi tenant environments. Signed-off-by: Pierangelo Di Pilato <[email protected]> * Codegen Signed-off-by: Pierangelo Di Pilato <[email protected]> --------- Signed-off-by: Pierangelo Di Pilato <[email protected]>
creydr · Nov 16, 2023 · 32d7dd8 · 32d7dd8
1 parent 203fa93
commit 32d7dd8
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 3 deletions.
diff --git a/pkg/apis/config/defaults.go b/pkg/apis/config/defaults.go
@@ -85,6 +85,8 @@ type Defaults struct {
 type ClassAndBrokerConfig struct {
 	BrokerClass   string `json:"brokerClass,omitempty"`
 	*BrokerConfig `json:",inline"`
+
+	DisallowDifferentNamespaceConfig *bool `json:"disallowDifferentNamespaceConfig,omitempty"`
 }
 
 // BrokerConfig contains configuration for a given namespace for broker. Allows

diff --git a/pkg/apis/config/zz_generated.deepcopy.go b/pkg/apis/config/zz_generated.deepcopy.go
diff --git a/pkg/apis/eventing/v1/broker_validation.go b/pkg/apis/eventing/v1/broker_validation.go
@@ -23,14 +23,31 @@ import (
 
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/kmp"
+
+	"knative.dev/eventing/pkg/apis/config"
 )
 
 const (
 	BrokerClassAnnotationKey = "eventing.knative.dev/broker.class"
 )
 
 func (b *Broker) Validate(ctx context.Context) *apis.FieldError {
-	withNS := apis.AllowDifferentNamespace(apis.WithinParent(ctx, b.ObjectMeta))
+	ctx = apis.WithinParent(ctx, b.ObjectMeta)
+
+	cfg := config.FromContextOrDefaults(ctx)
+	var brConfig *config.ClassAndBrokerConfig
+	if cfg.Defaults != nil {
+		if c, ok := cfg.Defaults.NamespaceDefaultsConfig[b.GetNamespace()]; ok {
+			brConfig = c
+		} else {
+			brConfig = cfg.Defaults.ClusterDefault
+		}
+	}
+
+	withNS := ctx
+	if brConfig == nil || brConfig.DisallowDifferentNamespaceConfig == nil || !*brConfig.DisallowDifferentNamespaceConfig {
+		withNS = apis.AllowDifferentNamespace(ctx)
+	}
 
 	// Make sure a BrokerClassAnnotation exists
 	var errs *apis.FieldError

diff --git a/pkg/apis/eventing/v1/broker_validation_test.go b/pkg/apis/eventing/v1/broker_validation_test.go
@@ -18,13 +18,17 @@ package v1
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1"
+	"k8s.io/utils/pointer"
 	"knative.dev/pkg/apis"
 	duckv1 "knative.dev/pkg/apis/duck/v1"
+
+	"knative.dev/eventing/pkg/apis/config"
+	eventingduckv1 "knative.dev/eventing/pkg/apis/duck/v1"
 )
 
 func TestBrokerImmutableFields(t *testing.T) {
@@ -80,6 +84,7 @@ func TestValidate(t *testing.T) {
 	tests := []struct {
 		name string
 		b    Broker
+		cfg  *config.Config
 		want *apis.FieldError
 	}{{
 		name: "missing annotation",
@@ -187,10 +192,92 @@ func TestValidate(t *testing.T) {
 			},
 		},
 		want: apis.ErrInvalidValue(invalidString, "spec.delivery.backoffDelay"),
+	}, {
+		name: "invalid config, cross namespace disallowed, cluster wide",
+		cfg: &config.Config{
+			Defaults: &config.Defaults{
+				NamespaceDefaultsConfig: nil,
+				ClusterDefault: &config.ClassAndBrokerConfig{
+					DisallowDifferentNamespaceConfig: pointer.Bool(true),
+				},
+			},
+		},
+		b: Broker{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"},
+				Namespace:   "ns1",
+			},
+			Spec: BrokerSpec{
+				Config: &duckv1.KReference{
+					Kind:       "ConfigMap",
+					Namespace:  "ns2",
+					Name:       "cm",
+					APIVersion: "v1",
+				},
+			},
+		},
+		want: &apis.FieldError{
+			Message: "mismatched namespaces",
+			Paths:   []string{"spec.config.namespace"},
+			Details: fmt.Sprintf("parent namespace: %q does not match ref: %q", "ns1", "ns2"),
+		},
+	}, {
+		name: "invalid config, cross namespace disallowed, namespace wide",
+		cfg: &config.Config{
+			Defaults: &config.Defaults{
+				NamespaceDefaultsConfig: map[string]*config.ClassAndBrokerConfig{
+					"ns1": {DisallowDifferentNamespaceConfig: pointer.Bool(true)},
+				},
+			},
+		},
+		b: Broker{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"},
+				Namespace:   "ns1",
+			},
+			Spec: BrokerSpec{
+				Config: &duckv1.KReference{
+					Kind:       "ConfigMap",
+					Namespace:  "ns2",
+					Name:       "cm",
+					APIVersion: "v1",
+				},
+			},
+		},
+		want: &apis.FieldError{
+			Message: "mismatched namespaces",
+			Paths:   []string{"spec.config.namespace"},
+			Details: fmt.Sprintf("parent namespace: %q does not match ref: %q", "ns1", "ns2"),
+		},
+	}, {
+		name: "valid config, cross namespace allowed, namespace wide",
+		cfg: &config.Config{
+			Defaults: &config.Defaults{
+				NamespaceDefaultsConfig: map[string]*config.ClassAndBrokerConfig{
+					"ns1": {DisallowDifferentNamespaceConfig: pointer.Bool(true)},
+				},
+			},
+		},
+		b: Broker{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{"eventing.knative.dev/broker.class": "MTChannelBasedBroker"},
+				Namespace:   "ns2",
+			},
+			Spec: BrokerSpec{
+				Config: &duckv1.KReference{
+					Kind:       "ConfigMap",
+					Namespace:  "ns3",
+					Name:       "cm",
+					APIVersion: "v1",
+				},
+			},
+		},
+		want: nil,
 	}}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			got := test.b.Validate(context.Background())
+			ctx := config.ToContext(context.Background(), test.cfg)
+			got := test.b.Validate(ctx)
 			if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
 				t.Error("Broker.Validate (-want, +got) =", diff)
 			}