Skip to content

Conversation

@ymesika
Copy link
Contributor

@ymesika ymesika commented Nov 24, 2025

Description

Design for this is available in: #12919 (pending approval)

This PR adds API key authentication support to TrafficPolicies, allowing API keys to be provided via HTTP headers, query parameters, or cookies.

Features:

  • API key sources: header, query parameter, and cookie
  • Secret management via SecretRef or SecretSelector
  • Policy attachment at gateway, HTTPRoute, and route levels
  • Uses Envoy's native envoy.filters.http.api_key_auth filter

Testing:

  • Unit, translator (golden files), and e2e tests
  • E2e tests cover all attachment levels, key sources, and secret update propagation

CI: Added e2e to cluster-five in CI workflow

Fixes #12909

Change Type

/kind feature

Changelog

Add support for configuring an API key authentication in TrafficPolicy with keys defined in secret(s)

Additional Notes

Copilot AI review requested due to automatic review settings November 24, 2025 16:35
@gateway-bot gateway-bot added kind/feature Categorizes issue or PR as related to a new feature. release-note labels Nov 24, 2025
Signed-off-by: Yossi Mesika <[email protected]>
Copy link
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 adds API key authentication support to TrafficPolicy, enabling users to authenticate requests using API keys provided via HTTP headers, query parameters, or cookies. The implementation uses Envoy's native envoy.filters.http.api_key_auth filter and integrates with Kubernetes secrets for key management.

Key Changes:

  • Added APIKeyAuthentication field to TrafficPolicy CRD with support for multiple key sources (header/query/cookie) and flexible secret management
  • Implemented API key auth filter handling in the trafficpolicy plugin following the same pattern as RBAC (disabled by default, configured per-route)
  • Added comprehensive test coverage including unit tests, translator tests (golden files), and e2e tests covering all attachment levels and secret update propagation

Reviewed changes

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

Show a summary per file
File Description
api/v1alpha1/traffic_policy_types.go Defines APIKeyAuthentication and APIKeySource types with validation constraints
api/v1alpha1/zz_generated.deepcopy.go Auto-generated deepcopy methods for new API types
install/helm/kgateway-crds/templates/gateway.kgateway.dev_trafficpolicies.yaml CRD schema definition for API key authentication
internal/kgateway/krtcollections/secrets.go Adds GetSecretCollection method to access secrets without reference grant checks
internal/kgateway/extensions2/plugins/trafficpolicy/api_key_auth.go Core implementation of API key auth translation to Envoy configuration
internal/kgateway/extensions2/plugins/trafficpolicy/api_key_auth_test.go Unit tests for API key auth IR, validation, and filter handling
internal/kgateway/extensions2/plugins/trafficpolicy/traffic_policy_plugin.go Integration of API key auth into trafficpolicy plugin filter chain
internal/kgateway/extensions2/plugins/trafficpolicy/merge.go Merge logic for API key auth policies
internal/kgateway/extensions2/plugins/trafficpolicy/constructor.go Constructor integration for API key auth IR
internal/kgateway/translator/gateway/gateway_translator_test.go Translator tests for API key auth at gateway, HTTPRoute, and route levels
internal/kgateway/translator/gateway/testutils/inputs/traffic-policy/ Golden file test inputs for various API key auth configurations
internal/kgateway/translator/gateway/testutils/outputs/traffic-policy/ Expected golden file outputs for API key auth translation
test/e2e/features/apikeyauth/types.go E2e test suite type definitions and manifest paths
test/e2e/features/apikeyauth/suite.go E2e test implementations covering all key sources and secret updates
test/e2e/features/apikeyauth/testdata/ Test manifests for e2e tests
test/e2e/tests/kgateway_tests.go Registration of APIKeyAuth test suite
.github/workflows/e2e.yaml Added APIKeyAuth tests to cluster-five CI workflow

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

Signed-off-by: Yossi Mesika <[email protected]>
@lgadban lgadban self-requested a review November 25, 2025 04:06
Copy link
Contributor

@lgadban lgadban left a comment

Choose a reason for hiding this comment

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

Can we add translator tests (and maybe augment the e2e test as well) to have a scenario with APIKey policy attached to both Gateway and Route?

IIUC the behavior should be that the route-specific policy is the only one applied

}

// +kubebuilder:validation:ExactlyOneOf=secretRef;secretSelector
type APIKeyAuthentication struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just realized this after the EP has merged, but we don't expose a disable option for APIKey.

We may need to in order to be consistent with our other policies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately the envoy api doesn't have a disabled field like other filter configs.
I think we can still support this by adding config to all routes instead of a higher level one. When it's disabled on a route we will avoid adding it.
Is this an approach that makes sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can do this in a follow up, but we have general pattern we can use to handle global disable for various policies by using Envoy's generic filter disable mechanism and a specifically configured composite filter.

See #12945 for more details on how we did it for JWT.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't manage to add it to this PR. Will open a follow-up an issue for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's be sure to not lose sight of this one

Copy link
Contributor

@lgadban lgadban left a comment

Choose a reason for hiding this comment

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

LGTM!
Approved with a few nits, but I definitely think we need to make the secret collection getter longer and more explicit in a follow up, possibly when we add global disable.

Comment on lines +125 to +126
hasMissingGrants = true
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

the change in behavior makes sense, although I can see this being tricky to debug.

Maybe in a follow up we can add a debug log here that will log when a secret that matches the selector is found but is not allowed to be reference due to a missing RefGrant. That way, with debug logging enabled, a user can understand what is incorrect.

@lgadban
Copy link
Contributor

lgadban commented Dec 8, 2025

thanks!

@lgadban lgadban added this pull request to the merge queue Dec 8, 2025
Merged via the queue into kgateway-dev:main with commit af11f9b Dec 8, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature Categorizes issue or PR as related to a new feature. release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Envoy Dataplane: API Key

3 participants