Add an option to make HTTP2 configurable (#5231)

Signed-off-by: siddhi bhor <sbhor@redhat.com>

# Conflicts:
#	cmd/controller/root.go

Co-authored-by: Gergely Brautigam <skarlso777@gmail.com>

cmd/controller/certcontroller.go

@@ -17,6 +17,7 @@ limitations under the License.
 package controller
 
 import (
+	"crypto/tls"
 	"os"
 	"time"
 
@@ -73,11 +74,19 @@ var certcontrollerCmd = &cobra.Command{
 			}
 		}
 
+		// Configure metrics server options
+		metricsServerOpts := server.Options{
+			BindAddress: metricsAddr,
+		}
+
+		// Disable HTTP/2 if not explicitly enabled
+		if !enableHTTP2 {
+			metricsServerOpts.TLSOpts = []func(*tls.Config){disableHTTP2}
+		}
+
 		mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
-			Scheme: scheme,
-			Metrics: server.Options{
-				BindAddress: metricsAddr,
-			},
+			Scheme:  scheme,
+			Metrics: metricsServerOpts,
 			WebhookServer: webhook.NewServer(webhook.Options{
 				Port: 9443,
 			}),
@@ -192,4 +201,6 @@ func init() {
 	certcontrollerCmd.Flags().StringVar(&loglevel, "loglevel", "info", "loglevel to use, one of: debug, info, warn, error, dpanic, panic, fatal")
 	certcontrollerCmd.Flags().StringVar(&zapTimeEncoding, "zap-time-encoding", "epoch", "Zap time encoding (one of 'epoch', 'millis', 'nano', 'iso8601', 'rfc3339' or 'rfc3339nano')")
 	certcontrollerCmd.Flags().DurationVar(&crdRequeueInterval, "crd-requeue-interval", time.Minute*5, "Time duration between reconciling CRDs for new certs")
+	certcontrollerCmd.Flags().BoolVar(&enableHTTP2, "enable-http2", false,
+		"If set, HTTP/2 will be enabled for the metrics server")
 }

cmd/controller/root.go

@@ -17,6 +17,7 @@ limitations under the License.
 package controller
 
 import (
+	"crypto/tls"
 	"os"
 	"time"
 
@@ -98,6 +99,7 @@ var (
 	certLookaheadInterval                 time.Duration
 	tlsCiphers                            string
 	tlsMinVersion                         string
+	enableHTTP2                           bool
 )
 
 const (
@@ -150,6 +152,11 @@ var rootCmd = &cobra.Command{
 			metricsOpts.CertName = metricsCertName
 			metricsOpts.KeyName = metricsKeyName
 		}
+		
+		// Disable HTTP/2 if not explicitly enabled
+		if !enableHTTP2 {
+			metricsOpts.TLSOpts = []func(*tls.Config){disableHTTP2}
+		}
 		mgrOpts := ctrl.Options{
 			Scheme:                 scheme,
 			Metrics:                metricsOpts,
@@ -354,8 +361,15 @@ func init() {
 	rootCmd.Flags().BoolVar(&enableFloodGate, "enable-flood-gate", true, "Enable flood gate. External secret will be reconciled only if the ClusterStore or Store have an healthy or unknown state.")
 	rootCmd.Flags().BoolVar(&enableGeneratorState, "enable-generator-state", true, "Whether the Controller should manage GeneratorState")
 	rootCmd.Flags().BoolVar(&enableExtendedMetricLabels, "enable-extended-metric-labels", false, "Enable recommended kubernetes annotations as labels in metrics.")
+	rootCmd.Flags().BoolVar(&enableHTTP2, "enable-http2", false,
+		"If set, HTTP/2 will be enabled for the metrics server")
 	fs := feature.Features()
 	for _, f := range fs {
 		rootCmd.Flags().AddFlagSet(f.Flags)
 	}
 }
+
+// disableHTTP2 is a TLS configuration function that disables HTTP/2.
+func disableHTTP2(cfg *tls.Config) {
+	cfg.NextProtos = []string{"http/1.1"}
+}

cmd/controller/webhook.go

@@ -101,24 +101,47 @@ var webhookCmd = &cobra.Command{
 			ctrl.Log.Error(err, "unable to fetch tls ciphers")
 			os.Exit(1)
 		}
-		mgrTLSOptions := func(cfg *tls.Config) {
-			cfg.CipherSuites = cipherList
+
+		// Configure TLS options for webhook server
+		var webhookTLSOpts []func(*tls.Config)
+
+		// Add cipher configuration if needed
+		if len(cipherList) > 0 {
+			webhookTLSOpts = append(webhookTLSOpts, func(cfg *tls.Config) {
+				cfg.CipherSuites = cipherList
+			})
+		}
+
+		// Add TLS version configuration
+		webhookTLSOpts = append(webhookTLSOpts, func(c *tls.Config) {
+			c.MinVersion = tlsVersion(tlsMinVersion)
+		})
+
+		// Add HTTP/2 disabling if needed
+		if !enableHTTP2 {
+			webhookTLSOpts = append(webhookTLSOpts, disableHTTP2)
 		}
+
+		// Configure metrics server options
+		metricsServerOpts := server.Options{
+			BindAddress: metricsAddr,
+		}
+
+		// Configure TLS options for metrics server
+		var metricsTLSOpts []func(*tls.Config)
+		if !enableHTTP2 {
+			metricsTLSOpts = append(metricsTLSOpts, disableHTTP2)
+		}
+		metricsServerOpts.TLSOpts = metricsTLSOpts
+
 		mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
-			Scheme: scheme,
-			Metrics: server.Options{
-				BindAddress: metricsAddr,
-			},
+			Scheme:                 scheme,
+			Metrics:                metricsServerOpts,
 			HealthProbeBindAddress: healthzAddr,
 			WebhookServer: webhook.NewServer(webhook.Options{
 				CertDir: certDir,
 				Port:    port,
-				TLSOpts: []func(*tls.Config){
-					mgrTLSOptions,
-					func(c *tls.Config) {
-						c.MinVersion = tlsVersion(tlsMinVersion)
-					},
-				},
+				TLSOpts: webhookTLSOpts,
 			}),
 		})
 		if err != nil {
@@ -234,4 +257,6 @@ func init() {
 		" Full lists of available ciphers can be found at https://pkg.go.dev/crypto/tls#pkg-constants."+
 		" E.g. 'TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256'")
 	webhookCmd.Flags().StringVar(&tlsMinVersion, "tls-min-version", "1.2", "minimum version of TLS supported.")
+	webhookCmd.Flags().BoolVar(&enableHTTP2, "enable-http2", false,
+		"If set, HTTP/2 will be enabled for the metrics and webhook server")
 }

deploy/charts/external-secrets/README.md

@@ -100,6 +100,7 @@ The command removes all the Kubernetes components associated with the chart and
 | deploymentAnnotations | object | `{}` | Annotations to add to Deployment |
 | dnsConfig | object | `{}` | Specifies `dnsOptions` to deployment |
 | dnsPolicy | string | `"ClusterFirst"` | Specifies `dnsPolicy` to deployment |
+| enableHTTP2 | bool | `false` | if true, HTTP2 will be enabled for the services created by all controllers, curently metrics and webhook. |
 | extendedMetricLabels | bool | `false` | If true external secrets will use recommended kubernetes annotations as prometheus metric labels. |
 | extraArgs | object | `{}` |  |
 | extraContainers | list | `[]` |  |

deploy/charts/external-secrets/templates/cert-controller-deployment.yaml

@@ -73,6 +73,9 @@ spec:
           {{- if .Values.installCRDs }}
           - --enable-partial-cache=true
           {{- end }}
+          {{- if .Values.enableHTTP2 }}
+          - --enable-http2=true
+          {{- end }}
           {{- range $key, $value := .Values.certController.extraArgs }}
             {{- if $value }}
           - --{{ $key }}={{ $value }}

deploy/charts/external-secrets/templates/deployment.yaml

@@ -88,6 +88,9 @@ spec:
           {{- if .Values.extendedMetricLabels }}
           - --enable-extended-metric-labels={{ .Values.extendedMetricLabels }}
           {{- end }}
+          {{- if .Values.enableHTTP2 }}
+          - --enable-http2=true
+          {{- end }}
           {{- if .Values.concurrent }}
           - --concurrent={{ .Values.concurrent }}
           {{- end }}

deploy/charts/external-secrets/templates/webhook-deployment.yaml

@@ -68,6 +68,9 @@ spec:
           {{- if .Values.webhook.lookaheadInterval }}
           - --lookahead-interval={{ .Values.webhook.lookaheadInterval }}
           {{- end }}
+          {{- if .Values.enableHTTP2 }}
+          - --enable-http2=true
+          {{- end }}
           {{- range $key, $value := .Values.webhook.extraArgs }}
             {{- if $value }}
           - --{{ $key }}={{ $value }}

deploy/charts/external-secrets/tests/cert_controller_test.yaml

@@ -210,3 +210,10 @@ tests:
     asserts:
       - hasDocuments:
           count: 0
+  - it: should not have enableHTTP2 flag by default
+    templates:
+      - cert-controller-deployment.yaml
+    asserts:
+      - notContains:
+          path: spec.template.spec.containers[0].args
+          content: "--enable-http2"

deploy/charts/external-secrets/tests/controller_test.yaml

@@ -51,9 +51,9 @@ tests:
     set:
       metrics.listen.port: 8888
     asserts:
-      - equal:
-          path: spec.template.spec.containers[0].args[1]
-          value: "--metrics-addr=:8888"
+      - contains:
+          path: spec.template.spec.containers[0].args
+          content: "--metrics-addr=:8888"
   - it: should override image flavour
     set:
       image.repository: ghcr.io/external-secrets/external-secrets
@@ -117,3 +117,15 @@ tests:
       - equal:
           path: spec.template.spec.containers[0].livenessProbe.httpGet.port
           value: "8080"
+  - it: should update args with enableHTTP2=true
+    set:
+      enableHTTP2: true
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].args
+          content: "--enable-http2=true"
+  - it: should not have enableHTTP2 flag by default
+    asserts:
+      - notContains:
+          path: spec.template.spec.containers[0].args
+          content: "--enable-http2"

deploy/charts/external-secrets/tests/webhook_test.yaml

@@ -424,3 +424,12 @@ tests:
       - equal:
           path: spec.strategy.rollingUpdate.maxUnavailable
           value: 0
+  - it: should update args with enableHTTP2=true
+    set:
+      enableHTTP2: true
+    templates:
+      - webhook-deployment.yaml
+    asserts:
+      - contains:
+          path: spec.template.spec.containers[0].args
+          content: "--enable-http2=true"

deploy/charts/external-secrets/values.schema.json

@@ -300,6 +300,9 @@
         "dnsPolicy": {
             "type": "string"
         },
+        "enableHTTP2": {
+            "type": "boolean"
+        },
         "extendedMetricLabels": {
             "type": "boolean"
         },

deploy/charts/external-secrets/values.yaml

@@ -103,6 +103,9 @@ processPushSecret: true
 # -- Specifies whether an external secret operator deployment be created.
 createOperator: true
 
+# -- if true, HTTP2 will be enabled for the services created by all controllers, curently metrics and webhook.
+enableHTTP2: false
+
 # -- Specifies the number of concurrent ExternalSecret Reconciles external-secret executes at
 # a time.
 concurrent: 1

docs/api/controller-options.md

@@ -35,6 +35,7 @@ The core controller is invoked without a subcommand and can be configured with t
 | `--metrics-addr`                              | string   | :8080   | The address the metric endpoint binds to.                                                                                                                          |
 | `--namespace`                                 | string   | -       | watch external secrets scoped in the provided namespace only. ClusterSecretStore can be used but only work if it doesn't reference resources from other namespaces |
 | `--store-requeue-interval`                    | duration | 5m0s    | Default Time duration between reconciling (Cluster)SecretStores                                                                                                    |
+| `--enable-http2`                              | boolean  | false   | If set, HTTP/2 will be enabled for the metrics server                                                                                                              |
 
 ## Cert Controller Flags
 
@@ -51,20 +52,22 @@ The core controller is invoked without a subcommand and can be configured with t
 | `--secret-namespace`       | string   | default                  | namespace of the secret to store certs                                                                                |
 | `--service-name`           | string   | external-secrets-webhook | Webhook service name                                                                                                  |
 | `--service-namespace`      | string   | default                  | Webhook service namespace                                                                                             |
+| `--enable-http2`           | boolean  | false                    | If set, HTTP/2 will be enabled for the metrics server                                                                 |
 
 ## Webhook Flags
 
 | Name                   | Type     | Default                               | Description                                                                                                                                                                                                                                                                                                                                                                                                              |
-| ---------------------- | -------- | ------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
+|------------------------|----------|---------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `--cert-dir`           | string   | /tmp/k8s-webhook-server/serving-certs | path to check for certs                                                                                                                                                                                                                                                                                                                                                                                                  |
 | `--check-interval`     | duration | 5m0s                                  | certificate check interval                                                                                                                                                                                                                                                                                                                                                                                               |
 | `--dns-name`           | string   | localhost                             | DNS name to validate certificates with                                                                                                                                                                                                                                                                                                                                                                                   |
 | `--healthz-addr`       | string   | :8081                                 | The address the health endpoint binds to.                                                                                                                                                                                                                                                                                                                                                                                |
 | `--help`               |          |                                       | help for webhook                                                                                                                                                                                                                                                                                                                                                                                                         |
 | `--loglevel`           | string   | info                                  | loglevel to use, one of: debug, info, warn, error, dpanic, panic, fatal                                                                                                                                                                                                                                                                                                                                                  |
-| `--zap-time-encoding`                                  | string   | epoch                          | time encoding to use, one of: epoch, millis, nano, iso8601, rfc3339, rfc3339nano                                                                                            |
+| `--zap-time-encoding`  | string   | epoch                                 | time encoding to use, one of: epoch, millis, nano, iso8601, rfc3339, rfc3339nano                                                                                                                                                                                                                                                                                                                                         |
 | `--lookahead-interval` | duration | 2160h0m0s (90d)                       | certificate check interval                                                                                                                                                                                                                                                                                                                                                                                               |
 | `--metrics-addr`       | string   | :8080                                 | The address the metric endpoint binds to.                                                                                                                                                                                                                                                                                                                                                                                |
 | `--port`               | number   | 10250                                 | Port number that the webhook server will serve.                                                                                                                                                                                                                                                                                                                                                                          |
 | `--tls-ciphers`        | string   |                                       | comma separated list of tls ciphers allowed. This does not apply to TLS 1.3 as the ciphers are selected automatically. The order of this list does not give preference to the ciphers, the ordering is done automatically. Full lists of available ciphers can be found at https://pkg.go.dev/crypto/tls#pkg-constants. E.g. 'TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256' |
 | `--tls-min-version`    | string   | 1.2                                   | minimum version of TLS supported.                                                                                                                                                                                                                                                                                                                                                                                        |
+| `--enable-http2`       | boolean  | false                                 | If set, HTTP/2 will be enabled for the metrics and webhook servers                                                                                                                                                                                                                                                                                                                                                       |