Skip to content

fix: reject invalid GatewayProxy plugin objects - #483

Open
shreemaan-abhishek wants to merge 1 commit into
masterfrom
fix/gatewayproxy-plugin-render-errors
Open

fix: reject invalid GatewayProxy plugin objects#483
shreemaan-abhishek wants to merge 1 commit into
masterfrom
fix/gatewayproxy-plugin-render-errors

Conversation

@shreemaan-abhishek

@shreemaan-abhishek shreemaan-abhishek commented Sep 16, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Type of change:

  • Bugfix

What this PR does / why we need it:

GatewayProxy plugin configuration and plugin metadata are expected to be JSON objects. When either field had another JSON shape, translation logged the decoding error and continued with partial output.

This change returns descriptive translation errors from both helpers and stages decoded entries before updating the result. Both Gateway and IngressClass reconciliation now stop before submitting incomplete configuration, retaining the previously applied state.

Regression tests cover plugin configuration and plugin metadata through both translation paths.

Part of #454

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

Summary by CodeRabbit

  • Bug Fixes
    • Gateway and IngressClass translation now reports errors from GatewayProxy plugins instead of silently continuing.
    • Invalid plugin configuration or metadata JSON is rejected when it is malformed or not an object.
    • Translation results are no longer produced when plugin rendering fails.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: ccc5428b-3395-4a31-b34c-fff9e1e89bdf

📥 Commits

Reviewing files that changed from the base of the PR and between f68758c and b144347.

📒 Files selected for processing (3)
  • internal/adc/translator/gateway.go
  • internal/adc/translator/gatewayproxy_render_test.go
  • internal/adc/translator/ingressclass.go

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

📝 Walkthrough

Walkthrough

Gateway and IngressClass translation now propagate GatewayProxy errors. Invalid JSON and non-object plugin configuration or metadata cause translation to fail. Results are staged before output maps are updated. Tests cover these error cases.

Changes

GatewayProxy error handling

Layer / File(s) Summary
GatewayProxy validation and staged updates
internal/adc/translator/gateway.go		 Gateway and GatewayProxy helper functions now return errors. Invalid JSON and non-object plugin configuration or metadata are rejected. Translated results are applied only after successful processing.
Caller propagation and regression tests
internal/adc/translator/ingressclass.go, internal/adc/translator/gatewayproxy_render_test.go		 TranslateIngressClass now returns GatewayProxy errors immediately. Tests cover invalid plugin configuration and metadata for Gateway and IngressClass translation.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Suggested reviewers: alinsran

Merge Risk: ⚪ Minimal · up to b1443

Invalid plugin configuration or metadata now stops reconciliation without applying partial configuration.

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review ⚠️ Warning Blocking issue: the PR adds only a unit test in internal/adc/translator/gatewayproxy_render_test.go. The test constructs a TranslateContext in memory and calls TranslateGateway and `TranslateIng… Add E2E tests that create or update a GatewayProxy through Kubernetes with non-object plugin configuration and plugin metadata. Exercise both Gateway and IngressClass reconciliation. Verify that reconciliation reports the translation error,…
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: rejecting invalid GatewayProxy plugin objects during translation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Check ✅ Passed PASS. The pull request does not introduce a security finding in the seven required categories. 1. Sensitive data exposure: No issues found. The existing verbose logs for decoded plugin maps remain unc…
Full details: E2e Test Quality Review

Explanation

Blocking issue: the PR adds only a unit test in internal/adc/translator/gatewayproxy_render_test.go. The test constructs a TranslateContext in memory and calls TranslateGateway and TranslateIngressClass directly. No changed file is under test/e2e, and the existing E2E GatewayProxy test covers only valid plugin configurations. The PR therefore does not test the full Kubernetes API → reconciliation/provider → ADC flow or verify that the previously applied state remains unchanged. The implementation does propagate translation errors and check provider errors correctly.

Resolution

Add E2E tests that create or update a GatewayProxy through Kubernetes with non-object plugin configuration and plugin metadata. Exercise both Gateway and IngressClass reconciliation. Verify that reconciliation reports the translation error, does not submit incomplete ADC configuration, and preserves the previously applied state.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/gatewayproxy-plugin-render-errors

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

conformance test report - apisix-standalone mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-09-16T11:51:40Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
  contact:
  - https://github.com/apache/apisix-ingress-controller/issues
  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:
    - GRPCRouteListenerHostnameMatching
    statistics:
      Failed: 0
      Passed: 14
      Skipped: 1
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 1
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
  name: GATEWAY-GRPC
  summary: Core tests partially succeeded with 1 test skips. Extended tests succeeded.
- core:
    result: partial
    skippedTests:
    - TLSRouteHostnameIntersection
    - TLSRouteInvalidBackendRefNonexistent
    - TLSRouteInvalidBackendRefUnknownKind
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 16
      Skipped: 4
  extended:
    result: partial
    skippedTests:
    - TLSRouteTerminateSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 3
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - TLSRouteModeTerminate
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
    - TLSRouteModeMixed
  name: GATEWAY-TLS
  summary: Core tests partially succeeded with 4 test skips. Extended tests partially
    succeeded with 1 test skips.
- core:
    result: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    - HTTPRouteInvalidBackendRefUnknownKind
    - HTTPRouteInvalidCrossNamespaceBackendRef
    - HTTPRouteInvalidNonExistentBackendRef
    - HTTPRouteListenerHostnameMatching
    - HTTPRouteMultipleGateways
    - HTTPRouteNoBackendRefs
    statistics:
      Failed: 0
      Passed: 30
      Skipped: 7
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - BackendTLSPolicy
    - BackendTLSPolicySANValidation
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRoute303RedirectStatusCode
    - HTTPRoute307RedirectStatusCode
    - HTTPRoute308RedirectStatusCode
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteCORS
    - HTTPRouteNamedRouteRule
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
    - HTTPRouteRetry
    - HTTPRouteRetryBackendTimeout
    - HTTPRouteRetryConnectionError
    - ListenerSet
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 7 test skips. Extended tests partially
    succeeded with 1 test skips.
succeededProvisionalTests:
- GatewayOptionalAddressValue

@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-09-16T11:51:46Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
  contact:
  - https://github.com/apache/apisix-ingress-controller/issues
  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:
    - HTTPRouteHTTPSListener
    - HTTPRouteInvalidBackendRefUnknownKind
    - HTTPRouteInvalidCrossNamespaceBackendRef
    - HTTPRouteInvalidNonExistentBackendRef
    - HTTPRouteListenerHostnameMatching
    - HTTPRouteMultipleGateways
    - HTTPRouteNoBackendRefs
    statistics:
      Failed: 0
      Passed: 30
      Skipped: 7
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - BackendTLSPolicy
    - BackendTLSPolicySANValidation
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRoute303RedirectStatusCode
    - HTTPRoute307RedirectStatusCode
    - HTTPRoute308RedirectStatusCode
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteCORS
    - HTTPRouteNamedRouteRule
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
    - HTTPRouteRetry
    - HTTPRouteRetryBackendTimeout
    - HTTPRouteRetryConnectionError
    - ListenerSet
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 7 test skips. Extended tests partially
    succeeded with 1 test skips.
- core:
    result: partial
    skippedTests:
    - GRPCRouteListenerHostnameMatching
    statistics:
      Failed: 0
      Passed: 14
      Skipped: 1
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 1
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
  name: GATEWAY-GRPC
  summary: Core tests partially succeeded with 1 test skips. Extended tests succeeded.
- core:
    result: partial
    skippedTests:
    - TLSRouteHostnameIntersection
    - TLSRouteInvalidBackendRefNonexistent
    - TLSRouteInvalidBackendRefUnknownKind
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 16
      Skipped: 4
  extended:
    result: partial
    skippedTests:
    - TLSRouteTerminateSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 3
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - TLSRouteModeTerminate
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
    - TLSRouteModeMixed
  name: GATEWAY-TLS
  summary: Core tests partially succeeded with 4 test skips. Extended tests partially
    succeeded with 1 test skips.
succeededProvisionalTests:
- GatewayOptionalAddressValue

@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2026-09-16T12:10:10Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
  contact:
  - https://github.com/apache/apisix-ingress-controller/issues
  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:
    - GatewayModifyListeners
    - HTTPRouteMultipleGateways
    - HTTPRouteNoBackendRefs
    result: failure
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 3
      Passed: 33
      Skipped: 1
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - BackendTLSPolicy
    - BackendTLSPolicySANValidation
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRoute303RedirectStatusCode
    - HTTPRoute307RedirectStatusCode
    - HTTPRoute308RedirectStatusCode
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteCORS
    - HTTPRouteNamedRouteRule
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
    - HTTPRouteRetry
    - HTTPRouteRetryBackendTimeout
    - HTTPRouteRetryConnectionError
    - ListenerSet
  name: GATEWAY-HTTP
  summary: Core tests failed with 3 test failures. Extended tests partially succeeded
    with 1 test skips.
- core:
    failedTests:
    - GatewayModifyListeners
    result: failure
    statistics:
      Failed: 1
      Passed: 14
      Skipped: 0
  extended:
    result: success
    statistics:
      Failed: 0
      Passed: 1
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
  name: GATEWAY-GRPC
  summary: Core tests failed with 1 test failures. Extended tests succeeded.
- core:
    failedTests:
    - GatewayModifyListeners
    - TLSRouteHostnameIntersection
    - TLSRouteInvalidBackendRefNonexistent
    - TLSRouteInvalidBackendRefUnknownKind
    - TLSRouteSimpleSameNamespace
    result: failure
    statistics:
      Failed: 5
      Passed: 15
      Skipped: 0
  extended:
    failedTests:
    - TLSRouteTerminateSimpleSameNamespace
    result: failure
    statistics:
      Failed: 1
      Passed: 3
      Skipped: 0
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - TLSRouteModeTerminate
    unsupportedFeatures:
    - GatewayBackendClientCertificate
    - GatewayFrontendClientCertificateValidation
    - GatewayFrontendClientCertificateValidationInsecureFallback
    - GatewayHTTPListenerIsolation
    - GatewayHTTPSListenerDetectMisdirectedRequests
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - ListenerSet
    - TLSRouteModeMixed
  name: GATEWAY-TLS
  summary: Core tests failed with 5 test failures. Extended tests failed with 1 test
    failures.
succeededProvisionalTests:
- GatewayOptionalAddressValue

@shreemaan-abhishek shreemaan-abhishek self-assigned this Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@shreemaan-abhishek shreemaan-abhishek

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@shreemaan-abhishek