From b0957d5f995452f215afdff5fc0785e537e9f2cc Mon Sep 17 00:00:00 2001 From: Pierangelo Di Pilato Date: Thu, 16 Nov 2023 20:48:52 +0100 Subject: [PATCH] Allow configuring whether to allow cross namespaces Brokers configuration references (#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 * Codegen Signed-off-by: Pierangelo Di Pilato --------- Signed-off-by: Pierangelo Di Pilato --- pkg/apis/config/defaults.go | 2 + pkg/apis/config/zz_generated.deepcopy.go | 5 + pkg/apis/eventing/v1/broker_validation.go | 19 +++- .../eventing/v1/broker_validation_test.go | 91 ++++++++++++++++++- 4 files changed, 114 insertions(+), 3 deletions(-) diff --git a/pkg/apis/config/defaults.go b/pkg/apis/config/defaults.go index 368f1be7e6c..f12b83ab534 100644 --- 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 index a57b366a93b..e02665cc874 100644 --- a/pkg/apis/config/zz_generated.deepcopy.go +++ b/pkg/apis/config/zz_generated.deepcopy.go @@ -60,6 +60,11 @@ func (in *ClassAndBrokerConfig) DeepCopyInto(out *ClassAndBrokerConfig) { *out = new(BrokerConfig) (*in).DeepCopyInto(*out) } + if in.DisallowDifferentNamespaceConfig != nil { + in, out := &in.DisallowDifferentNamespaceConfig, &out.DisallowDifferentNamespaceConfig + *out = new(bool) + **out = **in + } return } diff --git a/pkg/apis/eventing/v1/broker_validation.go b/pkg/apis/eventing/v1/broker_validation.go index d6c4180c689..1829872f9ee 100644 --- a/pkg/apis/eventing/v1/broker_validation.go +++ b/pkg/apis/eventing/v1/broker_validation.go @@ -23,6 +23,8 @@ import ( "knative.dev/pkg/apis" "knative.dev/pkg/kmp" + + "knative.dev/eventing/pkg/apis/config" ) const ( @@ -30,7 +32,22 @@ const ( ) 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 index 2b33739b7ea..ae4dd7e25f9 100644 --- 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) }