You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "shreemaan-abhishek (via GitHub)" <gi...@apache.org> on 2026/07/19 08:39:34 UTC
[PR] fix: verify control-plane TLS certificate by default [apisix-ingress-controller]
shreemaan-abhishek opened a new pull request, #2811:
URL: https://github.com/apache/apisix-ingress-controller/pull/2811
## What this PR does
`GatewayProxy` `spec.provider.controlPlane.tlsVerify` is a `*bool` with no default. When omitted (as the shipped `examples/httpbin` 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 manifest 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
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: verify control-plane TLS certificate by default [apisix-ingress-controller]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #2811:
URL: https://github.com/apache/apisix-ingress-controller/pull/2811#discussion_r3654148566
##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -133,7 +133,10 @@ type ControlPlaneProvider struct {
Service *ProviderService `json:"service,omitempty"`
// TlsVerify specifies whether to verify the TLS certificate of the control plane.
+ // Defaults to true. Setting it to false disables certificate verification and
+ // exposes the AdminKey to man-in-the-middle attacks over https endpoints.
// +optional
+ // +kubebuilder:default=true
Review Comment:
Agreed; this is already a concrete regression in the shipped manifests. `examples/httpbin/{quickstart,httproute,ingress,tcproute}.yaml` use the documented `https://127.0.0.1:7443` endpoint without `tlsVerify`, so they stop syncing after this default changes. Since no CA material can be passed to ADC, a private or self-signed CA has no secure migration path. Please add CA support plus an upgrade path and update those examples before flipping the default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: verify control-plane TLS certificate by default [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2811:
URL: https://github.com/apache/apisix-ingress-controller/pull/2811#discussion_r3654031187
##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -133,7 +133,10 @@ type ControlPlaneProvider struct {
Service *ProviderService `json:"service,omitempty"`
// TlsVerify specifies whether to verify the TLS certificate of the control plane.
+ // Defaults to true. Setting it to false disables certificate verification and
+ // exposes the AdminKey to man-in-the-middle attacks over https endpoints.
// +optional
+ // +kubebuilder:default=true
Review Comment:
`ControlPlaneProvider` has no way to supply a CA — only Endpoints/Service/TlsVerify/Auth — and the ADC executor exposes just `TlsSkipVerify` (internal/adc/client/executor.go:81,337). With a self-signed or private-CA control plane, the only escape from verify=true is `tlsVerify: false`.
The likely outcome is that everyone hits the error and pastes `tlsVerify: false` into their manifests, which leaves us worse off than today: the opt-out becomes boilerplate nobody thinks about, and changing the default is no longer available as a lever.
Suggest landing `controlPlane.caBundle` (inline PEM or a Secret ref) first, then flipping the default in a release with an upgrade note.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: verify control-plane TLS certificate by default [apisix-ingress-controller]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #2811:
URL: https://github.com/apache/apisix-ingress-controller/pull/2811#discussion_r4003117612
##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -133,7 +133,10 @@ type ControlPlaneProvider struct {
Service *ProviderService `json:"service,omitempty"`
// TlsVerify specifies whether to verify the TLS certificate of the control plane.
+ // Defaults to true. Setting it to false disables certificate verification and
+ // exposes the AdminKey to man-in-the-middle attacks over https endpoints.
// +optional
+ // +kubebuilder:default=true
Review Comment:
Addressed in the latest push:
- Control-plane CA support has landed in #2826, the ADC-side support has landed in api7/adc#552, and the controller now pins ADC 0.29.0.
- All four self-signed local examples explicitly set tlsVerify: false and label it as a development-only certificate.
- The upgrade guide now explains that CRD defaulting affects existing GatewayProxy objects and documents the system-CA, caCert, and temporary development opt-out migration paths.
- The branch is merged with current master and the focused translator tests pass.
apache/apisix-helm-chart#994 remains the coordinated release-packaging follow-up; no controller release containing this new default exists yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: verify control-plane TLS certificate by default [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2811:
URL: https://github.com/apache/apisix-ingress-controller/pull/2811#discussion_r3654031187
##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -133,7 +133,10 @@ type ControlPlaneProvider struct {
Service *ProviderService `json:"service,omitempty"`
// TlsVerify specifies whether to verify the TLS certificate of the control plane.
+ // Defaults to true. Setting it to false disables certificate verification and
+ // exposes the AdminKey to man-in-the-middle attacks over https endpoints.
// +optional
+ // +kubebuilder:default=true
Review Comment:
Blocking: `ControlPlaneProvider` has no way to supply a CA — only Endpoints/Service/TlsVerify/Auth — and the ADC executor exposes just `TlsSkipVerify` (internal/adc/client/executor.go:81,337). With a self-signed or private-CA control plane, the only escape from verify=true is `tlsVerify: false`.
The likely outcome is that everyone hits the error and pastes `tlsVerify: false` into their manifests, which leaves us worse off than today: the opt-out becomes boilerplate nobody thinks about, and changing the default is no longer available as a lever.
Suggest landing `controlPlane.caBundle` (inline PEM or a Secret ref) first, then flipping the default in a release with an upgrade note.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org