fix: verify control-plane TLS certificate by default by shreemaan-abhishek · Pull Request #438 · api7/api7-ingress-controller

shreemaan-abhishek · 2026-07-19T08:39:42Z

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
- Updated API and CRD field descriptions to include the default behavior and security warning about disabling TLS verification (including potential man-in-the-middle risk to AdminKey exposure).
Tests
- Added coverage to ensure the translator applies the tlsVerify default and honors explicit true/false values.

coderabbitai · 2026-07-19T08:40:01Z

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: a7f68793-e473-4052-a428-5eed1eea38a7

📥 Commits

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

📒 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

🚧 Files skipped from review as they are similar to previous changes (6)

internal/adc/translator/gatewayproxy.go
config/crd/bases/apisix.apache.org_gatewayproxies.yaml
internal/adc/translator/gatewayproxy_test.go
api/v1alpha1/gatewayproxy_types.go
docs/en/latest/reference/api-reference.md
config/crd-nocel/apisix.apache.org_v2.yaml

📝 Walkthrough

Walkthrough

tlsVerify now defaults to true across the API contract, CRD schemas, documentation, and GatewayProxy translation. Explicit false remains supported and is covered by translator tests.

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 declares a default of `true` and documents the security implications of disabling certificate verification.
Translator default and coverage `internal/adc/translator/gatewayproxy.go`, `internal/adc/translator/gatewayproxy_test.go`	Translation defaults `TlsVerify` to `true`, preserves explicit boolean values, and tests unset, false, and true inputs.

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

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
E2e Test Quality Review	⚠️ Warning	Adds only a unit test; no end-to-end coverage of the GatewayProxy→controller→ADC flow, so the secure-default behavior isn’t validated in a real install.	Add an E2E test that creates a GatewayProxy against a real control-plane endpoint and asserts omitted tlsVerify verifies certs, false opts out, and true keeps verification.

✅ 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 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	TlsVerify now defaults to true in CRD and translator, and executor maps it to tlsSkipVerify=false; no secret logging or auth bypass issues found.

✨ 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 · 2026-07-19T08:53:31Z

conformance test report - apisix-standalone mode

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

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-07-22T10:07:02Z"
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:
    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.

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>

shreemaan-abhishek self-assigned this Jul 20, 2026

shreemaan-abhishek force-pushed the fix/tlsverify-secure-default branch from ae51fe0 to a58e881 Compare July 22, 2026 09:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

shreemaan-abhishek commented Jul 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

shreemaan-abhishek commented Jul 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

conformance test report - apisix-standalone mode

Uh oh!

github-actions Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

conformance test report - apisix mode

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

shreemaan-abhishek commented Jul 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

github-actions Bot commented Jul 19, 2026 •

edited

Loading

github-actions Bot commented Jul 19, 2026 •

edited

Loading