feat: Add auth-proxy sidecar and EventPolicy support to EventTransformer#8883
feat: Add auth-proxy sidecar and EventPolicy support to EventTransformer#8883Arpit529Srivastava wants to merge 3 commits intoknative:mainfrom
Conversation
Signed-off-by: arpit529srivastava <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Arpit529Srivastava The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
knative-prow
bot
added
size/XL
|
Hi @Arpit529Srivastava. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
Signed-off-by: arpit529srivastava <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8883 +/- ##
==========================================
+ Coverage 50.63% 50.83% +0.20%
==========================================
Files 409 409
Lines 21658 21901 +243
==========================================
+ Hits 10966 11133 +167
- Misses 9835 9902 +67
- Partials 857 866 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: arpit529srivastava <[email protected]>
creydr
left a comment
creydr
left a comment
There was a problem hiding this comment.
/ok-to-test
Thanks a lot @Arpit529Srivastava for working on this. This looks very promising already. I left a few comments.
Could you also enable the e2e tests for it? We have for example the authz.AddressableAuthZConformance and oidc.AddressableOIDCConformance tests, which should help a lot on those.
| _, err := r.rolebindingLister.RoleBindings(rb.Namespace).Get(rb.Name) | ||
| if apierrors.IsNotFound(err) { | ||
| return nil | ||
| } | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get rolebinding %s/%s: %w", rb.Namespace, rb.Name, err) | ||
| } |
There was a problem hiding this comment.
wondering, if we need this and if we could directly delete it instead 🤔
There was a problem hiding this comment.
Can you add comments on your new methods, what they are for?
|
|
||
| const testAuthProxyImage = "quay.io/fake-auth-proxy" | ||
|
|
||
| func TestReconcileOIDC(t *testing.T) { |
There was a problem hiding this comment.
can we integrate this in the above test suite?
knative-prow
bot
added
ok-to-test
2 participants
Fixes #8715
Proposed Changes
This pr adds an
auth-proxysidecar toEventTransformdeployments when oidc authentication is enabled, following the same pattern used byIntegrationSink. #8708reconciles a namespace-scoped
RoleBindingso the auth-proxy can readEventPoliciesin the transform’s namespace, and also reconciles an aggregatedRoleBindingin theknative-eventingnamespace to allow access toconfig-featuresandconfig-loggingconfigmaps.sets
status.Address.Audiencewhen oidc is enabled and propagatesAppliedEventPoliciesStatusto reflect whichEventPoliciesapply to theEventTransform.updates service routing to go through the auth-proxy ports (
3128/3129) when oidc is enabled.Pre-review Checklist
Release Note
Docs