Skip to content

support per-route authz override - #7623

Open
therealak12 wants to merge 6 commits into
projectcontour:mainfrom
snapp-incubator:ext-authz-for-origin
Open

support per-route authz override#7623
therealak12 wants to merge 6 commits into
projectcontour:mainfrom
snapp-incubator:ext-authz-for-origin

Conversation

@therealak12

@therealak12 therealak12 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

The PR adds a new authzOverride field to HTTPProxy routes, allowing individual routes to use a different external authorization configuration from the virtualhost.

For backward compatibility, existing route.authPolicy and virtual-host/global authz behavior are preserved; However, the legacy field is ignored when authzOverride is set.

@therealak12
therealak12 force-pushed the ext-authz-for-origin branch 7 times, most recently from 8d3c18a to ae100e5 Compare July 11, 2026 07:09
@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.62500% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.40%. Comparing base (e21600e) to head (afd98ca).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
cmd/contour/serve.go 0.00% 22 Missing ⚠️
internal/dag/httpproxy_processor.go 76.05% 14 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7623      +/-   ##
==========================================
+ Coverage   82.34%   82.40%   +0.05%     
==========================================
  Files         130      130              
  Lines       15879    15980     +101     
==========================================
+ Hits        13076    13168      +92     
- Misses       2516     2522       +6     
- Partials      287      290       +3     
Files with missing lines Coverage Δ
internal/dag/dag.go 98.44% <ø> (ø)
internal/dag/ingress_processor.go 98.07% <100.00%> (+0.02%) ⬆️
internal/envoy/v3/route.go 93.25% <100.00%> (+0.41%) ⬆️
internal/xdscache/v3/listener.go 92.09% <100.00%> (ø)
internal/dag/httpproxy_processor.go 91.00% <76.05%> (-0.46%) ⬇️
cmd/contour/serve.go 23.72% <0.00%> (-0.18%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@therealak12
therealak12 marked this pull request as ready for review July 11, 2026 08:09
@therealak12
therealak12 requested a review from a team as a code owner July 11, 2026 08:09
@therealak12
therealak12 requested review from sunjayBhatia and tsaarni and removed request for a team July 11, 2026 08:09
@therealak12
therealak12 force-pushed the ext-authz-for-origin branch from 0e8ff8c to 7dcfe20 Compare July 11, 2026 09:21
@therealak12

Copy link
Copy Markdown
Contributor Author

IMHO, the test coverage is sufficient. However, I’d appreciate any guidance on how to satisfy Codecov!

The PR is also ready for review.

@tsaarni

@therealak12

therealak12 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Hi.
Could you please take a look at this PR?
@tsaarni
@sunjayBhatia

@therealak12

Copy link
Copy Markdown
Contributor Author

Sorry for pinging again — I just wanted to follow up on this. 🙏

Would it be possible to at least add a release-note label to this? 😊

Support for per-route authz overrides would be a huge help to us, so we’d really appreciate having this on the radar for an upcoming release.

@tsaarni @sunjayBhatia

@therealak12

Copy link
Copy Markdown
Contributor Author

A true pre-AI-era PR =). Not a single line here was written with the help of an LLM.

Signed-off-by: therealak12 <ak12hastam@gmail.com>
Signed-off-by: therealak12 <ak12hastam@gmail.com>
Signed-off-by: therealak12 <ak12hastam@gmail.com>
Signed-off-by: therealak12 <ak12hastam@gmail.com>
Signed-off-by: therealak12 <ak12hastam@gmail.com>
@therealak12
therealak12 force-pushed the ext-authz-for-origin branch from fb60513 to afd98ca Compare August 9, 2026 09:14
@tsaarni tsaarni added the release-note/minor A minor change that needs about a paragraph of explanation in the release notes. label Aug 11, 2026
Signed-off-by: therealak12 <ak12hastam@gmail.com>
@therealak12

Copy link
Copy Markdown
Contributor Author

I've added support for the headersToAdd field too.

@tsaarni

tsaarni commented Aug 31, 2026

Copy link
Copy Markdown
Member

Hi @therealak12 and sorry for the delay! 👋😊

I have some concerns about reusing the full configuration struct at the route level. Route-level overrides with complete ext_authz configs distribute the settings across multiple locations. I had similar concerns previously for the ext_proc design (#5866) which proposed this same approach.

Could we define a list of named authorization providers at the virtualhost level and reference them by name in individual routes instead? This matches the pattern used in JWT verification.

spec:
  virtualhost:
    fqdn: app.example.com
    tls:
      secretName: app-tls
    # New list of named authorization providers.
    # Deprecates the singular "authorization" field.
    authorizationProviders:
      - name: default-auth
        # Default: true applies to all routes that don't explicitly pick a provider.
        # At most one provider can be marked default.
        default: true
        extensionRef:
          name: default-authz
          namespace: auth
      - name: billing-auth
        extensionRef:
          name: billing-authz
          namespace: billing
        # serviceType defaults to grpc, same as today
      - name: partner-auth
        extensionRef:
          name: partner-authz
          namespace: partners
        serviceType: http
        httpSettings:
          pathPrefix: /check
  routes:
    - conditions:
        - prefix: /billing
      # authPolicy is extended with "require"
      authPolicy:
        # Require picks a named provider from authorizationProviders
        require: billing-auth
        context:
          scope: billing
      services:
        - name: billing-svc
          port: 80
    - conditions:
        - prefix: /public
      authPolicy:
        disabled: true
      services:
        - name: public-svc
          port: 80
    - conditions:
        - prefix: /
      # no authPolicy so it uses default "default-auth" provider automatically
      services:
        - name: app-svc
          port: 80

It should provide the same flexibility while keeping providers and routes defined separately, allowing multiple routes to share a single provider configuration without duplication (by referring to same name).

@therealak12

Copy link
Copy Markdown
Contributor Author

Hi @tsaarni, never mind 🌷.

Thanks for your suggestion. IMO, this is better and helps readability.
I'll apply the change asap.

@therealak12

Copy link
Copy Markdown
Contributor Author

I've added a design doc. We can discuss the design in the Markdown file, and the implementation will be straightforward afterward.

In the meantime, I'm proceeding with the implementation of the proposed design.

@tsaarni

@therealak12

therealak12 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

I think there’s a potential issue with this design.

Per-route JWT provider selection is the model that Envoy exposes.

However, ext authz is modeled differently in Envoy. It has separate configuration support per virtual host and per route.

If we deprecate the existing spec.virtualHost.authorization field, we'll lose some functionality. If we keep it, IMO, the API won’t fully align with Envoy's.
Overall, I prefer the current design, but I'm all ears for your feedback.

@tsaarni

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/minor A minor change that needs about a paragraph of explanation in the release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants