Re: [PR] feat: support a CA bundle for the control plane connection [apisix-ingress-controller]

["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