From 94dfb4a1caf12252c0fd7575b0011ae380aa2490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Mon, 11 Sep 2023 10:56:17 +0200 Subject: [PATCH] Expose a Brokers audience in its status (#7237) * Provision audience field of Broker * Pass GVK directly to auth.GetAudience() * Add unit test * Add e2e test * Add missing boilerplate --- pkg/apis/eventing/v1/broker_lifecycle.go | 10 ++- pkg/apis/eventing/v1/broker_lifecycle_test.go | 4 +- pkg/apis/eventing/v1/test_helper.go | 8 ++- pkg/auth/audience.go | 32 +++++++++ pkg/auth/audience_test.go | 65 +++++++++++++++++++ .../apiserversource/apiserversource_test.go | 2 +- pkg/reconciler/broker/broker.go | 16 ++++- pkg/reconciler/broker/broker_test.go | 48 ++++++++++++++ pkg/reconciler/testing/v1/broker.go | 12 ++-- test/experimental/auth_test.go | 45 +++++++++++++ test/experimental/config/features.yaml | 1 + test/experimental/features/auth/features.go | 44 +++++++++++++ .../rekt/resources/addressable/addressable.go | 10 +++ 13 files changed, 278 insertions(+), 19 deletions(-) create mode 100644 pkg/auth/audience.go create mode 100644 pkg/auth/audience_test.go create mode 100644 test/experimental/auth_test.go create mode 100644 test/experimental/features/auth/features.go diff --git a/pkg/apis/eventing/v1/broker_lifecycle.go b/pkg/apis/eventing/v1/broker_lifecycle.go index ebe569271ee..9bc846e89e7 100644 --- a/pkg/apis/eventing/v1/broker_lifecycle.go +++ b/pkg/apis/eventing/v1/broker_lifecycle.go @@ -74,16 +74,14 @@ func (bs *BrokerStatus) GetTopLevelCondition() *apis.Condition { // SetAddress makes this Broker addressable by setting the URI. It also // sets the BrokerConditionAddressable to true. -func (bs *BrokerStatus) SetAddress(url *apis.URL) { +func (bs *BrokerStatus) SetAddress(address *v1.Addressable) { bs.AddressStatus = v1.AddressStatus{ - Address: &v1.Addressable{ - URL: url, - }, + Address: address, } - if url != nil { + if address != nil && address.URL != nil { bs.GetConditionSet().Manage(bs).MarkTrue(BrokerConditionAddressable) - bs.AddressStatus.Address.Name = &url.Scheme + bs.AddressStatus.Address.Name = &address.URL.Scheme } else { bs.GetConditionSet().Manage(bs).MarkFalse(BrokerConditionAddressable, "nil URL", "URL is nil") } diff --git a/pkg/apis/eventing/v1/broker_lifecycle_test.go b/pkg/apis/eventing/v1/broker_lifecycle_test.go index 7ce6ce1d79b..df3c3c509bd 100644 --- a/pkg/apis/eventing/v1/broker_lifecycle_test.go +++ b/pkg/apis/eventing/v1/broker_lifecycle_test.go @@ -501,7 +501,9 @@ func TestBrokerIsReady(t *testing.T) { if test.markAddressable == nil && test.address == nil { bs.MarkBrokerAddressableUnknown("", "") } - bs.SetAddress(test.address) + bs.SetAddress(&duckv1.Addressable{ + URL: test.address, + }) b := Broker{Status: bs} got := b.IsReady() diff --git a/pkg/apis/eventing/v1/test_helper.go b/pkg/apis/eventing/v1/test_helper.go index 80b25100091..31c2c51c87c 100644 --- a/pkg/apis/eventing/v1/test_helper.go +++ b/pkg/apis/eventing/v1/test_helper.go @@ -61,7 +61,9 @@ func (t testHelper) ReadyBrokerStatus() *BrokerStatus { bs.PropagateIngressAvailability(t.AvailableEndpoints()) bs.PropagateTriggerChannelReadiness(t.ReadyChannelStatus()) bs.PropagateFilterAvailability(t.AvailableEndpoints()) - bs.SetAddress(apis.HTTP("example.com")) + bs.SetAddress(&duckv1.Addressable{ + URL: apis.HTTP("example.com"), + }) bs.MarkDeadLetterSinkResolvedSucceeded(eventingduckv1.DeliveryStatus{}) return bs } @@ -71,7 +73,9 @@ func (t testHelper) ReadyBrokerStatusWithoutDLS() *BrokerStatus { bs.PropagateIngressAvailability(t.AvailableEndpoints()) bs.PropagateTriggerChannelReadiness(t.ReadyChannelStatus()) bs.PropagateFilterAvailability(t.AvailableEndpoints()) - bs.SetAddress(apis.HTTP("example.com")) + bs.SetAddress(&duckv1.Addressable{ + URL: apis.HTTP("example.com"), + }) bs.MarkDeadLetterSinkNotConfigured() return bs } diff --git a/pkg/auth/audience.go b/pkg/auth/audience.go new file mode 100644 index 00000000000..2147ff0f648 --- /dev/null +++ b/pkg/auth/audience.go @@ -0,0 +1,32 @@ +/* +Copyright 2023 The Knative 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 auth + +import ( + "fmt" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// GetAudience returns the audience string for the given object in the format /// +func GetAudience(gvk schema.GroupVersionKind, objectMeta metav1.ObjectMeta) string { + aud := fmt.Sprintf("%s/%s/%s/%s", gvk.Group, gvk.Kind, objectMeta.Namespace, objectMeta.Name) + + return strings.ToLower(aud) +} diff --git a/pkg/auth/audience_test.go b/pkg/auth/audience_test.go new file mode 100644 index 00000000000..8110b407116 --- /dev/null +++ b/pkg/auth/audience_test.go @@ -0,0 +1,65 @@ +/* +Copyright 2023 The Knative 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 auth + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + v1 "knative.dev/eventing/pkg/apis/eventing/v1" +) + +func TestGetAudience(t *testing.T) { + + tests := []struct { + name string + gvk schema.GroupVersionKind + objectMeta metav1.ObjectMeta + want string + }{ + { + name: "should return audience in correct format", + gvk: schema.GroupVersionKind{ + Group: "group", + Version: "version", + Kind: "kind", + }, + objectMeta: metav1.ObjectMeta{ + Name: "name", + Namespace: "namespace", + }, + want: "group/kind/namespace/name", + }, + { + name: "should return audience in lower case", + gvk: v1.SchemeGroupVersion.WithKind("Broker"), + objectMeta: metav1.ObjectMeta{ + Name: "my-Broker", + Namespace: "my-Namespace", + }, + want: "eventing.knative.dev/broker/my-namespace/my-broker", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := GetAudience(tt.gvk, tt.objectMeta); got != tt.want { + t.Errorf("GetAudience() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/reconciler/apiserversource/apiserversource_test.go b/pkg/reconciler/apiserversource/apiserversource_test.go index a366fcd6671..d7219559b61 100644 --- a/pkg/reconciler/apiserversource/apiserversource_test.go +++ b/pkg/reconciler/apiserversource/apiserversource_test.go @@ -767,7 +767,7 @@ func TestReconcile(t *testing.T) { ), rttestingv1.NewBroker(sinkName, testNS, rttestingv1.WithInitBrokerConditions, - rttestingv1.WithBrokerAddress(sinkDNS), + rttestingv1.WithBrokerAddressURI(apis.HTTP(sinkDNS)), ), makeAvailableReceiveAdapter(t), }, diff --git a/pkg/reconciler/broker/broker.go b/pkg/reconciler/broker/broker.go index 97dde8fed01..4a88ee648bd 100644 --- a/pkg/reconciler/broker/broker.go +++ b/pkg/reconciler/broker/broker.go @@ -47,6 +47,7 @@ import ( eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" "knative.dev/eventing/pkg/apis/feature" messagingv1 "knative.dev/eventing/pkg/apis/messaging/v1" + "knative.dev/eventing/pkg/auth" clientset "knative.dev/eventing/pkg/client/clientset/versioned" brokerreconciler "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1/broker" messaginglisters "knative.dev/eventing/pkg/client/listers/messaging/v1" @@ -192,8 +193,8 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, b *eventingv1.Broker) pk // Route everything to shared ingress, just tack on the namespace/name as path // so we can route there appropriately. - transportEncryptionFlags := feature.FromContext(ctx) - if transportEncryptionFlags.IsPermissiveTransportEncryption() { + featureFlags := feature.FromContext(ctx) + if featureFlags.IsPermissiveTransportEncryption() { caCerts, err := r.getCaCerts() if err != nil { return err @@ -208,7 +209,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, b *eventingv1.Broker) pk // - http address with path-based routing b.Status.Addresses = []pkgduckv1.Addressable{httpsAddress, httpAddress} b.Status.Address = &httpAddress - } else if transportEncryptionFlags.IsStrictTransportEncryption() { + } else if featureFlags.IsStrictTransportEncryption() { // Strict mode: (only https addresses) // - status.address https address with path-based routing // - status.addresses: @@ -226,6 +227,15 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, b *eventingv1.Broker) pk b.Status.Address = &httpAddress } + if featureFlags.IsOIDCAuthentication() { + audience := auth.GetAudience(eventingv1.SchemeGroupVersion.WithKind("Broker"), b.ObjectMeta) + logging.FromContext(ctx).Debugw("Setting the brokers audience", zap.String("audience", audience)) + b.Status.Address.Audience = &audience + } else { + logging.FromContext(ctx).Debug("Clearing the brokers audience as OIDC is not enabled") + b.Status.Address.Audience = nil + } + b.GetConditionSet().Manage(b.GetStatus()).MarkTrue(eventingv1.BrokerConditionAddressable) // So, at this point the Broker is ready and everything should be solid diff --git a/pkg/reconciler/broker/broker_test.go b/pkg/reconciler/broker/broker_test.go index 31983871700..d20d34739a2 100644 --- a/pkg/reconciler/broker/broker_test.go +++ b/pkg/reconciler/broker/broker_test.go @@ -43,7 +43,9 @@ import ( "knative.dev/pkg/tracker" "knative.dev/eventing/pkg/apis/eventing" + eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1" "knative.dev/eventing/pkg/apis/feature" + "knative.dev/eventing/pkg/auth" fakeeventingclient "knative.dev/eventing/pkg/client/injection/client/fake" "knative.dev/eventing/pkg/client/injection/ducks/duck/v1/channelable" "knative.dev/eventing/pkg/client/injection/reconciler/eventing/v1/broker" @@ -92,6 +94,11 @@ var ( Path: fmt.Sprintf("/%s/%s", testNS, brokerName), } + brokerAudience = auth.GetAudience(eventingv1.SchemeGroupVersion.WithKind("Broker"), metav1.ObjectMeta{ + Name: brokerName, + Namespace: testNS, + }) + brokerDestv1 = duckv1.Destination{ Ref: &duckv1.KReference{ Name: sinkName, @@ -703,6 +710,47 @@ func TestReconcile(t *testing.T) { feature.TransportEncryption: feature.Strict, }), }, + { + Name: "Should provision audience if authentication enabled", + Key: testKey, + Objects: []runtime.Object{ + makeDLSServiceAsUnstructured(), + NewBroker(brokerName, testNS, + WithBrokerClass(eventing.MTChannelBrokerClassValue), + WithBrokerConfig(config()), + WithDeadLeaderSink(sinkSVCDest), + WithInitBrokerConditions), + createChannel(withChannelReady, withChannelDeadLetterSink(sinkSVCDest)), + imcConfigMap(), + NewEndpoints(filterServiceName, systemNS, + WithEndpointsLabels(FilterLabels()), + WithEndpointsAddresses(corev1.EndpointAddress{IP: "127.0.0.1"})), + NewEndpoints(ingressServiceName, systemNS, + WithEndpointsLabels(IngressLabels()), + WithEndpointsAddresses(corev1.EndpointAddress{IP: "127.0.0.1"})), + }, + WantErr: false, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: NewBroker(brokerName, testNS, + WithBrokerClass(eventing.MTChannelBrokerClassValue), + WithBrokerConfig(config()), + WithBrokerReadyWithDLS, + WithDeadLeaderSink(sinkSVCDest), + WithBrokerAddress(&duckv1.Addressable{ + URL: brokerAddress, + Audience: &brokerAudience, + }), + WithBrokerStatusDLS(dls), + WithChannelAddressAnnotation(triggerChannelURL), + WithChannelAPIVersionAnnotation(triggerChannelAPIVersion), + WithChannelKindAnnotation(triggerChannelKind), + WithChannelNameAnnotation(triggerChannelName), + ), + }}, + Ctx: feature.ToContext(context.Background(), feature.Flags{ + feature.OIDCAuthentication: feature.Enabled, + }), + }, } logger := logtesting.TestLogger(t) diff --git a/pkg/reconciler/testing/v1/broker.go b/pkg/reconciler/testing/v1/broker.go index 970b9abbd10..3ef470c251a 100644 --- a/pkg/reconciler/testing/v1/broker.go +++ b/pkg/reconciler/testing/v1/broker.go @@ -88,19 +88,19 @@ func WithBrokerConfig(config *duckv1.KReference) BrokerOption { } // WithBrokerAddress sets the Broker's address. -func WithBrokerAddress(address string) BrokerOption { +func WithBrokerAddress(address *duckv1.Addressable) BrokerOption { return func(b *v1.Broker) { - b.Status.SetAddress(&apis.URL{ - Scheme: "http", - Host: address, - }) + b.Status.SetAddress(address) } } // WithBrokerAddressURI sets the Broker's address as URI. func WithBrokerAddressURI(uri *apis.URL) BrokerOption { return func(b *v1.Broker) { - b.Status.SetAddress(uri) + b.Status.SetAddress(&duckv1.Addressable{ + Name: &uri.Scheme, + URL: uri, + }) } } diff --git a/test/experimental/auth_test.go b/test/experimental/auth_test.go new file mode 100644 index 00000000000..1e51c5d29b0 --- /dev/null +++ b/test/experimental/auth_test.go @@ -0,0 +1,45 @@ +//go:build e2e +// +build e2e + +/* +Copyright 2023 The Knative 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 experimental + +import ( + "testing" + + "knative.dev/pkg/system" + "knative.dev/reconciler-test/pkg/environment" + "knative.dev/reconciler-test/pkg/k8s" + "knative.dev/reconciler-test/pkg/knative" + + "knative.dev/eventing/test/experimental/features/auth" +) + +func TestBrokerAudiencePopulated(t *testing.T) { + t.Parallel() + + ctx, env := global.Environment( + knative.WithKnativeNamespace(system.Namespace()), + knative.WithLoggingConfig, + knative.WithTracingConfig, + k8s.WithEventListener, + environment.Managed(t), + ) + + env.Test(ctx, t, auth.BrokerGetsAudiencePopulated(env.Namespace())) +} diff --git a/test/experimental/config/features.yaml b/test/experimental/config/features.yaml index 8968e320835..6c50c90f313 100644 --- a/test/experimental/config/features.yaml +++ b/test/experimental/config/features.yaml @@ -26,3 +26,4 @@ data: delivery-timeout: "enabled" new-trigger-filters: "enabled" eventtype-auto-create: "enabled" + authentication.oidc: "enabled" diff --git a/test/experimental/features/auth/features.go b/test/experimental/features/auth/features.go new file mode 100644 index 00000000000..056e0294eeb --- /dev/null +++ b/test/experimental/features/auth/features.go @@ -0,0 +1,44 @@ +/* +Copyright 2023 The Knative 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 auth + +import ( + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/eventing/pkg/auth" + "knative.dev/eventing/test/rekt/resources/addressable" + "knative.dev/eventing/test/rekt/resources/broker" + "knative.dev/reconciler-test/pkg/feature" +) + +func BrokerGetsAudiencePopulated(namespace string) *feature.Feature { + f := feature.NewFeature() + + brokerName := feature.MakeRandomK8sName("broker") + + f.Setup("install broker", broker.Install(brokerName, broker.WithEnvConfig()...)) + f.Setup("broker is ready", broker.IsReady(brokerName)) + f.Setup("broker is addressable", broker.IsAddressable(brokerName)) + + expectedAudience := auth.GetAudience(broker.GVR().GroupVersion().WithKind("Broker"), v1.ObjectMeta{ + Name: brokerName, + Namespace: namespace, + }) + + f.Alpha("Broker").Must("have audience set", broker.ValidateAddress(brokerName, addressable.AssertAddressWithAudience(expectedAudience))) + + return f +} diff --git a/test/rekt/resources/addressable/addressable.go b/test/rekt/resources/addressable/addressable.go index df3997e261d..0c2877b596c 100644 --- a/test/rekt/resources/addressable/addressable.go +++ b/test/rekt/resources/addressable/addressable.go @@ -61,3 +61,13 @@ func AssertHTTPSAddress(addr *duckv1.Addressable) error { } return nil } + +func AssertAddressWithAudience(audience string) func(*duckv1.Addressable) error { + return func(addressable *duckv1.Addressable) error { + if (addressable.Audience == nil && audience != "") || (addressable.Audience != nil && *addressable.Audience != audience) { + return fmt.Errorf("audience of address (%v) does not match expected audience %s", addressable, audience) + } + + return nil + } +}