feat: support plugins field in ApisixConsumer#412
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughApisixConsumerSpec now marks AuthParameter as optional and adds an XValidation rule requiring at least one auth method or a non-empty plugins list. The CRD schema adds a consumer ChangesPlugin Support for ApisixConsumer
Sequence Diagram(s): sequenceDiagram
participant Client
participant Translator
participant AuthSelector
participant SpecPlugins
participant APISIX_Plugins
Client->>Translator: request consumer translation
Translator->>AuthSelector: select auth plugin(s) from authParameter
AuthSelector->>APISIX_Plugins: insert auth-derived plugin(s)
Translator->>SpecPlugins: iterate enabled consumer plugins
SpecPlugins->>Translator: buildPluginConfig(..., tctx.Secrets)
Translator->>APISIX_Plugins: insert/overwrite by plugin.Name
APISIX_Plugins->>Translator: final plugins set used for Consumer
🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Pull request overview
This PR extends the ApisixConsumer v2 API to support a generic plugins field so consumer-scoped APISIX plugins can be attached directly to an ApisixConsumer, in addition to (or instead of) the existing authParameter mechanism.
Changes:
- Updates
ApisixConsumerSpecto makeauthParameteroptional (pointer) and addsplugins []ApisixRoutePlugin, with an intended CEL constraint requiring at least one of them. - Extends the ADC translator to merge enabled
spec.pluginsinto the translated consumer plugin map (withpluginsoverriding auth plugin name collisions). - Updates the v1 webhook warning collector and tests to handle
authParameterbeing optional; adds a plugins-only admission warning test.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
api/v2/apisixconsumer_types.go |
Makes authParameter optional, adds plugins, and attempts to enforce “authParameter or plugins required” via XValidation. |
api/v2/zz_generated.deepcopy.go |
Regenerates deepcopy logic for the new pointer field and plugins slice. |
internal/adc/translator/apisixconsumer.go |
Adds nil-guard for authParameter and merges enabled generic plugins into the consumer plugin map. |
internal/webhook/v1/apisixconsumer_webhook.go |
Adds nil-guard for authParameter when collecting secret-reference warnings. |
internal/webhook/v1/apisixconsumer_webhook_test.go |
Updates tests for pointer authParameter and adds a plugins-only consumer test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/v2/apisixconsumer_types.go`:
- Around line 25-26: Update the CEL x-validation rule so it checks specific auth
method fields inside authParameter rather than only the presence of
authParameter; replace has(self.authParameter) with a disjunction that checks
has(self.authParameter.basicAuth) || has(self.authParameter.keyAuth) ||
has(self.authParameter.wolfRBAC) || has(self.authParameter.jwtAuth) ||
has(self.authParameter.hmacAuth) || has(self.authParameter.ldapAuth), and keep
the existing plugins clause (has(self.plugins) && size(self.plugins) > 0);
modify the kubebuilder validation annotation that currently references
has(self.authParameter) to use this combined check so empty authParameter
objects are rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54736f2f-5cec-4e5b-8103-a81726a116bc
📒 Files selected for processing (5)
api/v2/apisixconsumer_types.goapi/v2/zz_generated.deepcopy.gointernal/adc/translator/apisixconsumer.gointernal/webhook/v1/apisixconsumer_webhook.gointernal/webhook/v1/apisixconsumer_webhook_test.go
d9a35d7 to
b810022
Compare
b810022 to
6ad3c41
Compare
6ad3c41 to
24d08ad
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/crds/v2/consumer.go (1)
727-779: ⚡ Quick winMake cleanup failure-safe to prevent leaked resources on assertion failures.
Move cleanup registration to immediately after resource creation using
DeferCleanup, so cleanup still runs when an earlier assertion fails.Suggested patch
It("consumer-level limit-count plugin is enforced", func() { By("apply ApisixRoute") applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "plugin-route"}, &apiv2.ApisixRoute{}, fmt.Sprintf(pluginRoute, s.Namespace())) + DeferCleanup(func() { + err := s.DeleteResource("ApisixRoute", "plugin-route") + Expect(err).ShouldNot(HaveOccurred(), "deleting ApisixRoute") + }) By("apply ApisixConsumer with plugins") applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "consumer-with-plugins"}, &apiv2.ApisixConsumer{}, fmt.Sprintf(consumerWithPlugins, s.Namespace())) + DeferCleanup(func() { + err := s.DeleteResource("ApisixConsumer", "consumer-with-plugins") + Expect(err).ShouldNot(HaveOccurred(), "deleting ApisixConsumer") + }) @@ - By("delete ApisixConsumer") - err := s.DeleteResource("ApisixConsumer", "consumer-with-plugins") - Expect(err).ShouldNot(HaveOccurred(), "deleting ApisixConsumer") - - By("delete ApisixRoute") - err = s.DeleteResource("ApisixRoute", "plugin-route") - Expect(err).ShouldNot(HaveOccurred(), "deleting ApisixRoute") }) It("auth plugin configured via plugins field only", func() { By("apply ApisixRoute") applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "plugins-only-route"}, &apiv2.ApisixRoute{}, fmt.Sprintf(pluginsOnlyRoute, s.Namespace())) + DeferCleanup(func() { + err := s.DeleteResource("ApisixRoute", "plugins-only-route") + Expect(err).ShouldNot(HaveOccurred(), "deleting ApisixRoute") + }) By("apply ApisixConsumer with plugins only (no authParameter)") applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "consumer-plugins-only"}, &apiv2.ApisixConsumer{}, fmt.Sprintf(consumerPluginsOnly, s.Namespace())) + DeferCleanup(func() { + err := s.DeleteResource("ApisixConsumer", "consumer-plugins-only") + Expect(err).ShouldNot(HaveOccurred(), "deleting ApisixConsumer") + }) @@ - By("delete ApisixConsumer") - err := s.DeleteResource("ApisixConsumer", "consumer-plugins-only") - Expect(err).ShouldNot(HaveOccurred(), "deleting ApisixConsumer") @@ - By("delete ApisixRoute") - err = s.DeleteResource("ApisixRoute", "plugins-only-route") - Expect(err).ShouldNot(HaveOccurred(), "deleting ApisixRoute") })Also applies to: 822-862
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/crds/v2/consumer.go` around lines 727 - 779, The test registers explicit deletions at the end which won't run if an earlier assertion fails; after each applier.MustApplyAPIv2 call that creates the ApisixRoute ("plugin-route") and the ApisixConsumer ("consumer-with-plugins"), immediately call DeferCleanup to register s.DeleteResource("ApisixConsumer","consumer-with-plugins") and s.DeleteResource("ApisixRoute","plugin-route") (or equivalent cleanup closures) so resources are removed on test teardown; update the same pattern in the other test block (lines around the second occurrence) as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/e2e/crds/v2/consumer.go`:
- Around line 727-779: The test registers explicit deletions at the end which
won't run if an earlier assertion fails; after each applier.MustApplyAPIv2 call
that creates the ApisixRoute ("plugin-route") and the ApisixConsumer
("consumer-with-plugins"), immediately call DeferCleanup to register
s.DeleteResource("ApisixConsumer","consumer-with-plugins") and
s.DeleteResource("ApisixRoute","plugin-route") (or equivalent cleanup closures)
so resources are removed on test teardown; update the same pattern in the other
test block (lines around the second occurrence) as well.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78b1e6e2-99e7-4415-9ab7-c11378b94481
📒 Files selected for processing (5)
api/v2/apisixconsumer_types.goapi/v2/zz_generated.deepcopy.goconfig/crd/bases/apisix.apache.org_apisixconsumers.yamlinternal/adc/translator/apisixconsumer.gotest/e2e/crds/v2/consumer.go
🚧 Files skipped from review as they are similar to previous changes (4)
- api/v2/zz_generated.deepcopy.go
- config/crd/bases/apisix.apache.org_apisixconsumers.yaml
- api/v2/apisixconsumer_types.go
- internal/adc/translator/apisixconsumer.go
conformance test report - apisix-standalone modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-05-11T21:17:18Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
result: partial
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 0
Passed: 32
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests partially succeeded with 1 test skips. Extended tests partially
succeeded with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
conformance test report - apisix modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-05-11T21:17:57Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips.
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
failedTests:
- HTTPRouteInvalidBackendRefUnknownKind
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests partially succeeded
with 1 test skips. |
conformance test reportapiVersion: gateway.networking.k8s.io/v1
date: "2026-05-11T21:37:53Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
failedTests:
- GRPCExactMethodMatching
- GRPCRouteHeaderMatching
- GRPCRouteListenerHostnameMatching
- GatewayModifyListeners
result: failure
statistics:
Failed: 4
Passed: 8
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests failed with 4 test failures.
- core:
failedTests:
- GatewayModifyListeners
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
failedTests:
- HTTPRouteBackendProtocolWebSocket
result: failure
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 1
Passed: 10
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests failed with 1 test
failures.
- core:
failedTests:
- GatewayModifyListeners
- TLSRouteSimpleSameNamespace
result: failure
statistics:
Failed: 2
Passed: 9
Skipped: 0
name: GATEWAY-TLS
summary: Core tests failed with 2 test failures. |
24d08ad to
13443e0
Compare
13443e0 to
4f470a6
Compare
4f470a6 to
483108a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
docs/en/latest/reference/api-reference.md:794
- The docs now state JWT
algorithmcan only beHS256,HS512,RS256, orES256, but the ApisixConsumer API/schema does not enforce an enum and the translator passes the value through unchanged. If this is meant to be a real restriction, add kubebuilder enum validation forApisixConsumerJwtAuthValue.Algorithm; otherwise, keep the docs consistent with the actual accepted values/behavior.
| `key` _string_ | Key is the unique identifier for the JWT credential. |
| `secret` _string_ | Secret is the shared secret used to sign the JWT (for symmetric algorithms). |
| `public_key` _string_ | PublicKey is the public key used to verify JWT signatures (for asymmetric algorithms). |
| `private_key` _string_ | PrivateKey is the private key used to sign the JWT (for asymmetric algorithms). |
| `algorithm` _string_ | Algorithm specifies the signing algorithm. Can be `HS256`, `HS384`, `HS512`, `RS256`, `RS384`, `RS512`, `ES256`, `ES384`, `ES512`, `PS256`, `PS384`, `PS512`, or `EdDSA`. Currently APISIX only supports `HS256`, `HS512`, `RS256`, and `ES256`. API7 Enterprise supports all algorithms. |
| `exp` _integer_ | Exp is the token expiration period in seconds. |
docs/en/latest/reference/api-reference.md:1091
- The
filter_funcfield description no longer mentions the API7 Enterprise limitation, but the source type comment still says "supported in APISIX but not in API7 Enterprise" (seeapi/v2/apisixroute_types.go). Please keep the API reference consistent with the source-of-truth (either update the Go type comment/CRD generation or revert this doc change) to avoid misleading users about support.
| `remoteAddrs` _string array_ | RemoteAddrs is a list of source IP addresses or CIDR ranges to match. Supports both IPv4 and IPv6 formats. |
| `exprs` _[ApisixRouteHTTPMatchExprs](#apisixroutehttpmatchexprs)_ | NginxVars defines match conditions based on Nginx variables. |
| `filter_func` _string_ | FilterFunc is a user-defined function for advanced request filtering. The function can use Nginx variables through the `vars` parameter. This field is supported in APISIX but not in API7 Enterprise. |
config/crd/bases/apisix.apache.org_apisixconsumers.yaml:181
- In the generated CRD, the JWT
algorithmdescription was narrowed toHS256,HS512,RS256, orES256, but the Go API type (ApisixConsumerJwtAuthValue.Algorithm) still documents broader support and there is no enum validation restricting the field. Either add explicit schema validation (enum) and update the Go type comment to match, or revert this description change to avoid drifting generated manifests from the actual API behavior.
properties:
algorithm:
description: |-
Algorithm specifies the signing algorithm.
Can be `HS256`, `HS384`, `HS512`, `RS256`, `RS384`, `RS512`, `ES256`, `ES384`, `ES512`, `PS256`, `PS384`, `PS512`, or `EdDSA`.
Currently APISIX only supports `HS256`, `HS512`, `RS256`, and `ES256`. API7 Enterprise supports all algorithms.
483108a to
5200f36
Compare
5200f36 to
ead00a5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
api/v2/apisixconsumer_validation_test.go:123
- The schema validation tests in this file were updated for the AuthParameter pointer change, but there are no tests covering the new ApisixConsumerSpec XValidation that requires either an auth method or enabled plugins. Adding tests for (1) missing authParameter + empty plugins (should fail), (2) authParameter with no methods (should fail), and (3) plugins-only with an enabled plugin (should pass) would prevent regressions in the CRD/CEL rule.
func TestApisixConsumer_JwtAuth_SymmetricHS256(t *testing.T) {
v := loadApisixConsumerSchema(t)
ac := &apisixv2.ApisixConsumer{
Spec: apisixv2.ApisixConsumerSpec{
AuthParameter: &apisixv2.ApisixConsumerAuthParameter{
JwtAuth: &apisixv2.ApisixConsumerJwtAuth{
Value: &apisixv2.ApisixConsumerJwtAuthValue{
Key: "my-key",
Secret: "my-secret",
Algorithm: "HS256",
},
},
},
},
}
assert.NoError(t, v.Validate(t, ac))
ead00a5 to
c08ac1f
Compare
c08ac1f to
c61f413
Compare
Add a generic `plugins` field to ApisixConsumerSpec so that
consumer-scoped plugins (e.g. limit-count, limit-req) can be
attached to an ApisixConsumer resource directly, without being
limited to the auth plugins exposed through authParameter.
Key changes:
- ApisixConsumerSpec gains a Plugins []ApisixRoutePlugin field,
following the same pattern as ApisixRoute and ApisixGlobalRule.
Plugins entries are merged after the auth plugin derived from
authParameter; a name collision means the Plugins entry wins.
- authParameter is now optional (omitempty). A CEL rule enforces
that at least one auth method within authParameter OR at least
one plugin in plugins must be specified:
has(self.authParameter.basicAuth) || has(self.authParameter.keyAuth)
|| ... || (has(self.plugins) && size(self.plugins) > 0)
- Translator updated to process the new Plugins slice via the
existing buildPluginConfig helper.
- deepcopy updated for the new Plugins slice.
- No webhook changes required; validation is handled entirely
by the CRD-level CEL rule (x-kubernetes-validations).
Closes #397
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c61f413 to
cc4d278
Compare
Summary
Closes #397
Add a generic
pluginsfield toApisixConsumerSpecso that consumer-scoped plugins (e.g.limit-count,limit-req) can be attached to anApisixConsumerresource directly, without being limited to the auth plugins exposed throughauthParameter.Changes
API (
api/v2/apisixconsumer_types.go)authParameteris now optional (omitempty). BothauthParameterandpluginsare optional at the API level; the controller handles the case where neither is set.Plugins []ApisixRoutePluginfield, following the same pattern asApisixRouteandApisixGlobalRule.Translator (
internal/adc/translator/apisixconsumer.go)authParameter, iteratesSpec.Pluginsand merges enabled plugins into the consumer plugin map via the existingbuildPluginConfighelper. An enabled plugin with the same name as the auth plugin derived fromauthParametertakes precedence.Deepcopy (
api/v2/zz_generated.deepcopy.go)ApisixConsumerSpec.DeepCopyIntoupdated to handle the newPluginsslice and the now-optionalAuthParameterpointer.Example
Auth via
authParameter+ extra plugins (existing style, still works)Auth configured directly in
plugins(new style,authParameteromitted)