Skip to content

fix: verify control-plane TLS certificate by default#438

Open
shreemaan-abhishek wants to merge 1 commit into
masterfrom
fix/tlsverify-secure-default
Open

fix: verify control-plane TLS certificate by default#438
shreemaan-abhishek wants to merge 1 commit into
masterfrom
fix/tlsverify-secure-default

Conversation

@shreemaan-abhishek

@shreemaan-abhishek shreemaan-abhishek commented Jul 19, 2026

Copy link
Copy Markdown

What this PR does

GatewayProxy spec.provider.controlPlane.tlsVerify is a *bool with no default. When omitted (as the shipped example manifests do, while pointing at https:// Admin API endpoints), it decodes to nil, the translator leaves cfg.TlsVerify at the Go zero value false, and the executor inverts that into tlsSkipVerify: true. The result is that a default install skips certificate verification on the connection that carries the AdminKey and the full gateway config.

This makes verification the secure default:

  • +kubebuilder:default=true on the CRD field, so an omitted tlsVerify becomes true.
  • cfg.TlsVerify = true in the translator before honoring an explicit value, so behavior is correct even against an older CRD without the default. Only an explicit tlsVerify: false opts out.
  • Regenerated CRD manifests (bases + crd-nocel) and API reference docs; documented that setting it to false disables certificate verification.

Testing

Added a translator unit test asserting the default: unset -> verify, explicit false -> skip, explicit true -> verify.

go test ./internal/adc/translator/ -run TestTranslateGatewayProxyToConfig_TlsVerifyDefault

Synced with apache/apisix-ingress-controller#2811.

Summary by CodeRabbit

  • Bug Fixes

    • TLS certificate verification now defaults to enabled when tlsVerify is not specified.
    • Explicitly setting tlsVerify: false continues to disable certificate verification.
  • Documentation

    • Added guidance explaining the security risks of disabling TLS verification, including potential exposure to man-in-the-middle attacks.
    • Documented the default verification behavior in the API reference.

GatewayProxy's controlPlane.tlsVerify is a *bool with no default, so
omitting it left the Go zero value false, which the executor inverts to
tlsSkipVerify:true. A fresh install following the shipped https examples
therefore skipped certificate verification on the channel carrying the
AdminKey.

Default tlsVerify to true via +kubebuilder:default=true and a secure
default in the translator before honoring an explicit override, so only
an explicit tlsVerify:false opts out. Regenerate CRD/docs and add a
translator test locking in the default.

Signed-off-by: Abhishek Choudhary <shreemaan.abhishek@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7b8aa47e-82ef-4e12-a53b-9fba0913fa79

📥 Commits

Reviewing files that changed from the base of the PR and between 5f84a00 and ae51fe0.

📒 Files selected for processing (6)
  • api/v1alpha1/gatewayproxy_types.go
  • config/crd-nocel/apisix.apache.org_v2.yaml
  • config/crd/bases/apisix.apache.org_gatewayproxies.yaml
  • docs/en/latest/reference/api-reference.md
  • internal/adc/translator/gatewayproxy.go
  • internal/adc/translator/gatewayproxy_test.go

📝 Walkthrough

Walkthrough

tlsVerify now defaults to true in the API contract, CRD schemas, documentation, and GatewayProxy translation. Explicit false remains supported, and translator tests cover unset, false, and true values.

Changes

TLS verification default

Layer / File(s) Summary
TLS verification API contract
api/v1alpha1/gatewayproxy_types.go, config/crd..., docs/en/latest/reference/api-reference.md
The tlsVerify field now declares a default of true and documents the certificate verification and security implications of setting it to false.
Translator default and coverage
internal/adc/translator/gatewayproxy.go, internal/adc/translator/gatewayproxy_test.go
Translation defaults TlsVerify to true, while explicit boolean values override the default and are covered by table-driven tests.

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

Suggested reviewers: alinsran

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review ⚠️ Warning The PR adds only a translator unit test; no E2E covers GatewayProxy defaulting through a live control-plane flow, so the key business path is unverified. Add an E2E GatewayProxy case that creates the CR without tlsVerify and verifies the live control-plane connection defaults to TLS verification, plus explicit false/true coverage.
✅ 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 accurately summarizes the main change: enabling control-plane TLS certificate verification by default.
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 No security issues found; tlsVerify now defaults securely to true and explicit false remains the only opt-out.
✨ 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/tlsverify-secure-default

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

@github-actions

Copy link
Copy Markdown
Contributor

conformance test report - apisix-standalone mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-07-19T08:53:29Z"
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.

@github-actions

Copy link
Copy Markdown
Contributor

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-07-19T08:54:30Z"
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:
    - 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.
- 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant