feat: support a CA bundle for the control plane connection - #2826
feat: support a CA bundle for the control plane connection#2826shreemaan-abhishek wants to merge 4 commits into
Conversation
tlsVerify offered only two states: verify against the system trust store, or do not verify at all. A control plane using a self-signed or private-CA certificate has no way to satisfy the first, so the only escape from the connection error is tlsVerify: false -- which turns the insecure opt-out into copy-paste boilerplate. Add the missing third state: an optional PEM-encoded caBundle on GatewayProxy.spec.provider.controlPlane, carried through the translated config to the ADC server, which verifies the control plane against it in place of the system trust store. Unusable CA material is rejected up front -- by a CEL rule at admission and by a PEM parse in the translator -- rather than surfacing later as an opaque TLS failure.
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds support for providing a custom CA bundle for verifying the control plane TLS certificate when using the ControlPlane provider, enabling secure verification for private/self-signed control planes.
Changes:
- Adds
caBundletoGatewayProxy.spec.provider.controlPlanewith admission-time validation. - Validates CA bundle material during translation and propagates it through the ADC request as
caCert. - Updates docs/CRD and adds tests covering translation and request payload behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/adc/translator/gatewayproxy_test.go | New tests validating CA bundle translation behavior and error handling. |
| internal/adc/translator/gatewayproxy.go | Validates caBundle as PEM and copies it into translated ADC config. |
| internal/adc/client/executor_test.go | Tests that caCert is omitted/present in ADC requests based on bundle presence. |
| internal/adc/client/executor.go | Adds caCert to ADC task options and redacts it in logs via hasCaCert. |
| docs/en/latest/reference/api-reference.md | Documents the new caBundle field and its behavior. |
| config/crd/bases/apisix.apache.org_gatewayproxies.yaml | Regenerated CRD schema including caBundle and CEL validation rule. |
| api/v1alpha1/gatewayproxy_types.go | Adds CaBundle field and kubebuilder CEL validation. |
| api/adc/types.go | Adds Config.CaBundle and changes JSON marshaling to report presence. |
Comments suppressed due to low confidence (2)
api/adc/types.go:823
- Adding
hasCaBundletoConfig.MarshalJSONchanges the on-the-wire JSON for every request (it will emit\"hasCaBundle\":falsewhen unset), which undermines the stated wire-compatibility goal and can break older ADC servers if they reject unknown fields. If this field is only intended for logging/redaction, keep the request JSON schema unchanged by removinghasCaBundlefromMarshalJSONand instead expose it via a logging-only method (e.g.,MarshalLog), or at minimum tag it asjson:\"hasCaBundle,omitempty\"and only include it when true and the server schema supports it.
func (c Config) MarshalJSON() ([]byte, error) {
api/adc/types.go:1
- Adding
hasCaBundletoConfig.MarshalJSONchanges the on-the-wire JSON for every request (it will emit\"hasCaBundle\":falsewhen unset), which undermines the stated wire-compatibility goal and can break older ADC servers if they reject unknown fields. If this field is only intended for logging/redaction, keep the request JSON schema unchanged by removinghasCaBundlefromMarshalJSONand instead expose it via a logging-only method (e.g.,MarshalLog), or at minimum tag it asjson:\"hasCaBundle,omitempty\"and only include it when true and the server schema supports it.
// Licensed to the Apache Software Foundation (ASF) under one
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // ControlPlaneProvider defines configuration for control plane provider. | ||
| // +kubebuilder:validation:XValidation:rule="has(self.endpoints) != has(self.service)" | ||
| // +kubebuilder:validation:XValidation:rule="oldSelf == null || (!has(self.mode) && !has(oldSelf.mode)) || self.mode == oldSelf.mode",message="mode is immutable" | ||
| // +kubebuilder:validation:XValidation:rule="!has(self.caBundle) || self.caBundle.contains('-----BEGIN CERTIFICATE-----')",message="caBundle must be a PEM-encoded certificate" |
There was a problem hiding this comment.
This remains valid after the translator parser follow-up: a value containing only -----BEGIN CERTIFICATE----- still passes admission and then fails during reconciliation. Please make the CEL rule require a complete PEM block (at least the BEGIN and END markers) and add admission coverage for truncated input.
x509.CertPool skips PEM blocks it cannot decode, so a bundle whose second certificate is broken passed validation here and failed later at the ADC server, which parses the whole bundle. Reject it up front instead.
|
Pushed a follow-up from the review of the ADC-side PR (api7/adc#537): the CA bundle is now validated by parsing every certificate in it.
Test cases added for a header with no certificate, an unparseable body, a private key in place of a certificate, and one good plus one broken certificate; a multi-certificate bundle is still accepted. |
Both call sites pass http.MethodPut, and the new test made unparam report it. Set the method inside instead of threading it through.
| // CaCert is the PEM-encoded CA certificate (or bundle) the ADC server verifies | ||
| // the control plane against. Older ADC servers ignore it, and omitempty keeps | ||
| // requests without a CA bundle byte for byte what they were. | ||
| CaCert string `json:"caCert,omitempty"` |
There was a problem hiding this comment.
[P1] Ship an ADC version that honors caCert. This repository and its Helm chart still use ADC 0.27.1, while api7/adc#537 is open and unreleased; the current sidecar accepts this unknown option but ignores it. The CRD can therefore be accepted while private-CA verification still fails in every normal install. Please merge and release the ADC change, bump the pinned/chart image, and exercise this TLS path end to end before merging this PR.
| // Set it when the control plane uses a self-signed or private CA certificate. | ||
| // It has no effect when tlsVerify is false. | ||
| // +optional | ||
| CaBundle string `json:"caBundle,omitempty"` |
There was a problem hiding this comment.
[P1] Update the Helm-bundled CRD before exposing this field. apache/apisix-helm-chart still has no caBundle property in charts/apisix-ingress-controller/crds/apisixic-crds.yaml. With that standard installation the API server prunes this unknown field, so the controller never receives it. Please add the paired Helm chart PR and release dependency.
What this PR does
GatewayProxy.spec.provider.controlPlane.tlsVerifyoffers only two states today:tlsVerify: false— no verification.tlsVerify: true— verify against the system trust store only, which works only if the control plane's certificate chains to a publicly trusted CA.There is no way to supply a custom CA, so for the common case of a self-signed or private-CA control plane a user who wants verification on has no path to make it succeed. The only escape from the resulting connection error is
tlsVerify: false— which risks turning the insecure opt-out into copy-paste boilerplate, and undercuts the secure default being introduced in #2811.This adds the missing third state —
tlsVerify: true+ a CA bundle:Raised from the review discussion on #2811 (#2811 (comment)), thanks @AlinsRan.
Design notes
Inline PEM, not a Secret/ConfigMap ref. A CA certificate is public material, so a Secret buys no confidentiality here, and an inline field is what Kubernetes itself uses for the same job (
WebhookClientConfig.CABundle). It also keeps the change to the data path: no new watch, index, or RBAC rule, and rotating the bundle is an edit of the GatewayProxy the controller already reconciles on. AcaBundleRefcan be layered on later without breaking this field.Invalid CA material fails fast, in two places. A CEL rule rejects a non-PEM
caBundleat admission, and the translator parses it withx509.CertPool.AppendCertsFromPEMand returns an error before any config is pushed — so a typo surfaces as a clear message instead of an opaque TLS failure at connect time.Interaction with
tlsVerify. The bundle replaces the system trust store when verification is on, and is ignored when it is off — the controller logs that case rather than silently doing nothing. It is still sent, so flippingtlsVerifyback on needs no other change.Wire compatibility. The bundle reaches the ADC server as
caCertin the task options,omitemptyso that a GatewayProxy without a CA bundle produces byte-for-byte the request an older ADC server already accepts. Logging carries onlyhasCaCert/hasCaBundlebooleans, never the material itself.Dependency
Honoring
caCertrequires the companion ADC change in api7/adc#537 — the ADC server currently picks between two static agents (rejectUnauthorizedtrue/false) and has no way to trust a custom CA. Until an ADC build carrying that change ships, the field is accepted, validated, and transmitted, but not yet acted on by the ADC server. Same change for the enterprise controller: api7/api7-ingress-controller#447 (tracked by api7/api7-ingress-controller#446).Changes
api/v1alpha1/gatewayproxy_types.go:caBundleonControlPlaneProvider, plus the CEL validation rule.api/adc/types.go:Config.CaBundle;MarshalJSONreportshasCaBundlerather than the PEM.internal/adc/translator/gatewayproxy.go: validate the PEM, warn whentlsVerifyis off, set it on the config.internal/adc/client/executor.go: carry it to the ADC server ascaCert.Tests
internal/adc/translator/gatewayproxy_test.go(new): the bundle reachesConfig, stays empty when unset, is rejected when not PEM, and survivestlsVerify: false.internal/adc/client/executor_test.go:caCertis absent from the request body without a bundle and present with one, withtlsSkipVerifystill false.The CEL rule was also exercised against a real API server via envtest (CRDs from
config/crd/bases): a PEM bundle and an absent bundle are admitted,not-a-certificateis rejected withcaBundle must be a PEM-encoded certificate. That check is not committed, since this package has no envtest specs today and adding the first one would makego test ./internal/controllerrequire the kubebuilder assets.