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 <pierdipi@redhat.com> * Codegen Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com> --------- Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>
pierDipi · Nov 17, 2023 · b0957d5 · b0957d5
1 parent ade9d79
commit b0957d5
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)
 			}