From 77069863ad269ea1f22bfa7fc271b4d4879ba6ad Mon Sep 17 00:00:00 2001 From: gunishmatta Date: Mon, 15 Jan 2024 01:00:00 +0530 Subject: [PATCH 1/7] Support exposing the Audience of a Broker --- control-plane/pkg/reconciler/broker/broker.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/control-plane/pkg/reconciler/broker/broker.go b/control-plane/pkg/reconciler/broker/broker.go index 2433cae62b..161879afe8 100644 --- a/control-plane/pkg/reconciler/broker/broker.go +++ b/control-plane/pkg/reconciler/broker/broker.go @@ -19,6 +19,8 @@ package broker import ( "context" "fmt" + "knative.dev/eventing/pkg/auth" + "knative.dev/pkg/logging" "strings" "time" @@ -260,6 +262,14 @@ func (r *Reconciler) reconcileKind(ctx context.Context, broker *eventing.Broker) addressableStatus.Address = &httpAddress addressableStatus.Addresses = []duckv1.Addressable{httpAddress} } + if feature.FromContext(ctx).IsOIDCAuthentication() { + audience := auth.GetAudience(eventing.SchemeGroupVersion.WithKind("Broker"), broker.ObjectMeta) + logging.FromContext(ctx).Debugw("Setting the brokers audience", zap.String("audience", audience)) + broker.Status.Address.Audience = &audience + } else { + logging.FromContext(ctx).Debug("Clearing the brokers audience as OIDC is not enabled") + broker.Status.Address.Audience = nil + } proberAddressable := prober.ProberAddressable{ AddressStatus: &addressableStatus, From ce1750c212e06e87c3d445a01a9482467418bb6a Mon Sep 17 00:00:00 2001 From: gunishmatta Date: Mon, 15 Jan 2024 15:02:08 +0530 Subject: [PATCH 2/7] fix formatting --- control-plane/pkg/reconciler/broker/broker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control-plane/pkg/reconciler/broker/broker.go b/control-plane/pkg/reconciler/broker/broker.go index 161879afe8..edda648f5c 100644 --- a/control-plane/pkg/reconciler/broker/broker.go +++ b/control-plane/pkg/reconciler/broker/broker.go @@ -19,7 +19,6 @@ package broker import ( "context" "fmt" - "knative.dev/eventing/pkg/auth" "knative.dev/pkg/logging" "strings" "time" @@ -33,6 +32,7 @@ import ( "k8s.io/client-go/util/retry" eventing "knative.dev/eventing/pkg/apis/eventing/v1" "knative.dev/eventing/pkg/apis/feature" + "knative.dev/eventing/pkg/auth" duckv1 "knative.dev/pkg/apis/duck/v1" "knative.dev/pkg/controller" "knative.dev/pkg/network" From b6e5044e8583cdd3cad6795abc4a20bda9acccca Mon Sep 17 00:00:00 2001 From: gunishmatta Date: Mon, 15 Jan 2024 15:09:08 +0530 Subject: [PATCH 3/7] fix formatting --- control-plane/pkg/reconciler/broker/broker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control-plane/pkg/reconciler/broker/broker.go b/control-plane/pkg/reconciler/broker/broker.go index edda648f5c..033ab8b48b 100644 --- a/control-plane/pkg/reconciler/broker/broker.go +++ b/control-plane/pkg/reconciler/broker/broker.go @@ -19,7 +19,6 @@ package broker import ( "context" "fmt" - "knative.dev/pkg/logging" "strings" "time" @@ -35,6 +34,7 @@ import ( "knative.dev/eventing/pkg/auth" duckv1 "knative.dev/pkg/apis/duck/v1" "knative.dev/pkg/controller" + "knative.dev/pkg/logging" "knative.dev/pkg/network" "knative.dev/pkg/reconciler" "knative.dev/pkg/resolver" From e14fab67e3bdd6f799b07bbba2291d1ab2e800d4 Mon Sep 17 00:00:00 2001 From: gunishmatta Date: Sun, 21 Jan 2024 17:54:57 +0530 Subject: [PATCH 4/7] test fixes --- control-plane/pkg/reconciler/broker/broker.go | 25 +++-- .../pkg/reconciler/broker/broker_test.go | 104 +++++++++++++++++- 2 files changed, 117 insertions(+), 12 deletions(-) diff --git a/control-plane/pkg/reconciler/broker/broker.go b/control-plane/pkg/reconciler/broker/broker.go index 033ab8b48b..d0b8cc37e3 100644 --- a/control-plane/pkg/reconciler/broker/broker.go +++ b/control-plane/pkg/reconciler/broker/broker.go @@ -19,6 +19,8 @@ package broker import ( "context" "fmt" + "knative.dev/eventing/pkg/auth" + "knative.dev/pkg/logging" "strings" "time" @@ -31,10 +33,8 @@ import ( "k8s.io/client-go/util/retry" eventing "knative.dev/eventing/pkg/apis/eventing/v1" "knative.dev/eventing/pkg/apis/feature" - "knative.dev/eventing/pkg/auth" duckv1 "knative.dev/pkg/apis/duck/v1" "knative.dev/pkg/controller" - "knative.dev/pkg/logging" "knative.dev/pkg/network" "knative.dev/pkg/reconciler" "knative.dev/pkg/resolver" @@ -262,15 +262,6 @@ func (r *Reconciler) reconcileKind(ctx context.Context, broker *eventing.Broker) addressableStatus.Address = &httpAddress addressableStatus.Addresses = []duckv1.Addressable{httpAddress} } - if feature.FromContext(ctx).IsOIDCAuthentication() { - audience := auth.GetAudience(eventing.SchemeGroupVersion.WithKind("Broker"), broker.ObjectMeta) - logging.FromContext(ctx).Debugw("Setting the brokers audience", zap.String("audience", audience)) - broker.Status.Address.Audience = &audience - } else { - logging.FromContext(ctx).Debug("Clearing the brokers audience as OIDC is not enabled") - broker.Status.Address.Audience = nil - } - proberAddressable := prober.ProberAddressable{ AddressStatus: &addressableStatus, ResourceKey: types.NamespacedName{ @@ -287,6 +278,18 @@ func (r *Reconciler) reconcileKind(ctx context.Context, broker *eventing.Broker) broker.Status.Address = addressableStatus.Address broker.Status.Addresses = addressableStatus.Addresses + + if feature.FromContext(ctx).IsOIDCAuthentication() && broker.Status.Address != nil { + audience := auth.GetAudience(eventing.SchemeGroupVersion.WithKind("Broker"), broker.ObjectMeta) + logging.FromContext(ctx).Debugw("Setting the brokers audience", zap.String("audience", audience)) + broker.Status.Address.Audience = &audience + } else { + logging.FromContext(ctx).Debug("Clearing the brokers audience as OIDC is not enabled") + if broker.Status.Address != nil { + broker.Status.Address.Audience = nil + } + } + broker.GetConditionSet().Manage(broker.GetStatus()).MarkTrue(base.ConditionAddressable) return nil diff --git a/control-plane/pkg/reconciler/broker/broker_test.go b/control-plane/pkg/reconciler/broker/broker_test.go index 6c46dd8ee1..ebfd7c9ecf 100644 --- a/control-plane/pkg/reconciler/broker/broker_test.go +++ b/control-plane/pkg/reconciler/broker/broker_test.go @@ -19,6 +19,7 @@ package broker_test // different package name due to import cycles. (broker -> t import ( "context" "fmt" + "knative.dev/eventing/pkg/auth" "net/url" "testing" "text/template" @@ -106,6 +107,10 @@ var ( linear = eventingduck.BackoffPolicyLinear exponential = eventingduck.BackoffPolicyExponential customBrokerTopicTemplate = customTemplate() + brokerAudience = auth.GetAudience(eventing.SchemeGroupVersion.WithKind("Broker"), metav1.ObjectMeta{ + Name: BrokerName, + Namespace: BrokerNamespace, + }) ) var DefaultEnv = &config.Env{ @@ -121,7 +126,6 @@ var DefaultEnv = &config.Env{ func TestBrokerReconciler(t *testing.T) { eventing.RegisterAlternateBrokerConditionSet(base.IngressConditionSet) - t.Parallel() for _, f := range Formats { @@ -2221,6 +2225,104 @@ func brokerReconciliation(t *testing.T, format string, env config.Env) { }), }, }, + { + Name: "Should provision audience if authentication enabled", + Objects: []runtime.Object{ + NewBroker( + WithBrokerConfig(KReference(BrokerConfig(bootstrapServers, 20, 5, + BrokerAuthConfig("secret-1"), + ))), + ), + NewSSLSecret(ConfigMapNamespace, "secret-1"), + BrokerConfig(bootstrapServers, 20, 5, BrokerAuthConfig("secret-1")), + NewConfigMapWithBinaryData(env.DataPlaneConfigMapNamespace, env.ContractConfigMapName, nil), + NewService(), + BrokerReceiverPod(env.SystemNamespace, map[string]string{ + "annotation_to_preserve": "value_to_preserve", + }), + BrokerDispatcherPod(env.SystemNamespace, map[string]string{ + "annotation_to_preserve": "value_to_preserve", + }), + }, + Key: testKey, + WantEvents: []string{ + finalizerUpdatedEvent, + }, + WantUpdates: []clientgotesting.UpdateActionImpl{ + SecretFinalizerUpdate("secret-1", SecretFinalizerName), + ConfigMapUpdate(env.DataPlaneConfigMapNamespace, env.ContractConfigMapName, env.ContractConfigMapFormat, &contract.Contract{ + Resources: []*contract.Resource{ + { + Uid: BrokerUUID, + Topics: []string{BrokerTopic()}, + Ingress: &contract.Ingress{Path: receiver.Path(BrokerNamespace, BrokerName)}, + BootstrapServers: bootstrapServers, + Reference: BrokerReference(), + Auth: &contract.Resource_AuthSecret{ + AuthSecret: &contract.Reference{ + Uuid: SecretUUID, + Namespace: ConfigMapNamespace, + Name: "secret-1", + Version: SecretResourceVersion, + }, + }, + }, + }, + Generation: 1, + }), + BrokerReceiverPodUpdate(env.SystemNamespace, map[string]string{ + base.VolumeGenerationAnnotationKey: "1", + "annotation_to_preserve": "value_to_preserve", + }), + BrokerDispatcherPodUpdate(env.SystemNamespace, map[string]string{ + base.VolumeGenerationAnnotationKey: "1", + "annotation_to_preserve": "value_to_preserve", + }), + }, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers(), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{ + { + Object: NewBroker( + WithBrokerConfig(KReference(BrokerConfig(bootstrapServers, 20, 5, + BrokerAuthConfig("secret-1"), + ))), + reconcilertesting.WithInitBrokerConditions, + StatusBrokerConfigMapUpdatedReady(&env), + StatusBrokerDataPlaneAvailable, + StatusBrokerConfigParsed, + StatusBrokerTopicReady, + BrokerConfigMapAnnotations(), + WithTopicStatusAnnotation(BrokerTopic()), + BrokerConfigMapSecretAnnotation("secret-1"), + BrokerAddressable(&env), + StatusBrokerProbeSucceeded, + WithBrokerAddresses([]duckv1.Addressable{ + { + Name: pointer.String("http"), + URL: brokerAddress, + }, + }), + WithBrokerAddress(duckv1.Addressable{ + Name: pointer.String("http"), + URL: brokerAddress, + Audience: &brokerAudience, + }), + WithBrokerAddessable(), + ), + }, + }, + OtherTestData: map[string]interface{}{ + ExpectedTopicDetail: sarama.TopicDetail{ + NumPartitions: 20, + ReplicationFactor: 5, + }, + }, + Ctx: feature.ToContext(context.Background(), feature.Flags{ + feature.OIDCAuthentication: feature.Enabled, + }), + }, } for i := range table { From 6d9b33d839855e1653c839bc809f7a43a5acc575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Sun, 4 Feb 2024 12:14:42 +0100 Subject: [PATCH 5/7] Populate broker.status.addresses[*].audience field too --- control-plane/pkg/reconciler/broker/broker.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/control-plane/pkg/reconciler/broker/broker.go b/control-plane/pkg/reconciler/broker/broker.go index 4523fb0ccb..9505afa3f4 100644 --- a/control-plane/pkg/reconciler/broker/broker.go +++ b/control-plane/pkg/reconciler/broker/broker.go @@ -288,11 +288,19 @@ func (r *Reconciler) reconcileKind(ctx context.Context, broker *eventing.Broker) audience := auth.GetAudience(eventing.SchemeGroupVersion.WithKind("Broker"), broker.ObjectMeta) logging.FromContext(ctx).Debugw("Setting the brokers audience", zap.String("audience", audience)) broker.Status.Address.Audience = &audience + + for i := range broker.Status.Addresses { + broker.Status.Addresses[i].Audience = &audience + } } else { logging.FromContext(ctx).Debug("Clearing the brokers audience as OIDC is not enabled") if broker.Status.Address != nil { broker.Status.Address.Audience = nil } + + for i := range broker.Status.Addresses { + broker.Status.Addresses[i].Audience = nil + } } broker.GetConditionSet().Manage(broker.GetStatus()).MarkTrue(base.ConditionAddressable) From 5f7bd8bee9537a496a3d2a11f7db7a11daf5d8b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Thu, 8 Feb 2024 09:56:26 +0100 Subject: [PATCH 6/7] Run goimports and gofmt --- control-plane/pkg/reconciler/broker/broker.go | 5 +++-- control-plane/pkg/reconciler/broker/broker_test.go | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/control-plane/pkg/reconciler/broker/broker.go b/control-plane/pkg/reconciler/broker/broker.go index 9505afa3f4..cdf89e1695 100644 --- a/control-plane/pkg/reconciler/broker/broker.go +++ b/control-plane/pkg/reconciler/broker/broker.go @@ -19,11 +19,12 @@ package broker import ( "context" "fmt" - "knative.dev/eventing/pkg/auth" - "knative.dev/pkg/logging" "strings" "time" + "knative.dev/eventing/pkg/auth" + "knative.dev/pkg/logging" + "go.uber.org/zap" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" diff --git a/control-plane/pkg/reconciler/broker/broker_test.go b/control-plane/pkg/reconciler/broker/broker_test.go index ebfd7c9ecf..2d8eaf7863 100644 --- a/control-plane/pkg/reconciler/broker/broker_test.go +++ b/control-plane/pkg/reconciler/broker/broker_test.go @@ -19,11 +19,12 @@ package broker_test // different package name due to import cycles. (broker -> t import ( "context" "fmt" - "knative.dev/eventing/pkg/auth" "net/url" "testing" "text/template" + "knative.dev/eventing/pkg/auth" + "knative.dev/eventing-kafka-broker/control-plane/pkg/counter" "k8s.io/apimachinery/pkg/runtime/schema" From fefdc8096d85526092ce0dd3c3ce39607e564be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Thu, 8 Feb 2024 10:05:51 +0100 Subject: [PATCH 7/7] Fix unit test --- control-plane/pkg/reconciler/broker/broker_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/control-plane/pkg/reconciler/broker/broker_test.go b/control-plane/pkg/reconciler/broker/broker_test.go index 2d8eaf7863..08a1e3a1ac 100644 --- a/control-plane/pkg/reconciler/broker/broker_test.go +++ b/control-plane/pkg/reconciler/broker/broker_test.go @@ -2301,8 +2301,9 @@ func brokerReconciliation(t *testing.T, format string, env config.Env) { StatusBrokerProbeSucceeded, WithBrokerAddresses([]duckv1.Addressable{ { - Name: pointer.String("http"), - URL: brokerAddress, + Name: pointer.String("http"), + URL: brokerAddress, + Audience: &brokerAudience, }, }), WithBrokerAddress(duckv1.Addressable{