[1.16] fix: allows setting Istio CA_ADDR independently (#9898)

* fix: allows setting Istio CA_ADDR independently (#9756) * fix: allows setting Istio CA_ADDR independently https://istio.io/latest/docs/reference/commands/pilot-agent/ The Istio documentation indicates that CA_ADDR defaults to the PROXY_CONFIG discovery address. Though it should be possible to specify a CA_ADDR that is unrelated to the discovery address. However, Gloo helm forces these two separate fields to be aligned as it is driven from a single helm value. This change introduces an `istioSpiffeCertProviderAddress` property resolving the problem. The default is kept as `istioDiscoveryAddress` to ensure backward compatibility. * docs: add property documentation + issue link * move changelog to beta15 folder * helm_test: fix compile issues, resolve some deprecated function calls --------- Co-authored-by: Sam Heilbron <[email protected]> * move changelog * corev1 -> v1 * fix tests for 1.16 --------- Co-authored-by: Jérémy Lourenço <[email protected]> Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
kgateway-dev · Aug 15, 2024 · 3995b9c · 3995b9c
1 parent 02ddabc
commit 3995b9c
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 9 deletions.
diff --git a/changelog/v1.16.19/istio-spiffe-cert-provider-address.yaml b/changelog/v1.16.19/istio-spiffe-cert-provider-address.yaml
@@ -0,0 +1,7 @@
+changelog:
+  - type: HELM
+    issueLink: https://github.com/solo-io/gloo/issues/9855
+    resolvesIssue: false
+    description: >-
+      Introduce `gatewayProxies.gatewayProxy.istioSpiffeCertProviderAddress` which overrides the Istio SPIFFE
+      certificate provider (`CA_ADDR` env variable). It defaults to `gatewayProxies.gatewayProxy.discoveryAddress`.
diff --git a/docs/content/reference/values.txt b/docs/content/reference/values.txt
@@ -802,6 +802,7 @@
 |gatewayProxies.NAME.istioMetaMeshId|string||ISTIO_META_MESH_ID Environment Variable. Defaults to "cluster.local"|
 |gatewayProxies.NAME.istioMetaClusterId|string||ISTIO_META_CLUSTER_ID Environment Variable. Defaults to "Kubernetes"|
 |gatewayProxies.NAME.istioDiscoveryAddress|string||discoveryAddress field of the PROXY_CONFIG environment variable. Defaults to "istiod.istio-system.svc:15012"|
+|gatewayProxies.NAME.istioSpiffeCertProviderAddress|string||Address of the spiffe certificate provider. Defaults to istioDiscoveryAddress|
 |gatewayProxies.NAME.envoyLogLevel|string||Level at which the pod should log. Options include "trace", "info", "debug", "warn", "error", "critical" and "off". Default level is info|
 |gatewayProxies.NAME.envoyStatsConfig.NAME|interface||Envoy statistics configuration, such as tagging. For more info, see https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/metrics/v3/stats.proto#config-metrics-v3-statsconfig|
 |gatewayProxies.NAME.xdsServiceAddress|string||The k8s service name for the xds server. Defaults to gloo.|
@@ -1051,7 +1052,8 @@
 |gatewayProxies.gatewayProxy.podDisruptionBudget.kubeResourceOverride.NAME|interface||override fields in the generated resource by specifying the yaml structure to override under the top-level key.|
 |gatewayProxies.gatewayProxy.istioMetaMeshId|string||ISTIO_META_MESH_ID Environment Variable. Defaults to "cluster.local"|
 |gatewayProxies.gatewayProxy.istioMetaClusterId|string||ISTIO_META_CLUSTER_ID Environment Variable. Defaults to "Kubernetes"|
-|gatewayProxies.gatewayProxy.istioDiscoveryAddress|string||discoveryAddress field of the PROXY_CONFIG environment variable. Defaults to "istiod.istio-system.svc:15012"|
+|gatewayProxies.gatewayProxy.istioDiscoveryAddress|string|istiod.istio-system.svc:15012|discoveryAddress field of the PROXY_CONFIG environment variable. Defaults to "istiod.istio-system.svc:15012"|
+|gatewayProxies.gatewayProxy.istioSpiffeCertProviderAddress|string||Address of the spiffe certificate provider. Defaults to istioDiscoveryAddress|
 |gatewayProxies.gatewayProxy.envoyLogLevel|string||Level at which the pod should log. Options include "trace", "info", "debug", "warn", "error", "critical" and "off". Default level is info|
 |gatewayProxies.gatewayProxy.envoyStatsConfig.NAME|interface||Envoy statistics configuration, such as tagging. For more info, see https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/metrics/v3/stats.proto#config-metrics-v3-statsconfig|
 |gatewayProxies.gatewayProxy.xdsServiceAddress|string||The k8s service name for the xds server. Defaults to gloo.|

diff --git a/install/helm/gloo/generate/values.go b/install/helm/gloo/generate/values.go
@@ -513,6 +513,7 @@ type GatewayProxy struct {
 	IstioMetaMeshId                *string                          `json:"istioMetaMeshId,omitempty" desc:"ISTIO_META_MESH_ID Environment Variable. Defaults to \"cluster.local\""`
 	IstioMetaClusterId             *string                          `json:"istioMetaClusterId,omitempty" desc:"ISTIO_META_CLUSTER_ID Environment Variable. Defaults to \"Kubernetes\""`
 	IstioDiscoveryAddress          *string                          `json:"istioDiscoveryAddress,omitempty" desc:"discoveryAddress field of the PROXY_CONFIG environment variable. Defaults to \"istiod.istio-system.svc:15012\""`
+	IstioSpiffeCertProviderAddress *string                          `json:"istioSpiffeCertProviderAddress,omitempty" desc:"Address of the spiffe certificate provider. Defaults to istioDiscoveryAddress"`
 	EnvoyLogLevel                  *string                          `json:"envoyLogLevel,omitempty" desc:"Level at which the pod should log. Options include \"trace\", \"info\", \"debug\", \"warn\", \"error\", \"critical\" and \"off\". Default level is info"`
 	EnvoyStatsConfig               map[string]interface{}           `json:"envoyStatsConfig,omitempty" desc:"Envoy statistics configuration, such as tagging. For more info, see https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/metrics/v3/stats.proto#config-metrics-v3-statsconfig"`
 	XdsServiceAddress              *string                          `json:"xdsServiceAddress,omitempty" desc:"The k8s service name for the xds server. Defaults to gloo."`

diff --git a/install/helm/gloo/templates/7-gateway-proxy-deployment.yaml b/install/helm/gloo/templates/7-gateway-proxy-deployment.yaml
@@ -405,14 +405,14 @@ spec:
           - name: PILOT_CERT_PROVIDER
             value: istiod
           - name: CA_ADDR
-            value: {{ $spec.istioDiscoveryAddress | default "istiod.istio-system.svc:15012" }}
+            value: {{ $spec.istioSpiffeCertProviderAddress | default $spec.istioDiscoveryAddress }}
           - name: ISTIO_META_MESH_ID
             value: {{ $spec.istioMetaMeshId | default "cluster.local"}}
           - name: ISTIO_META_CLUSTER_ID
             value: {{ $spec.istioMetaClusterId | default "Kubernetes"}}
           - name: PROXY_CONFIG
             value: |
-              {"discoveryAddress": {{ $spec.istioDiscoveryAddress | default "istiod.istio-system.svc:15012" }}}
+              {"discoveryAddress": {{ $spec.istioDiscoveryAddress }}}
           - name: POD_NAME
             valueFrom:
               fieldRef:

diff --git a/install/helm/gloo/values-template.yaml b/install/helm/gloo/values-template.yaml
@@ -125,6 +125,7 @@ gatewayProxies:
       enabled: false
       port: 15443
       secretName: failover-downstream
+    istioDiscoveryAddress: istiod.istio-system.svc:15012
     kind:
       deployment:
         replicas: 1

diff --git a/install/test/helm_test.go b/install/test/helm_test.go
@@ -1437,8 +1437,8 @@ spec:
 					svc := rb.GetService()
 					svc.Spec.Selector = selector
 					svc.Spec.Type = v1.ServiceTypeLoadBalancer
-					svc.Spec.Ports[0].TargetPort = intstr.FromInt(8080)
-					svc.Spec.Ports[1].TargetPort = intstr.FromInt(8443)
+					svc.Spec.Ports[0].TargetPort = intstr.FromInt32(8080)
+					svc.Spec.Ports[1].TargetPort = intstr.FromInt32(8443)
 					svc.Annotations = map[string]string{"test": "test"}
 					testManifest.ExpectService(svc)
 				})
@@ -2818,6 +2818,30 @@ spec:
 						})
 					}
 
+					checkSpiffeCertProviderAddressEqual := func(expected string) {
+						testManifest.SelectResources(func(resource *unstructured.Unstructured) bool {
+							return resource.GetKind() == "Deployment" && resource.GetName() == "gateway-proxy"
+						}).ExpectAll(func(deployment *unstructured.Unstructured) {
+							deploymentObject, err := kuberesource.ConvertUnstructured(deployment)
+							Expect(err).NotTo(HaveOccurred(), "Deployment should be able to convert from unstructured")
+							structuredDeployment, ok := deploymentObject.(*appsv1.Deployment)
+							Expect(ok).To(BeTrue(), "Deployment should be able to cast to a structured deployment")
+							isCAAddrSet := false
+
+							for _, container := range structuredDeployment.Spec.Template.Spec.Containers {
+								for _, env := range container.Env {
+									if env.Name == "CA_ADDR" {
+										isCAAddrSet = true
+										Expect(env.Value).To(Equal(expected), fmt.Sprintf("SPIFFE cert address should be value: %v", expected))
+										break
+									}
+								}
+							}
+
+							Expect(isCAAddrSet).To(BeTrue(), "Istio's CA_ADDR was not set")
+						})
+					}
+
 					BeforeEach(func() {
 						selector = map[string]string{
 							"gloo":             "gateway-proxy",
@@ -3237,7 +3261,7 @@ spec:
 							ProbeHandler: v1.ProbeHandler{
 								HTTPGet: &v1.HTTPGetAction{
 									Path:   "/ready",
-									Port:   intstr.FromInt(19000),
+									Port:   intstr.FromInt32(19000),
 									Scheme: "HTTP",
 								},
 							},
@@ -3249,7 +3273,7 @@ spec:
 							ProbeHandler: v1.ProbeHandler{
 								HTTPGet: &v1.HTTPGetAction{
 									Path:   "/server_info",
-									Port:   intstr.FromInt(19000),
+									Port:   intstr.FromInt32(19000),
 									Scheme: "HTTP",
 								},
 							},
@@ -3664,12 +3688,29 @@ spec:
 						prepareMakefile(namespace, helmValues{
 							valuesArgs: []string{
 								"global.glooMtls.enabled=true",
+								// In 1.17 and above, this is controlled by "global.istioIntegration.enabled=true"
 								"global.istioSDS.enabled=true",
 								"gatewayProxies.gatewayProxy.istioDiscoveryAddress=" + val,
 							},
 						})
 
 						checkDiscoveryAddressEqual(val)
+						checkSpiffeCertProviderAddressEqual(val)
+					})
+
+					It("can set spiffeCertProviderAddress value in CA_ADDR env var", func() {
+						val := "istiod-1-8-6.istio-system.svc:15012"
+
+						prepareMakefile(namespace, helmValues{
+							valuesArgs: []string{
+								"global.glooMtls.enabled=true",
+								// In 1.17 and above, this is controlled by "global.istioIntegration.enabled=true"
+								"global.istioSDS.enabled=true",
+								"gatewayProxies.gatewayProxy.istioSpiffeCertProviderAddress=" + val,
+							},
+						})
+
+						checkSpiffeCertProviderAddressEqual(val)
 					})
 
 					It("istio's discoveryAddress default value set", func() {
@@ -3678,13 +3719,28 @@ spec:
 						prepareMakefile(namespace, helmValues{
 							valuesArgs: []string{
 								"global.glooMtls.enabled=true",
+								// In 1.17 and above, this is controlled by "global.istioIntegration.enabled=true"
 								"global.istioSDS.enabled=true",
 							},
 						})
 
 						checkDiscoveryAddressEqual(def)
 					})
 
+					It("istio's spiffeCertProviderAddress default value set", func() {
+						def := "istiod.istio-system.svc:15012"
+
+						prepareMakefile(namespace, helmValues{
+							valuesArgs: []string{
+								"global.glooMtls.enabled=true",
+								// In 1.17 and above, this is controlled by "global.istioIntegration.enabled=true"
+								"global.istioSDS.enabled=true",
+							},
+						})
+
+						checkSpiffeCertProviderAddressEqual(def)
+					})
+
 					It("can add extra volume mounts to the gateway-proxy container deployment", func() {
 
 						gatewayProxyDeployment.Spec.Template.Spec.Containers[0].VolumeMounts = append(
@@ -4781,11 +4837,10 @@ metadata:
 								v1.ResourceCPU:    resource.MustParse("500m"),
 							},
 						}
-
 						deploy.Spec.Template.Spec.Containers[0].ReadinessProbe = &v1.Probe{
 							ProbeHandler: v1.ProbeHandler{
 								TCPSocket: &v1.TCPSocketAction{
-									Port: intstr.FromInt(9977),
+									Port: intstr.FromInt32(9977),
 								},
 							},
 							InitialDelaySeconds: 3,