Backport fixes and metrics (#9688)

* Small Gateway Touch-Ups Backport of (#9668) * fix helm values for GwParams + sidecars * remove extensions from deployer; no longer needed * filter irrelevant GwClasses --------- Co-authored-by: Jacob Bohanon <[email protected]> * GGv2: Add Prometheus stats listener to Envoy Backport of (#9658) --------- Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: Jacob Bohanon <[email protected]> * changelog
kgateway-dev · Jun 25, 2024 · 7d4fa08 · 7d4fa08
1 parent 9a8e3aa
commit 7d4fa08
Show file tree

Hide file tree

Showing 21 changed files with 1,121 additions and 262 deletions.
diff --git a/changelog/v1.17.0-rc7/various-gateway-touch-ups.yaml b/changelog/v1.17.0-rc7/various-gateway-touch-ups.yaml
@@ -0,0 +1,21 @@
+changelog:
+  - type: NON_USER_FACING
+    issueLink: https://github.com/solo-io/solo-projects/issues/6381
+    resolvesIssue: false
+    description: >-
+      Follow up to https://github.com/solo-io/gloo/pull/9657 --
+      Renders out securityContext for sds and istio sidecar containers to dynamic GG proxy and
+      fixes the helm docs for these containers.
+  - type: FIX
+    issueLink: https://github.com/solo-io/gloo/issues/9660
+    resolvesIssue: true
+    description: >-
+      Only act on GatewayClass resources that use the Gloo Gateway controller name
+  - type: NON_USER_FACING
+    description: >-
+      Remove unused Extensions from Deployer component
+  - type: NEW_FEATURE
+    issueLink: https://github.com/solo-io/solo-projects/issues/6393
+    resolvesIssue: true
+    description: |
+      Export Prometheus Metrics on Port 9091 for Gloo Gateway.
diff --git a/docs/content/reference/values.txt b/docs/content/reference/values.txt
@@ -64,7 +64,6 @@
 |kubeGateway.gatewayParameters.glooGateway.sdsContainer.securityContext.procMount|string|||
 |kubeGateway.gatewayParameters.glooGateway.sdsContainer.securityContext.seccompProfile.type|string|||
 |kubeGateway.gatewayParameters.glooGateway.sdsContainer.securityContext.seccompProfile.localhostProfile|string|||
-|kubeGateway.gatewayParameters.glooGateway.sdsContainer.securityContext.mergePolicy|string||How to combine the defined security policy with the default security policy. Valid values are "", "no-merge", and "helm-merge". If defined as an empty string or "no-merge", use the defined security context as is.  If "helm-merge", merge this security context with the default security context according to the logic of [the helm 'merge' function](https://helm.sh/docs/chart_template_guide/function_list/#merge-mustmerge). This is intended to be used to modify a field in a security context, while using all other default values. Please note that due to how helm's 'merge' function works, you can not override a 'true' value with a 'false' value, and for that case you will need to define the entire security context and set this value to false. Default value is "".|
 |kubeGateway.gatewayParameters.glooGateway.sdsContainer.logLevel|string|info|Log level for sds.  Options include "info", "debug", "warn", "error", "panic" and "fatal". Default level is info.|
 |kubeGateway.gatewayParameters.glooGateway.sdsContainer.sdsResources.limits.memory|string||amount of memory|
 |kubeGateway.gatewayParameters.glooGateway.sdsContainer.sdsResources.limits.cpu|string||amount of CPUs|
@@ -98,12 +97,15 @@
 |kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.securityContext.procMount|string|||
 |kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.securityContext.seccompProfile.type|string|||
 |kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.securityContext.seccompProfile.localhostProfile|string|||
-|kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.securityContext.mergePolicy|string||How to combine the defined security policy with the default security policy. Valid values are "", "no-merge", and "helm-merge". If defined as an empty string or "no-merge", use the defined security context as is.  If "helm-merge", merge this security context with the default security context according to the logic of [the helm 'merge' function](https://helm.sh/docs/chart_template_guide/function_list/#merge-mustmerge). This is intended to be used to modify a field in a security context, while using all other default values. Please note that due to how helm's 'merge' function works, you can not override a 'true' value with a 'false' value, and for that case you will need to define the entire security context and set this value to false. Default value is "".|
 |kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.logLevel|string|warning|Log level for istio-proxy. Options include "info", "debug", "warning", and "error". Default level is info Default is 'warning'.|
 |kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.istioMetaMeshId|string|cluster.local|ISTIO_META_MESH_ID Environment Variable. Warning: this value is only supported with Kubernetes Gateway API proxy. Defaults to "cluster.local"|
 |kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.istioMetaClusterId|string|Kubernetes|ISTIO_META_CLUSTER_ID Environment Variable. Warning: this value is only supported with Kubernetes Gateway API proxy. Defaults to "Kubernetes"|
 |kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.istioDiscoveryAddress|string|istiod.istio-system.svc:15012|discoveryAddress field of the PROXY_CONFIG environment variable. Warning: this value is only supported with Kubernetes Gateway API proxy. Defaults to "istiod.istio-system.svc:15012"|
 |kubeGateway.gatewayParameters.glooGateway.istio.customSidecars[]|interface||Override the default Istio sidecar in gateway-proxy with a custom container. Ignored if Istio.enabled is false|
+|kubeGateway.gatewayParameters.glooGateway.stats.enabled|bool|true|Enable the prometheus endpoint|
+|kubeGateway.gatewayParameters.glooGateway.stats.routePrefixRewrite|string|/stats/prometheus|Set the prefix rewrite used for the prometheus endpoint|
+|kubeGateway.gatewayParameters.glooGateway.stats.enableStatsRoute|bool|true|Enable the stats endpoint|
+|kubeGateway.gatewayParameters.glooGateway.stats.statsRoutePrefixRewrite|string|/stats|Set the prefix rewrite used for the stats endpoint|
 |settings.watchNamespaces[]|string||whitelist of namespaces for Gloo Edge to watch for services and CRDs. Empty list means all namespaces|
 |settings.writeNamespace|string||namespace where intermediary CRDs will be written to, e.g. Upstreams written by Gloo Edge Discovery.|
 |settings.integrations.knative.enabled|bool|false|enabled knative components|

diff --git a/install/helm/gloo/crds/gateway.gloo.solo.io_crds.yaml b/install/helm/gloo/crds/gateway.gloo.solo.io_crds.yaml
@@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    crd.solo.io/specHash: c30098945b4c73d8
+    crd.solo.io/specHash: a61fce8a0e07ddb
   labels:
     app: gloo-gateway
     app.kubernetes.io/name: gloo-gateway
@@ -1412,6 +1412,21 @@ spec:
                         - LoadBalancer
                         type: string
                     type: object
+                  stats:
+                    properties:
+                      enableStatsRoute:
+                        nullable: true
+                        type: boolean
+                      enabled:
+                        nullable: true
+                        type: boolean
+                      routePrefixRewrite:
+                        nullable: true
+                        type: string
+                      statsRoutePrefixRewrite:
+                        nullable: true
+                        type: string
+                    type: object
                 type: object
               selfManaged:
                 maxProperties: 0

diff --git a/install/helm/gloo/generate/values.go b/install/helm/gloo/generate/values.go
@@ -317,17 +317,25 @@ type GatewayParametersForGatewayClasses struct {
 }
 
 type GatewayParameters struct {
-	EnvoyContainer  *EnvoyContainer        `json:"envoyContainer,omitempty" desc:"Config for the Envoy container of the proxy deployment."`
-	ProxyDeployment *ProvisionedDeployment `json:"proxyDeployment,omitempty" desc:"Options specific to the deployment of the dynamically provisioned gateway proxy. Only a subset of all possible options is available. See \"ProvisionedDeployment\" for which are configurable via helm."`
-	Service         *ProvisionedService    `json:"service,omitempty" desc:"Options specific to the service of the dynamically provisioned gateway proxy. Only a subset of all possible options is available. See \"ProvisionedService\" for which are configurable via helm."`
-	SdsContainer    *SdsContainer          `json:"sdsContainer,omitempty" desc:"Config used to manage the Gloo Gateway SDS container."`
-	Istio           *Istio                 `json:"istio,omitempty" desc:"Configs used to manage Istio integration."`
+	EnvoyContainer  *EnvoyContainer            `json:"envoyContainer,omitempty" desc:"Config for the Envoy container of the proxy deployment."`
+	ProxyDeployment *ProvisionedDeployment     `json:"proxyDeployment,omitempty" desc:"Options specific to the deployment of the dynamically provisioned gateway proxy. Only a subset of all possible options is available. See \"ProvisionedDeployment\" for which are configurable via helm."`
+	Service         *ProvisionedService        `json:"service,omitempty" desc:"Options specific to the service of the dynamically provisioned gateway proxy. Only a subset of all possible options is available. See \"ProvisionedService\" for which are configurable via helm."`
+	SdsContainer    *GatewayParamsSdsContainer `json:"sdsContainer,omitempty" desc:"Config used to manage the Gloo Gateway SDS container."`
+	Istio           *Istio                     `json:"istio,omitempty" desc:"Configs used to manage Istio integration."`
+	Stats           *GatewayParamsStatsConfig  `json:"stats,omitempty" desc:"Config used to manage the stats endpoints exposed on the deployed proxies"`
 	// TODO(npolshak): Add support for GlooMtls
 }
 
+type GatewayParamsStatsConfig struct {
+	Enabled                 *bool   `json:"enabled,omitempty" desc:"Enable the prometheus endpoint"`
+	RoutePrefixRewrite      *string `json:"routePrefixRewrite,omitempty" desc:"Set the prefix rewrite used for the prometheus endpoint"`
+	EnableStatsRoute        *bool   `json:"enableStatsRoute,omitempty" desc:"Enable the stats endpoint"`
+	StatsRoutePrefixRewrite *string `json:"statsRoutePrefixRewrite,omitempty" desc:"Set the prefix rewrite used for the stats endpoint"`
+}
+
 type Istio struct {
-	IstioProxyContainer *IstioProxyContainer `json:"istioProxyContainer,omitempty" desc:"Config used to manage the istio-proxy container."`
-	CustomSidecars      []interface{}        `json:"customSidecars,omitempty" desc:"Override the default Istio sidecar in gateway-proxy with a custom container. Ignored if Istio.enabled is false"`
+	IstioProxyContainer *GatewayParamsIstioProxyContainer `json:"istioProxyContainer,omitempty" desc:"Config used to manage the istio-proxy container."`
+	CustomSidecars      []interface{}                     `json:"customSidecars,omitempty" desc:"Override the default Istio sidecar in gateway-proxy with a custom container. Ignored if Istio.enabled is false"`
 }
 
 type ProvisionedDeployment struct {
@@ -807,6 +815,13 @@ type SdsContainer struct {
 	Resources       *ResourceRequirements `json:"sdsResources,omitempty" desc:"Sets default resource requirements for all sds containers."`
 }
 
+type GatewayParamsSdsContainer struct {
+	Image           *Image                        `json:"image,omitempty"`
+	SecurityContext *GatewayParamsSecurityContext `json:"securityContext,omitempty" desc:"securityContext for sds gloo deployment container. If this is defined it supersedes any values set in FloatingUserId, RunAsUser, DisableNetBind, RunUnprivileged. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.26/#securitycontext-v1-core for details."`
+	LogLevel        *string                       `json:"logLevel,omitempty" desc:"Log level for sds.  Options include \"info\", \"debug\", \"warn\", \"error\", \"panic\" and \"fatal\". Default level is info."`
+	Resources       *ResourceRequirements         `json:"sdsResources,omitempty" desc:"Sets default resource requirements for all sds containers."`
+}
+
 type EnvoySidecarContainer struct {
 	Image           *Image           `json:"image,omitempty"`
 	SecurityContext *SecurityContext `json:"securityContext,omitempty" desc:"securityContext for envoy-sidecar gloo deployment container. If this is defined it supercedes any values set in FloatingUserId, RunAsUser, DisableNetBind, RunUnprivileged. See [security context](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.26/#securitycontext-v1-core) for details."`
@@ -824,6 +839,18 @@ type IstioProxyContainer struct {
 	IstioDiscoveryAddress *string `json:"istioDiscoveryAddress,omitempty" desc:"discoveryAddress field of the PROXY_CONFIG environment variable. Warning: this value is only supported with Kubernetes Gateway API proxy. Defaults to \"istiod.istio-system.svc:15012\""`
 }
 
+type GatewayParamsIstioProxyContainer struct {
+	Image           *Image                        `json:"image,omitempty" desc:"Istio-proxy image to use for mTLS"`
+	SecurityContext *GatewayParamsSecurityContext `json:"securityContext,omitempty" desc:"securityContext for istio-proxy deployment container. If this is defined it supercedes any values set in FloatingUserId, RunAsUser, DisableNetBind, RunUnprivileged. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.26/#securitycontext-v1-core for details."`
+	LogLevel        *string                       `json:"logLevel,omitempty" desc:"Log level for istio-proxy. Options include \"info\", \"debug\", \"warning\", and \"error\". Default level is info Default is 'warning'."`
+
+	// TODO(npolshak): Deprecate GatewayProxy IstioMetaMeshId/IstioMetaClusterId/IstioDiscoveryAddress in favor of IstioProxyContainer
+	// Note: these are only supported for k8s Gateway API.
+	IstioMetaMeshId       *string `json:"istioMetaMeshId,omitempty" desc:"ISTIO_META_MESH_ID Environment Variable. Warning: this value is only supported with Kubernetes Gateway API proxy. Defaults to \"cluster.local\""`
+	IstioMetaClusterId    *string `json:"istioMetaClusterId,omitempty" desc:"ISTIO_META_CLUSTER_ID Environment Variable. Warning: this value is only supported with Kubernetes Gateway API proxy. Defaults to \"Kubernetes\""`
+	IstioDiscoveryAddress *string `json:"istioDiscoveryAddress,omitempty" desc:"discoveryAddress field of the PROXY_CONFIG environment variable. Warning: this value is only supported with Kubernetes Gateway API proxy. Defaults to \"istiod.istio-system.svc:15012\""`
+}
+
 type IstioSDS struct {
 	// NOTE: IstioSDS.Enabled is deprecated. Use IstioIntegration.Enabled instead.
 	Enabled        *bool         `json:"enabled,omitempty" desc:"Enables SDS cert-rotator sidecar for istio mTLS cert rotation. Warning: this value is deprecated and will be removed in a future release. Use global.istioIntegration.enabled instead."`

diff --git a/install/helm/gloo/templates/43-gatewayparameters.yaml b/install/helm/gloo/templates/43-gatewayparameters.yaml
@@ -88,6 +88,10 @@ spec:
         istioDiscoveryAddress: {{ $gg.istio.istioProxyContainer.istioDiscoveryAddress }}
         istioMetaMeshId: {{ $gg.istio.istioProxyContainer.istioMetaMeshId }}
         istioMetaClusterId: {{ $gg.istio.istioProxyContainer.istioMetaClusterId }}
+{{- if $gg.stats }}
+    stats:
+    {{- toYaml $gg.stats | nindent 6 }}
+{{- end -}}{{/* if $gg.stats */}}
 {{- end }}{{/* if and $gg.istio.istioProxyContainer (not $gg.istio.customSidecars) */}}
 {{- end }}{{/* if $gg.sdsContainer */}}
 {{- end }}{{/* if .Values.kubeGateway.enabled */}}
diff --git a/install/helm/gloo/values-template.yaml b/install/helm/gloo/values-template.yaml
@@ -44,6 +44,11 @@ kubeGateway:
           istioDiscoveryAddress: istiod.istio-system.svc:15012
           istioMetaMeshId: cluster.local
           istioMetaClusterId: Kubernetes
+      stats:
+        enabled: true
+        routePrefixRewrite: "/stats/prometheus"
+        enableStatsRoute: true
+        statsRoutePrefixRewrite: "/stats"
 
 settings:
   # if this is set to false, default settings will be created by pods upon boot

diff --git a/install/test/k8sgateway_test.go b/install/test/k8sgateway_test.go
@@ -21,7 +21,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
 			testManifest TestManifest
 			valuesArgs   []string
 		)
-		prepareMakefile := func(namespace string, values helmValues) {
+		prepareHelmManifest := func(namespace string, values helmValues) {
 			tm, err := rendererTestCase.renderer.RenderManifest(namespace, values)
 			ExpectWithOffset(1, err).NotTo(HaveOccurred(), "Failed to render manifest")
 			testManifest = tm
@@ -32,7 +32,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
 		})
 
 		JustBeforeEach(func() {
-			prepareMakefile(namespace, helmValues{valuesArgs: valuesArgs})
+			prepareHelmManifest(namespace, helmValues{valuesArgs: valuesArgs})
 		})
 		When("kube gateway integration is enabled", func() {
 			BeforeEach(func() {
@@ -107,6 +107,11 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
 				Expect(gwpKube.GetSdsContainer().GetResources()).To(BeNil())
 
 				Expect(gwpKube.GetService().GetType()).To(Equal(kube.Service_LoadBalancer))
+
+				Expect(gwpKube.GetStats().GetEnabled().GetValue()).To(BeTrue())
+				Expect(gwpKube.GetStats().GetRoutePrefixRewrite().GetValue()).To(Equal("/stats/prometheus"))
+				Expect(gwpKube.GetStats().GetEnableStatsRoute().GetValue()).To(BeTrue())
+				Expect(gwpKube.GetStats().GetStatsRoutePrefixRewrite().GetValue()).To(Equal("/stats"))
 			})
 
 			When("overrides are set", func() {
@@ -153,6 +158,10 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
 						"kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.logLevel=debug",
 						"kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.securityContext.runAsNonRoot=null",
 						"kubeGateway.gatewayParameters.glooGateway.istio.istioProxyContainer.securityContext.runAsUser=888",
+						"kubeGateway.gatewayParameters.glooGateway.stats.enabled=false",
+						"kubeGateway.gatewayParameters.glooGateway.stats.routePrefixRewrite=/foo/bar",
+						"kubeGateway.gatewayParameters.glooGateway.stats.enableStatsRoute=false",
+						"kubeGateway.gatewayParameters.glooGateway.stats.statsRoutePrefixRewrite=/scooby/doo",
 						"global.istioIntegration.enabled=true",
 					}
 					valuesArgs = append(valuesArgs, extraValuesArgs...)
@@ -222,6 +231,11 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
 					Expect(gwpKube.GetSdsContainer().GetResources().GetLimits()).To(matchers.ContainMapElements(sdsLimits))
 
 					Expect(gwpKube.GetService().GetType()).To(Equal(kube.Service_ClusterIP))
+
+					Expect(gwpKube.GetStats().GetEnabled().GetValue()).To(BeFalse())
+					Expect(gwpKube.GetStats().GetRoutePrefixRewrite().GetValue()).To(Equal("/foo/bar"))
+					Expect(gwpKube.GetStats().GetEnableStatsRoute().GetValue()).To(BeFalse())
+					Expect(gwpKube.GetStats().GetStatsRoutePrefixRewrite().GetValue()).To(Equal("/scooby/doo"))
 				})
 			})
 

diff --git a/projects/gateway2/api/v1alpha1/gateway_parameters.proto b/projects/gateway2/api/v1alpha1/gateway_parameters.proto
@@ -64,6 +64,9 @@ message KubernetesProxyConfig {
 
   // Istio integration configuration.
   IstioIntegration istio = 6;
+
+  // Optional stats configuration (enabled by default)
+  StatsConfig stats = 8;
 }
 
 // Configuration for the Proxy deployment in Kubernetes.
@@ -212,3 +215,17 @@ message GatewayParametersStatus {
   // implemented.
   // repeated k8s.io.apimachinery.pkg.apis.meta.v1.Condition conditions = 1;
 }
+
+message StatsConfig {
+  // Whether to expose metrics annotations and ports for scraping metrics.
+  .google.protobuf.BoolValue enabled = 1;
+
+  // The Envoy stats endpoint to which the metrics are written
+  .google.protobuf.StringValue route_prefix_rewrite = 2;
+
+  // Enables an additional route to the stats cluster defaulting to /stats
+  .google.protobuf.BoolValue enable_stats_route = 3;
+
+  // The Envoy stats endpoint with general metrics for the additional stats route
+  .google.protobuf.StringValue stats_route_prefix_rewrite = 4;
+}