Support exposing the Audience of a Broker #3600

gunishmatta · 2024-01-14T19:32:58Z

Fixes #3520

Proposed Changes

Support exposing the Audience of a Broker based on flag IsOIDCAuthentication

linux-foundation-easycla · 2024-01-14T19:33:01Z

The committers listed above are authorized under a signed CLA.

✅ login: gunishmatta / name: Gunish Matta (7706986, ce1750c, b6e5044, e14fab6, 6083dbf, bfb297d)
✅ login: creydr / name: Christoph Stäbler (6d9b33d, 5f7bd8b, fefdc80)

knative-prow · 2024-01-14T19:33:06Z

Welcome @gunishmatta! It looks like this is your first PR to knative-extensions/eventing-kafka-broker 🎉

knative-prow · 2024-01-14T19:33:07Z

Hi @gunishmatta. Thanks for your PR.

I'm waiting for a knative-extensions member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

pierDipi · 2024-01-15T08:43:25Z

/cc @creydr @Leo6Leo

pierDipi · 2024-01-15T08:44:20Z

Thanks for the PR @gunishmatta, please check linter errors https://github.com/knative-extensions/eventing-kafka-broker/actions/runs/7521296763/job/20481144990?pr=3600

gunishmatta · 2024-01-15T09:40:48Z

@pierDipi I have fixed the formatting issues, can you please approve the workflow run?

creydr · 2024-01-15T12:19:38Z

/ok-to-test

creydr

Hello @gunishmatta,
thanks for working on this. Can you also add a test in

eventing-kafka-broker/control-plane/pkg/reconciler/broker/broker_test.go

Line 132 in b6e5044

func brokerReconciliation(t *testing.T, format string, env config.Env) {

for this behavior?

You can check on knative/eventing#7237 too, how this was done for the Broker in eventing-core.

Leo6Leo · 2024-01-15T14:33:11Z

@gunishmatta Thanks for the PR! As we can see from the error log of the failing tests, we can see there problems with your code. Hope this information give you some hint!

containerID: containerd://783e4cd89babfca38b1dec1b2fafc1e65d30ccbe2ca0c444a3f64891c2d6bee3
      image: sha256:47ff6e6f1028017bd4e6fefb9b5046f630f6913d16a7750dbd91da68804e6bb7
      imageID: gcr.io/knative-boskos-80/eventing-kafka-broker-e2e-img/7587/kafka-controller-409c76cb00d7d7c46941c7e4c366e4a6@sha256:cfb93cd819ad68831e609a727e848822f10413ed458f53e58ef473c5e19b215d
      lastState:
        terminated:
          containerID: containerd://783e4cd89babfca38b1dec1b2fafc1e65d30ccbe2ca0c444a3f64891c2d6bee3
          exitCode: 2
          finishedAt: "2024-01-15T12:39:41Z"
          message: "cileKind(0xc00007d4a0, {0x2591bc8, 0xc001805620}, 0xc000fbaa80)\n\tknative.dev/eventing-kafka-broker/control-plane/pkg/reconciler/broker/broker.go:271
            +0x2836\nknative.dev/eventing-kafka-broker/control-plane/pkg/reconciler/broker.(*Reconciler).ReconcileKind.func1()\n\tknative.dev/eventing-kafka-broker/control-plane/pkg/reconciler/broker/broker.go:90
            +0x25\nk8s.io/client-go/util/retry.OnError.func1()\n\tk8s.io/client-go@v0.28.5/util/retry/util.go:51
            +0x30\nk8s.io/apimachinery/pkg/util/wait.runConditionWithCrashProtection(0x2202727?)\n\tk8s.io/apimachinery@v0.28.5/pkg/util/wait/wait.go:145
            +0x3e\nk8s.io/apimachinery/pkg/util/wait.ExponentialBackoff({0x989680,
            0x4014000000000000, 0x3fb999999999999a, 0x4, 0x0}, 0xc000fbab98?)\n\tk8s.io/apimachinery@v0.28.5/pkg/util/wait/backoff.go:461
            +0x5a\nk8s.io/client-go/util/retry.OnError({0x989680, 0x4014000000000000,
            0x3fb999999999999a, 0x4, 0x0}, 0x2202727?, 0x5?)\n\tk8s.io/client-go@v0.28.5/util/retry/util.go:50
            +0xa5\nk8s.io/client-go/util/retry.RetryOnConflict(...)\n\tk8s.io/client-go@v0.28.5/util/retry/util.go:104\nknative.dev/eventing-kafka-broker/control-plane/pkg/reconciler/broker.(*Reconciler).ReconcileKind(0x6?,
            {0x2591bc8?, 0xc001805620?}, 0xc000fbab98?)\n\tknative.dev/eventing-kafka-broker/control-plane/pkg/reconciler/broker/broker.go:89
            +0xb5\nknative.dev/eventing/pkg/client/injection/reconciler/eventing/v1/broker.(*reconcilerImpl).Reconcile(0xc0002654a0,
            {0x2591bc8, 0xc0018055c0}, {0xc0016cbf60, 0xd})\n\tknative.dev/eventing@v0.39.1-0.20240111175334-3cbddd640f53/pkg/client/injection/reconciler/eventing/v1/broker/reconciler.go:251
            +0x5f9\nknative.dev/pkg/controller.(*Impl).processNextWorkItem(0xc0005e8a20)\n\tknative.dev/pkg@v0.0.0-20240111013350-bc230ae58d14/controller/controller.go:542
            +0x4ad\nknative.dev/pkg/controller.(*Impl).RunContext.func3()\n\tknative.dev/pkg@v0.0.0-20240111013350-bc230ae58d14/controller/controller.go:491
            +0x59\ncreated by knative.dev/pkg/controller.(*Impl).RunContext in goroutine
            365\n\tknative.dev/pkg@v0.0.0-20240111013350-bc230ae58d14/controller/controller.go:489
            +0x349\n"
          reason: Error
          startedAt: "2024-01-15T12:39:09Z"

You can get the complete log here. Let us know if you have any other questions!

creydr · 2024-01-19T07:52:41Z

Hello @gunishmatta,
can we help you somehow on this?

gunishmatta · 2024-01-21T11:01:29Z

@creydr sorry was busy with work during the week, will debug it today

…dience-of-broker

gunishmatta · 2024-01-21T13:24:23Z

/retest-required

gunishmatta · 2024-01-21T15:38:00Z

/retest-required

gunishmatta · 2024-01-22T07:19:35Z

Hi Team @pierDipi @creydr , Please give a final review to the PR and provide the feedback.

Thanks a lot

Looking forward to contribute more to knative community.

gunishmatta · 2024-01-23T16:55:18Z

Thanks for the feedback, I will be working on the feedback, you can expect an update within a few days.

…broker into feature/support-expose-audience-of-broker

creydr · 2024-01-29T15:54:55Z

Hey @gunishmatta,
can I help you somehow on this?
I am working on exposing the audience from a KafkaChannel in #3622. Maybe this helps you a bit.

Two/three main things what I've seen, what's missing:

Setting the audience in broker.Status.Addresses[*].Audience (see Support exposing the Audience of a Broker #3600 (comment))
Reconcile Brokers, if features config map changes (view here for an example, how we do it for a Channel)

optional, but nice to have: add an e2e test to make sure the audience gets populated. We have already a "test set" for it, you need only to call the correct functions. See

eventing-kafka-broker/test/e2e_new_channel/kafka_channel_test.go

Lines 77 to 93 in 3288691

    
           func TestKafkaChannelOIDC(t *testing.T) { 
        
           	// Run Test In Parallel With Others 
        
           	t.Parallel() 
        
           	ctx, env := global.Environment( 
        
           		knative.WithKnativeNamespace(system.Namespace()), 
        
           		knative.WithLoggingConfig, 
        
           		knative.WithTracingConfig, 
        
           		k8s.WithEventListener, 
        
           		environment.WithPollTimings(3*time.Second, 120*time.Second), 
        
           		environment.Managed(t), 
        
           	) 
        
           	name := feature.MakeRandomK8sName("kafkaChannel") 
        
           	env.Prerequisite(ctx, t, channel.ImplGoesReady(name)) 
        
           	env.Test(ctx, t, oidc.AddressableHasAudiencePopulated(kafkachannelresource.GVR(), kafkachannelresource.GVK().Kind, name, env.Namespace()))

and https://github.com/knative/eventing/blob/3391d1ea191dbf3416f1281ceda623412fd32759/test/rekt/broker_test.go#L254-L271 for an example.

control-plane/pkg/reconciler/broker/broker.go

gunishmatta · 2024-01-30T08:18:08Z

HI @creydr very sorry, was occupied with work commitments last week, I will try to get out time and update the PR before this weekend.

Thanks for the support.

Leo6Leo · 2024-02-05T14:35:12Z

@gunishmatta Hey Gunish, are you being blocked by any issue? anything we could help you with on this PR?

creydr · 2024-02-08T09:06:45Z

@gunishmatta Since it seemed you were occupied with other work and there was not much left to do, I added the missing bits from my comment in 6d9b33d (and run gofmt & goimports 5f7bd8b). I hope that's ok.

@Leo6Leo looks OK from my side. Can you recheck (as I committed to this PR now too 😄 )

/assign @Leo6Leo

creydr · 2024-02-08T09:46:22Z

/test channel-reconciler-tests-ssl

creydr · 2024-02-08T10:47:52Z

/test reconciler-tests-namespaced-broker

creydr · 2024-02-08T20:47:14Z

@Leo6Leo can you PTAL

Leo6Leo · 2024-02-08T20:50:34Z

/lgtm

As the comments have been addressed

knative-prow · 2024-02-08T21:27:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: creydr, gunishmatta

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [creydr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

creydr · 2024-02-08T23:03:17Z

/test reconciler-tests

creydr · 2024-02-09T06:46:27Z

/retest-required

creydr · 2024-02-09T08:32:56Z

/test reconciler-tests-namespaced-broker

* Support exposing the Audience of a Broker * fix formatting * fix formatting * test fixes * Populate broker.status.addresses[*].audience field too * Run goimports and gofmt * Fix unit test --------- Co-authored-by: Christoph Stäbler <cstabler@redhat.com>

* Update contract with fields for OIDC information (knative-extensions#3632) * Update contract to include OIDC information * Run hack/update-codegen.sh * Move OIDC SA to egress * Expose OIDC audience of KafkaChannel in its status (knative-extensions#3622) * Provision .status.address.audience and .status.addresses[*].audience in KafkaChannel * Add kafka Channel e2e test to check if audience is provisioned * Run goimport * Update deps * Auto generate Triggers OIDC identity service account and expose in its status (knative-extensions#3604) * Support auto generation of Triggers identity service account and expose in AuthStatus Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * fix unit test Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * make ServiceAccountLister public Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * add oidc unit test Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * fix unit test Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * integrate oidc unit test Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * add some logic to reconcile triggers, if the features config map gets updated Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * fix unit test Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * revert vendor/knative.dev/pkg/webhook/resourcesemantics/defaulting/controller.go Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * fix unit test Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * fix Verify Deps and Codegen test Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * fix unit test and reconcile triggers, in case of the features configmap changes Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> --------- Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> * Create KafkaSources OIDC service account and expose in its status (knative-extensions#3660) * Create KafkaSources OIDC service account and expose in its status * Run goimport * Provision contract with OIDC information (knative-extensions#3646) * Provision contract with OIDC information * Add DLS audience in KafkaChannel CRD * Update KafkaSource to expose its sinks audience in status * Update Trigger test to include OIDC SA in contract * Propagate KafkaSources OIDC serviceAccountName to consumer and consumergroup * Propagate triggerv2s serviceAccountName to consumergroup * Fix unit test * [data-plane] Add the caching for OIDC JWT token to tokenprovider (knative-extensions#3663) * Add the caching to tokenprovider * Add the boiler * Running the codegen * Running the codegen * Revert "Running the codegen" This reverts commit 0ccf69c. * Use constant, set buffer before token expire, remove unnecessary change * Codegen changes * Codegen changes * Update data-plane/core/src/main/java/dev/knative/eventing/kafka/broker/core/oidc/TokenProvider.java Co-authored-by: Christoph Stäbler <cstabler@redhat.com> * Fix the review comments * Run codegen * Codegen changes * Code gen again --------- Co-authored-by: Christoph Stäbler <cstabler@redhat.com> * Receiver reject requests for wrong audience (knative-extensions#3675) * Receiver: reject request for wrong audience * Switch to AuthenticationHandler * Fix "Request has already been read" issue * Change TokenVerifier to an interface * Initialize TokenVerifier in main * Add test for AuthenticationHandler * Only initialize OIDC discovery config in main and create a TokenVerifier per verticle instance. * Rerun hack/update-codegen.sh * Move TokenVerifier setup into setup() to prevent null pointer exception when vertx is null * Update KafkaChannel OIDC e2e tests, to run OIDC conformance tests so the receiver is tested too. * Run OIDC e2e tests as part of the reconciler suite * Fix KafkaChannelOIDC e2e test * Fix lint issue * Address review comments * Dispatcher authenticate requests (knative-extensions#3677) * Change TokenProvider to return future to get a token * Dispatcher add OIDC to token, when target has an audience set * Add e2e test * Support exposing the Audience of a Broker (knative-extensions#3600) * Support exposing the Audience of a Broker * fix formatting * fix formatting * test fixes * Populate broker.status.addresses[*].audience field too * Run goimports and gofmt * Fix unit test --------- Co-authored-by: Christoph Stäbler <cstabler@redhat.com> * Add broker OIDC e2e tests (knative-extensions#3685) * Add broker OIDC e2e tests * Fix broker template to allow TLS & OIDC configuration on dead letter sink * Remove unneeded check when setting broker audience (knative-extensions#3708) * Check status code of OIDC discovery response (knative-extensions#3707) * Check status code of OIDC discovery endpoint * Run update-codegen.sh * Only allow 200 status code on OIDC discovery endpoint * Run update-deps.sh * Add OIDC tests to encryption/auth test suite * run make generate-release * TokenVerifier: execute blocking calls in parallel (knative-extensions#3728) * TokenVerifier: execute blocking calls in parallel * Revert "TokenVerifier: execute blocking calls in parallel" This reverts commit f3dbde9. * Revert: removed changes in contract.pb.go * Cancel receiver pod start on invalid OIDC config only if authentication.oidc is enabled (knative-extensions#3761) * Cancel pod start on invalid OIDC config only if authentication.oidc is enabled * Update namespaced broker to copy features configmap too. * Add unit test for FeaturesConfig class (knative-extensions#3771) * Add unit test for FeaturesConfig class * Update data-plane/core/src/test/java/dev/knative/eventing/kafka/broker/core/features/FeaturesConfigTest.java Co-authored-by: Calum Murray <cmurray@redhat.com> --------- Co-authored-by: Calum Murray <cmurray@redhat.com> * Run make generate-release again --------- Signed-off-by: pingjiang <xiangpingjiang1998@gmail.com> Co-authored-by: cola <45722758+xiangpingjiang@users.noreply.github.com> Co-authored-by: Leo Li <leoli@redhat.com> Co-authored-by: Gunish Matta <33680363+gunishmatta@users.noreply.github.com> Co-authored-by: Partha Ghosh <112557191+parth721@users.noreply.github.com> Co-authored-by: Calum Murray <cmurray@redhat.com>

Support exposing the Audience of a Broker

7706986

knative-prow bot requested review from aliok and pierDipi January 14, 2024 19:33

knative-prow bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 14, 2024

knative-prow bot added the area/control-plane label Jan 14, 2024

knative-prow bot requested review from creydr and Leo6Leo January 15, 2024 08:43

gunishmatta added 2 commits January 15, 2024 15:02

fix formatting

ce1750c

fix formatting

b6e5044

knative-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 15, 2024

creydr reviewed Jan 15, 2024

View reviewed changes

test fixes

e14fab6

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 21, 2024

Merge branch 'knative-extensions:main' into feature/support-expose-au…

6083dbf

…dience-of-broker

Merge branch 'main' of https://github.com/gunishmatta/eventing-kafka-…

bfb297d

…broker into feature/support-expose-audience-of-broker

creydr reviewed Jan 29, 2024

View reviewed changes

control-plane/pkg/reconciler/broker/broker.go Show resolved Hide resolved

creydr mentioned this pull request Feb 1, 2024

Provision contract with OIDC information #3646

Merged

creydr added 3 commits February 8, 2024 09:55

Populate broker.status.addresses[*].audience field too

6d9b33d

Run goimports and gofmt

5f7bd8b

Fix unit test

fefdc80

knative-prow bot assigned Leo6Leo Feb 8, 2024

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 8, 2024

creydr approved these changes Feb 8, 2024

View reviewed changes

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 8, 2024

creydr mentioned this pull request Feb 9, 2024

[data-plane]: Add Broker OIDC e2e tests #3684

Closed

knative-prow bot merged commit d6a383f into knative-extensions:main Feb 9, 2024
37 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support exposing the Audience of a Broker #3600

Support exposing the Audience of a Broker #3600

gunishmatta commented Jan 14, 2024

linux-foundation-easycla bot commented Jan 14, 2024 •

edited

Loading

knative-prow bot commented Jan 14, 2024

knative-prow bot commented Jan 14, 2024

pierDipi commented Jan 15, 2024

pierDipi commented Jan 15, 2024

gunishmatta commented Jan 15, 2024 •

edited

Loading

creydr commented Jan 15, 2024

creydr left a comment

Leo6Leo commented Jan 15, 2024 •

edited

Loading

creydr commented Jan 19, 2024

gunishmatta commented Jan 21, 2024

gunishmatta commented Jan 21, 2024

gunishmatta commented Jan 21, 2024

gunishmatta commented Jan 22, 2024

gunishmatta commented Jan 23, 2024

creydr commented Jan 29, 2024 •

edited

Loading

gunishmatta commented Jan 30, 2024

Leo6Leo commented Feb 5, 2024

creydr commented Feb 8, 2024

creydr commented Feb 8, 2024

creydr commented Feb 8, 2024

creydr commented Feb 8, 2024

Leo6Leo commented Feb 8, 2024

knative-prow bot commented Feb 8, 2024

creydr commented Feb 8, 2024

creydr commented Feb 9, 2024

creydr commented Feb 9, 2024

Support exposing the Audience of a Broker #3600

Support exposing the Audience of a Broker #3600

Conversation

gunishmatta commented Jan 14, 2024

Proposed Changes

linux-foundation-easycla bot commented Jan 14, 2024 • edited Loading

knative-prow bot commented Jan 14, 2024

knative-prow bot commented Jan 14, 2024

pierDipi commented Jan 15, 2024

pierDipi commented Jan 15, 2024

gunishmatta commented Jan 15, 2024 • edited Loading

creydr commented Jan 15, 2024

creydr left a comment

Choose a reason for hiding this comment

Leo6Leo commented Jan 15, 2024 • edited Loading

creydr commented Jan 19, 2024

gunishmatta commented Jan 21, 2024

gunishmatta commented Jan 21, 2024

gunishmatta commented Jan 21, 2024

gunishmatta commented Jan 22, 2024

gunishmatta commented Jan 23, 2024

creydr commented Jan 29, 2024 • edited Loading

gunishmatta commented Jan 30, 2024

Leo6Leo commented Feb 5, 2024

creydr commented Feb 8, 2024

creydr commented Feb 8, 2024

creydr commented Feb 8, 2024

creydr commented Feb 8, 2024

Leo6Leo commented Feb 8, 2024

knative-prow bot commented Feb 8, 2024

creydr commented Feb 8, 2024

creydr commented Feb 9, 2024

creydr commented Feb 9, 2024

linux-foundation-easycla bot commented Jan 14, 2024 •

edited

Loading

gunishmatta commented Jan 15, 2024 •

edited

Loading

Leo6Leo commented Jan 15, 2024 •

edited

Loading

creydr commented Jan 29, 2024 •

edited

Loading