Skip to content

fix(authz): remove list interception from authz webhook #1980

Open
Zaggy21 wants to merge 5 commits into
mainfrom
fix/remove-list-interception-from-authz
Open

fix(authz): remove list interception from authz webhook #1980
Zaggy21 wants to merge 5 commits into
mainfrom
fix/remove-list-interception-from-authz

Conversation

@Zaggy21
Copy link
Copy Markdown
Contributor

@Zaggy21 Zaggy21 commented May 11, 2026

Description

This PR removes the list verb from authorization webhook's match conditions and clarifies denial message for collection operations.

The webhook authorizes access by fetching a specific named resource and checking its greenhouse.sap/owned-by label against the requester's support-group claims. This design is fundamentally incompatible with list operations - sending list requests to the webhook adds pointless round-trips.

What type of PR is this? (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🎨 Style
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build
  • 🔁 CI
  • 📦 Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

Added tests?

  • 👍 yes
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help
  • Separate ticket for tests # (issue/pr)

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Added to documentation?

  • 📜 README.md
  • 🤝 Documentation pages updated
  • 🙅 no documentation needed
  • (if applicable) generated OpenAPI docs for CRD changes

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

Zaggy21 added 2 commits May 11, 2026 12:59
…al message for collection operations

On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
Copilot AI review requested due to automatic review settings May 11, 2026 11:28
@Zaggy21 Zaggy21 requested a review from a team as a code owner May 11, 2026 11:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts the authz webhook to stop intercepting Kubernetes list requests (which the webhook cannot meaningfully authorize with its “fetch-by-name then check owned-by label” approach) and updates the denial messaging/tests accordingly.

Changes:

  • Removed list from webhook CEL matchConditions in the dev-env AuthorizationConfiguration manifests.
  • Updated the webhook denial reason when ResourceAttributes are missing / have no name to mention collection operations and recommend RBAC.
  • Updated the corresponding unit test assertion for the new denial reason text.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
dev-env/webhook/structured-authz-secure.yaml Removes list from webhook verb matchConditions.
dev-env/webhook/structured-authz-insecure.yaml Removes list from webhook verb matchConditions.
cmd/authz/authorization.go Updates denial reason returned when attributes are missing or unnamed.
cmd/authz/authorization_test.go Updates test expectation for the new denial reason.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dev-env/webhook/structured-authz-secure.yaml Outdated
Comment thread dev-env/webhook/structured-authz-insecure.yaml Outdated
Comment thread cmd/authz/authorization.go
Comment thread cmd/authz/authorization_test.go
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
@github-actions github-actions Bot added size/M and removed size/XS labels May 11, 2026
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
Comment thread cmd/authz/authorization.go Outdated
Comment thread cmd/authz/authorization_test.go Outdated
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
@github-actions github-actions Bot added size/S and removed size/M labels May 11, 2026
@Zaggy21 Zaggy21 requested a review from abhijith-darshan May 11, 2026 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants