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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
