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/27 07:18:02 UTC
[PR] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
shreemaan-abhishek opened a new pull request, #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826
## What this PR does
`GatewayProxy.spec.provider.controlPlane.tlsVerify` offers only two states today:
1. `tlsVerify: false` — no verification.
2. `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**:
```yaml
apiVersion: apisix.apache.org/v1alpha1
kind: GatewayProxy
spec:
provider:
type: ControlPlane
controlPlane:
endpoints:
- https://apisix-admin.default.svc:9180
tlsVerify: true
caBundle: |
-----BEGIN CERTIFICATE-----
MIID...
-----END CERTIFICATE-----
auth:
type: AdminKey
adminKey:
valueFrom:
secretKeyRef:
name: admin-key
key: token
```
Raised from the review discussion on #2811 (https://github.com/apache/apisix-ingress-controller/pull/2811#discussion_r3654031187), 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. A `caBundleRef` can be layered on later without breaking this field.
**Invalid CA material fails fast, in two places.** A CEL rule rejects a non-PEM `caBundle` at admission, and the translator parses it with `x509.CertPool.AppendCertsFromPEM` and 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 flipping `tlsVerify` back on needs no other change.
**Wire compatibility.** The bundle reaches the ADC server as `caCert` in the task options, `omitempty` so that a GatewayProxy without a CA bundle produces byte-for-byte the request an older ADC server already accepts. Logging carries only `hasCaCert` / `hasCaBundle` booleans, never the material itself.
## Dependency
Honoring `caCert` requires the companion ADC change in api7/adc#537 — the ADC server currently picks between two static agents (`rejectUnauthorized` true/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#448 (tracked by api7/api7-ingress-controller#446).
## Changes
- `api/v1alpha1/gatewayproxy_types.go`: `caBundle` on `ControlPlaneProvider`, plus the CEL validation rule.
- `api/adc/types.go`: `Config.CaBundle`; `MarshalJSON` reports `hasCaBundle` rather than the PEM.
- `internal/adc/translator/gatewayproxy.go`: validate the PEM, warn when `tlsVerify` is off, set it on the config.
- `internal/adc/client/executor.go`: carry it to the ADC server as `caCert`.
- Regenerated CRD; API reference updated by hand to match.
## Tests
- `internal/adc/translator/gatewayproxy_test.go` (new): the bundle reaches `Config`, stays empty when unset, is rejected when not PEM, and survives `tlsVerify: false`.
- `internal/adc/client/executor_test.go`: `caCert` is absent from the request body without a bundle and present with one, with `tlsSkipVerify` still false.
```
go build ./... ok
go test ./internal/adc/... ./api/... ok
go vet ./api/... ./internal/adc/... ok
```
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-certificate` is rejected with `caBundle 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 make `go test ./internal/controller` require the kubebuilder assets.
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#discussion_r3710370168
##########
internal/adc/client/executor.go:
##########
@@ -84,7 +84,11 @@ type ADCServerOpts struct {
LabelSelector map[string]string `json:"labelSelector,omitempty"`
IncludeResourceType []string `json:"includeResourceType,omitempty"`
TlsSkipVerify *bool `json:"tlsSkipVerify,omitempty"`
- CacheKey string `json:"cacheKey"`
+ // 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"`
Review Comment:
+1
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#issuecomment-5104017714
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.
`x509.CertPool.AppendCertsFromPEM` returns true as long as one block parses and silently skips the rest, so a bundle whose second certificate is broken passed here and then failed at the ADC server — which does parse the whole bundle — as an opaque sync error. That is exactly the late, unclear failure this field was meant to avoid, so the translator now rejects it up front, with the same semantics on both sides.
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.
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#issuecomment-5421085710
Added in `70bc09e3` — `test/e2e/crds/v1alpha1/gatewayproxy_tls.go`, labelled `apisix.apache.org, v1alpha1, gatewayproxy` so it runs in the existing matrix.
The admin API only listens on plain HTTP, so the spec puts an openresty in front of it to stand in for a control plane published over TLS. Its certificate is signed by a CA generated per test, which is exactly the case `caBundle` exists for: nothing in the system trust store can verify it. That image is already pulled and loaded by `make kind-load-images`, so no new dependency.
Three specs:
| spec | asserts |
| --- | --- |
| `syncs to a private-CA control plane when caBundle is trusted` | route is programmed and serves 200 — the sync got through TLS verification |
| `fails to sync when caBundle is missing` | controller logs `unable to verify the first certificate`, and the route is never programmed |
| `rejects a caBundle that is not a certificate` | the API server rejects it via the CEL rule |
Run locally against kind, both provider modes:
```
PROVIDER_TYPE=apisix-standalone make e2e-test TEST_FOCUS='control plane TLS'
Ran 3 of 237 Specs in 97.522 seconds -- SUCCESS! 3 Passed | 0 Failed
PROVIDER_TYPE=apisix make e2e-test TEST_FOCUS='control plane TLS'
Ran 3 of 236 Specs in 115.785 seconds -- SUCCESS! 3 Passed | 0 Failed
```
Writing it turned up something worth flagging separately: the spec generates its certificates locally instead of using `scaffold.GenerateMACert`, because that helper gives the CA and the leaf **the same subject** (`O=Acme Co`, no CN) — `ssl.go:150` and `ssl.go:180`. OpenSSL reads such a leaf as self-issued and fails with `DEPTH_ZERO_SELF_SIGNED_CERT` without ever chaining to the CA. My first run failed on exactly that, with the controller sending everything correctly:
```
"config": {...,"tlsVerify":true,"hasCaBundle":true}
"error": ... HTTP 500: {"message":"Error: self-signed certificate; ..."}
```
It goes unnoticed today because the current callers verify with Go's x509 (webhook `caBundle`) or use the CA only as a client-auth trust anchor, neither of which trips that OpenSSL check. Happy to fix the helper in a follow-up if you'd like — I kept it out of this PR to avoid touching the certificates other specs depend on.
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek merged PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#discussion_r3665203976
##########
internal/adc/translator/gatewayproxy.go:
##########
@@ -56,6 +57,17 @@ func (t *Translator) TranslateGatewayProxyToConfig(tctx *provider.TranslateConte
cfg.TlsVerify = *cp.TlsVerify
}
+ if cp.CaBundle != "" {
+ // reject unusable CA material here rather than at connect time
+ if !x509.NewCertPool().AppendCertsFromPEM([]byte(cp.CaBundle)) {
+ return nil, errors.New("invalid caBundle: no PEM-encoded certificate found")
+ }
Review Comment:
`AppendCertsFromPEM` only validates that *some* certificates can be parsed; it does not ensure they are appropriate CA certificates. If the intent is specifically a CA bundle, consider parsing the certs and rejecting bundles that contain no certs with `IsCA == true` (or otherwise documenting that any certificate can be used as a trust anchor). This reduces configuration pitfalls and aligns behavior with the field description.
##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -120,6 +120,7 @@ type ControlPlaneAuth struct {
// 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"
Review Comment:
The CEL validation rule is very permissive: any string containing `-----BEGIN CERTIFICATE-----` will pass, including non-PEM or truncated values. To better match the error message and intent, consider tightening the rule to require both BEGIN/END markers (and ideally allow surrounding whitespace) using `matches()` with a regex, or at least check for `-----END CERTIFICATE-----` as well.
##########
internal/adc/client/executor_test.go:
##########
@@ -70,6 +70,37 @@ func TestHTTPADCExecutorBuildHTTPRequestBypassCache(t *testing.T) {
assert.NotContains(t, raw, "bypassCache")
}
+func TestHTTPADCExecutorBuildHTTPRequestCaCert(t *testing.T) {
+ e := &HTTPADCExecutor{
+ serverURL: "http://127.0.0.1:3000",
+ log: logr.Discard(),
+ }
+
+ build := func(config adctypes.Config) (ADCServerOpts, string) {
+ req, err := e.buildHTTPRequest(context.Background(), "https://apisix:9180", config, nil, nil,
+ &adctypes.Resources{}, http.MethodPut, pathSync)
+ require.NoError(t, err)
+ body, err := io.ReadAll(req.Body)
+ require.NoError(t, err)
+ var parsed ADCServerRequest
+ require.NoError(t, json.Unmarshal(body, &parsed))
+ return parsed.Task.Opts, string(body)
+ }
+
+ // Without a CA bundle the request stays what an ADC server that predates caCert
+ // already accepts.
+ opts, raw := build(adctypes.Config{Name: "GatewayProxy/ns/name", TlsVerify: true})
+ assert.Empty(t, opts.CaCert)
+ assert.NotContains(t, raw, "caCert")
Review Comment:
Given the compatibility requirement, this test should also assert that the request body does not gain any new fields when no bundle is set (e.g., `hasCaBundle` if present in the serialized `Config`). Adding an explicit `assert.NotContains(t, raw, \"hasCaBundle\")` (or equivalent structural assertion) would prevent accidental wire-format drift.
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#discussion_r3767976233
##########
internal/adc/client/executor.go:
##########
@@ -84,7 +84,11 @@ type ADCServerOpts struct {
LabelSelector map[string]string `json:"labelSelector,omitempty"`
IncludeResourceType []string `json:"includeResourceType,omitempty"`
TlsSkipVerify *bool `json:"tlsSkipVerify,omitempty"`
- CacheKey string `json:"cacheKey"`
+ // 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"`
Review Comment:
Done — pinned ADC **0.29.0** in `6e5a89d6` (`Makefile` `ADC_VERSION` and `config/manager/kustomization.yaml` sidecar tag).
One correction to the premise: api7/adc#537 was **closed, not merged**. The feature landed through api7/adc#552 (`feat(server): support custom tls config per endpoint`), released in **v0.29.0** — `ghcr.io/api7/adc:0.29.0` is published. It happens to use the same wire field this PR already sends, `caCert` in the task opts, alongside `tlsClientCert`/`tlsClientKey` for mTLS, so nothing changed on the controller side.
Exercised end to end against the real image rather than reasoned about. `ghcr.io/api7/adc:0.29.0` in ingress mode, an HTTPS backend whose certificate is signed by a private CA, and the exact request body `buildHTTPRequest` produces:
```
# tlsSkipVerify:false, no caCert -- a private-CA control plane today
{"message":"Error: unable to verify the first certificate; ..."}
# tlsSkipVerify:false + caCert -- what this PR sends
{"message":"AxiosError: Request failed with status code 404"}
```
The second is the fake control plane answering over a completed TLS handshake, so the bundle is genuinely being trusted. For contrast, the previously pinned 0.27.1 returns the same `ECONNREFUSED`/verification error with or without `caCert` — it accepts the option and ignores it, exactly as you described.
Chart-side bump is in apache/apisix-helm-chart#994.
##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -136,6 +137,13 @@ type ControlPlaneProvider struct {
// +optional
TlsVerify *bool `json:"tlsVerify,omitempty"`
+ // CaBundle is a PEM-encoded CA certificate (or bundle) used to verify the
+ // control plane's TLS certificate, in place of the system trust store.
+ // 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"`
Review Comment:
Good catch — confirmed on `apache/apisix-helm-chart` `master`: `charts/apisix-ingress-controller/crds/apisixic-crds.yaml` has `tlsVerify` but no `caBundle`, so a standard Helm install would prune the field.
Paired chart PR: **apache/apisix-helm-chart#994**. It adds the property plus its CEL rule, bumps the ADC sidecar to 0.29.0, and bumps the chart `version` to 1.2.3 following #992.
The CRD edit is not hand-written: I lifted the property and CEL blocks verbatim from this PR's `controller-gen` output so the chart cannot drift in wording or wrapping. Verified by parsing both files and comparing the schemas:
```
provider subtree identical to generated CRD: True
whole GatewayProxy schema identical: True
```
`appVersion` is left alone there, since this controller change is not released 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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#discussion_r3860054482
##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -136,6 +137,13 @@ type ControlPlaneProvider struct {
// +optional
TlsVerify *bool `json:"tlsVerify,omitempty"`
+ // CaBundle is a PEM-encoded CA certificate (or bundle) used to verify the
+ // control plane's TLS certificate, in place of the system trust store.
+ // 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"`
Review Comment:
```yaml
caCert:
value: |
-----BEGIN CERTIFICATE-----
valueFrom:
secretKeyRef: {name: cp-ca, key: ca.crt}
```
I think this would be better, don't you think?
Secret can be left unimplemented for now.
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#issuecomment-5419333503
Need e2e test.
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#discussion_r3710333577
##########
internal/adc/translator/gatewayproxy.go:
##########
@@ -56,6 +57,17 @@ func (t *Translator) TranslateGatewayProxyToConfig(tctx *provider.TranslateConte
cfg.TlsVerify = *cp.TlsVerify
}
+ if cp.CaBundle != "" {
+ // reject unusable CA material here rather than at connect time
+ if !x509.NewCertPool().AppendCertsFromPEM([]byte(cp.CaBundle)) {
+ return nil, errors.New("invalid caBundle: no PEM-encoded certificate found")
+ }
Review Comment:
I would not require `IsCA` here. A self-signed endpoint certificate can be supplied directly as a TLS trust anchor even when its Basic Constraints do not mark it as a CA, and enforcing `IsCA` would reject that supported case. Parsing every `CERTIFICATE` block is the right validation boundary.
##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -120,6 +120,7 @@ type ControlPlaneAuth struct {
// 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"
Review 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.
##########
internal/adc/client/executor_test.go:
##########
@@ -70,6 +70,37 @@ func TestHTTPADCExecutorBuildHTTPRequestBypassCache(t *testing.T) {
assert.NotContains(t, raw, "bypassCache")
}
+func TestHTTPADCExecutorBuildHTTPRequestCaCert(t *testing.T) {
+ e := &HTTPADCExecutor{
+ serverURL: "http://127.0.0.1:3000",
+ log: logr.Discard(),
+ }
+
+ build := func(config adctypes.Config) (ADCServerOpts, string) {
+ req, err := e.buildHTTPRequest(context.Background(), "https://apisix:9180", config, nil, nil,
+ &adctypes.Resources{}, http.MethodPut, pathSync)
+ require.NoError(t, err)
+ body, err := io.ReadAll(req.Body)
+ require.NoError(t, err)
+ var parsed ADCServerRequest
+ require.NoError(t, json.Unmarshal(body, &parsed))
+ return parsed.Task.Opts, string(body)
+ }
+
+ // Without a CA bundle the request stays what an ADC server that predates caCert
+ // already accepts.
+ opts, raw := build(adctypes.Config{Name: "GatewayProxy/ns/name", TlsVerify: true})
+ assert.Empty(t, opts.CaCert)
+ assert.NotContains(t, raw, "caCert")
Review Comment:
`adctypes.Config.MarshalJSON` is not used to build the ADC request: `ADCServerTask.Config` is an `adctypes.Resources`. The `hasCaBundle` field is therefore log-only, while the existing raw-body assertion that `caCert` is absent already covers wire compatibility.
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "nic-6443 (via GitHub)" <gi...@apache.org>.
nic-6443 commented on code in PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#discussion_r3710323591
##########
internal/adc/client/executor.go:
##########
@@ -84,7 +84,11 @@ type ADCServerOpts struct {
LabelSelector map[string]string `json:"labelSelector,omitempty"`
IncludeResourceType []string `json:"includeResourceType,omitempty"`
TlsSkipVerify *bool `json:"tlsSkipVerify,omitempty"`
- CacheKey string `json:"cacheKey"`
+ // 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"`
Review 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.
##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -136,6 +137,13 @@ type ControlPlaneProvider struct {
// +optional
TlsVerify *bool `json:"tlsVerify,omitempty"`
+ // CaBundle is a PEM-encoded CA certificate (or bundle) used to verify the
+ // control plane's TLS certificate, in place of the system trust store.
+ // 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"`
Review 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.
--
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] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on PR #2826:
URL: https://github.com/apache/apisix-ingress-controller/pull/2826#issuecomment-5175968326
Two blockers before this can merge:
**1. The ADC side isn't in yet.** api7/adc#537 is still open, and `ADC_VERSION` is pinned to `0.27.1` (Makefile:30) for both the sidecar image and e2e. So adc merge → release → bump here are all needed before `caBundle` does anything.
The compatibility note in the description is also off: `SyncTask.opts` / `ValidateTask.opts` in `apps/cli/src/server/schema.ts` are `z.looseObject`, not strict — only the top-level `task` is strict. An unknown `caCert` is silently dropped, not rejected. Net effect if this merges now: the field validates, syncs, and is discarded; the user still gets the same opaque TLS failure this PR set out to remove. Accepted-but-inert is worse than absent.
**2. No e2e — and none is possible yet.** Coverage today is unit-level only (translator sets `Config.CaBundle`, executor puts `caCert` on the wire). e2e has no TLS control plane at all: the endpoint is hardcoded `http://…:9180` (`scaffold/apisix_deployer.go:475`) and `tlsVerify` is always `false` (`apisix_deployer.go:488`). Until ADC honors `caCert`, an e2e test could only assert CEL validation and field plumbing — not the one behavior that matters.
Suggested order: land api7/adc#537 and release it → bump `ADC_VERSION` → add e2e with a self-signed-CA HTTPS admin endpoint: sync succeeds with `tlsVerify: true` + `caBundle`, fails without it. That needs a TLS-control-plane option in the e2e scaffold first, which is the bulk of the missing work.
(The red `e2e-test (apisix-standalone, networking.k8s.io)` is the known standalone flake, unrelated.)
--
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