update based on reviews

Author: salonichf5

charts/nginx-gateway-fabric/README.md

@@ -245,7 +245,7 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri
 | `nginx.usage.resolver` | The nameserver used to resolve the NGINX Plus usage reporting endpoint. Used with NGINX Instance Manager. | string | `""` |
 | `nginx.usage.secretName` | The name of the Secret containing the JWT for NGINX Plus usage reporting. Must exist in the same namespace that the NGINX Gateway Fabric control plane is running in (default namespace: nginx-gateway). | string | `"nplus-license"` |
 | `nginx.usage.skipVerify` | Disable client verification of the NGINX Plus usage reporting server certificate. | bool | `false` |
-| `nginxGateway` | The nginxGateway section contains configuration for the NGINX Gateway Fabric control plane deployment. | object | `{"affinity":{},"autoscaling":{"enable":false},"config":{"logging":{"level":"info"}},"configAnnotations":{},"extraVolumeMounts":[],"extraVolumes":[],"gatewayClassAnnotations":{},"gatewayClassName":"nginx","gatewayControllerName":"gateway.nginx.org/nginx-gateway-controller","gwAPIExperimentalFeatures":{"enable":false},"gwAPIInferenceExtension":{"enable":false},"image":{"pullPolicy":"Always","repository":"ghcr.io/nginx/nginx-gateway-fabric","tag":"edge"},"kind":"deployment","labels":{},"leaderElection":{"enable":true,"lockName":""},"lifecycle":{},"metrics":{"enable":true,"port":9113,"secure":false},"name":"","nodeSelector":{},"podAnnotations":{},"productTelemetry":{"enable":true},"readinessProbe":{"enable":true,"initialDelaySeconds":3,"port":8081},"replicas":1,"resources":{},"service":{"annotations":{},"labels":{}},"serviceAccount":{"annotations":{},"imagePullSecret":"","imagePullSecrets":[],"name":""},"snippetsFilters":{"enable":false},"terminationGracePeriodSeconds":30,"tolerations":[],"topologySpreadConstraints":[]}` |
+| `nginxGateway` | The nginxGateway section contains configuration for the NGINX Gateway Fabric control plane deployment. | object | `{"affinity":{},"autoscaling":{"enable":false},"config":{"logging":{"level":"info"}},"configAnnotations":{},"extraVolumeMounts":[],"extraVolumes":[],"gatewayClassAnnotations":{},"gatewayClassName":"nginx","gatewayControllerName":"gateway.nginx.org/nginx-gateway-controller","gwAPIExperimentalFeatures":{"enable":false},"gwAPIInferenceExtension":{"enable":false,"endpointPicker":{"disableTLS":false,"enableSecureVerify":false}},"image":{"pullPolicy":"Always","repository":"ghcr.io/nginx/nginx-gateway-fabric","tag":"edge"},"kind":"deployment","labels":{},"leaderElection":{"enable":true,"lockName":""},"lifecycle":{},"metrics":{"enable":true,"port":9113,"secure":false},"name":"","nodeSelector":{},"podAnnotations":{},"productTelemetry":{"enable":true},"readinessProbe":{"enable":true,"initialDelaySeconds":3,"port":8081},"replicas":1,"resources":{},"service":{"annotations":{},"labels":{}},"serviceAccount":{"annotations":{},"imagePullSecret":"","imagePullSecrets":[],"name":""},"snippetsFilters":{"enable":false},"terminationGracePeriodSeconds":30,"tolerations":[],"topologySpreadConstraints":[]}` |
 | `nginxGateway.affinity` | The affinity of the NGINX Gateway Fabric control plane pod. | object | `{}` |
 | `nginxGateway.autoscaling` | Autoscaling configuration for the NGINX Gateway Fabric control plane. | object | `{"enable":false}` |
 | `nginxGateway.autoscaling.enable` | Enable or disable Horizontal Pod Autoscaler for the control plane. | bool | `false` |
@@ -258,6 +258,9 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri
 | `nginxGateway.gatewayControllerName` | The name of the Gateway controller. The controller name must be of the form: DOMAIN/PATH. The controller's domain is gateway.nginx.org. | string | `"gateway.nginx.org/nginx-gateway-controller"` |
 | `nginxGateway.gwAPIExperimentalFeatures.enable` | Enable the experimental features of Gateway API which are supported by NGINX Gateway Fabric. Requires the Gateway APIs installed from the experimental channel. | bool | `false` |
 | `nginxGateway.gwAPIInferenceExtension.enable` | Enable Gateway API Inference Extension support. Allows for configuring InferencePools to route traffic to AI workloads. | bool | `false` |
+| `nginxGateway.gwAPIInferenceExtension.endpointPicker` | EndpointPicker TLS configuration. | object | `{"disableTLS":false,"enableSecureVerify":false}` |
+| `nginxGateway.gwAPIInferenceExtension.endpointPicker.disableTLS` | Disable TLS for EndpointPicker communication. By default, TLS is enabled. Set to true only for development/testing or when using a service mesh for encryption. | bool | `false` |
+| `nginxGateway.gwAPIInferenceExtension.endpointPicker.enableSecureVerify` | Enable TLS certificate verification when connecting to the EndpointPicker. By default, certificate verification is disabled. REQUIRED: Must be false until Gateway API Inference Extension EndpointPicker supports mounting certificates. See: https://github.com/kubernetes-sigs/gateway-api-inference-extension/issues/1556 | bool | `false` |
 | `nginxGateway.image` | The image configuration for the NGINX Gateway Fabric control plane. | object | `{"pullPolicy":"Always","repository":"ghcr.io/nginx/nginx-gateway-fabric","tag":"edge"}` |
 | `nginxGateway.image.repository` | The NGINX Gateway Fabric image to use | string | `"ghcr.io/nginx/nginx-gateway-fabric"` |
 | `nginxGateway.kind` | The kind of the NGINX Gateway Fabric installation - currently, only deployment is supported. | string | `"deployment"` |

charts/nginx-gateway-fabric/templates/deployment.yaml

@@ -103,6 +103,12 @@ spec:
         {{- if .Values.nginxGateway.gwAPIInferenceExtension.enable }}
         - --gateway-api-inference-extension
         {{- end }}
+        {{- if .Values.nginxGateway.gwAPIInferenceExtension.endpointPicker.disableTLS }}
+        - --endpoint-picker-disable-tls
+        {{- end }}
+        {{- if .Values.nginxGateway.gwAPIInferenceExtension.endpointPicker.enableSecureVerify }}
+        - --endpoint-picker-enable-secure-verify
+        {{- end }}
         {{- if .Values.nginxGateway.snippetsFilters.enable }}
         - --snippets-filters
         {{- end }}

charts/nginx-gateway-fabric/values.schema.json

@@ -846,6 +846,28 @@
               "required": [],
               "title": "enable",
               "type": "boolean"
+            },
+            "endpointPicker": {
+              "description": "EndpointPicker TLS configuration.",
+              "properties": {
+                "disableTLS": {
+                  "default": false,
+                  "description": "Disable TLS for EndpointPicker communication. By default, TLS is enabled.\nSet to true only for development/testing or when using a service mesh for encryption.",
+                  "required": [],
+                  "title": "disableTLS",
+                  "type": "boolean"
+                },
+                "enableSecureVerify": {
+                  "default": false,
+                  "description": "Enable TLS certificate verification when connecting to the EndpointPicker.\nBy default, certificate verification is disabled.\nREQUIRED: Must be false until Gateway API Inference Extension EndpointPicker supports mounting certificates.\nSee: https://github.com/kubernetes-sigs/gateway-api-inference-extension/issues/1556",
+                  "required": [],
+                  "title": "enableSecureVerify",
+                  "type": "boolean"
+                }
+              },
+              "required": [],
+              "title": "endpointPicker",
+              "type": "object"
             }
           },
           "required": [],

charts/nginx-gateway-fabric/values.yaml

@@ -214,6 +214,19 @@ nginxGateway:
     # -- Enable Gateway API Inference Extension support. Allows for configuring InferencePools to route traffic to AI workloads.
     enable: false
 
+    # -- EndpointPicker TLS configuration.
+    endpointPicker:
+      # -- Disable TLS for EndpointPicker communication. By default, TLS is enabled.
+      # Set to true only for development/testing or when using a service mesh for encryption.
+      disableTLS: false
+
+      # -- Enable TLS certificate verification when connecting to the EndpointPicker.
+      # By default, certificate verification is disabled.
+      # REQUIRED: Must be false until Gateway API Inference Extension EndpointPicker supports mounting certificates.
+      # See: https://github.com/kubernetes-sigs/gateway-api-inference-extension/issues/1556
+      enableSecureVerify: false
+
+
   snippetsFilters:
     # -- Enable SnippetsFilters feature. SnippetsFilters allow inserting NGINX configuration into the generated NGINX
     # config for HTTPRoute and GRPCRoute resources.

cmd/gateway/commands.go

@@ -37,17 +37,17 @@ const (
 		`The controller name must be of the form: DOMAIN/PATH. The controller's domain is '%s'`
 	plusFlag = "nginx-plus"
 
-	serverTLSSecret                    = "server-tls"
-	agentTLSSecret                     = "agent-tls"
-	nginxOneTelemetryEndpointHost      = "agent.connect.nginx.com"
-	endpointPickerEnableTLSFlag        = "endpoint-picker-enable-tls"
-	endpointPickerSkipSecureVerifyFlag = "endpoint-picker-skip-secure-verify"
+	serverTLSSecret                      = "server-tls"
+	agentTLSSecret                       = "agent-tls"
+	nginxOneTelemetryEndpointHost        = "agent.connect.nginx.com"
+	endpointPickerDisableTLSFlag         = "endpoint-picker-disable-tls"
+	endpointPickerEnableSecureVerifyFlag = "endpoint-picker-enable-secure-verify"
 )
 
 // common flags.
 var (
-	endpointPickerEnableTLS        bool
-	endpointPickerSkipSecureVerify bool
+	endpointPickerDisableTLS         bool
+	endpointPickerEnableSecureVerify bool
 )
 
 // usageReportParams holds the parameters for building the usage report configuration for PLUS.
@@ -296,8 +296,8 @@ func createControllerCommand() *cobra.Command {
 					EndpointPort:           nginxOneConsoleTelemetryEndpointPort.value,
 					EndpointTLSSkipVerify:  nginxOneConsoleTLSSkipVerify,
 				},
-				EndpointPickerEnableTLS:        endpointPickerEnableTLS,
-				EndpointPickerSkipSecureVerify: endpointPickerSkipSecureVerify,
+				EndpointPickerDisableTLS:         endpointPickerDisableTLS,
+				EndpointPickerEnableSecureVerify: endpointPickerEnableSecureVerify,
 			}
 
 			if err := controller.StartManager(conf); err != nil {
@@ -330,20 +330,6 @@ func createControllerCommand() *cobra.Command {
 			` Lives in the same Namespace as the controller.`,
 	)
 
-	cmd.Flags().BoolVar(
-		&endpointPickerEnableTLS,
-		endpointPickerEnableTLSFlag,
-		true,
-		"Enables TLS when connecting to the endpoint picker.",
-	)
-
-	cmd.Flags().BoolVar(
-		&endpointPickerSkipSecureVerify,
-		endpointPickerSkipSecureVerifyFlag,
-		true,
-		"Disables server certificate verification when connecting to the endpoint picker, if TLS is enabled",
-	)
-
 	cmd.Flags().Var(
 		&serviceName,
 		serviceFlag,
@@ -465,6 +451,8 @@ func createControllerCommand() *cobra.Command {
 			"traffic to AI workloads.",
 	)
 
+	addEPPConnectionFlags(cmd)
+
 	cmd.Flags().Var(
 		&nginxDockerSecrets,
 		nginxDockerSecretFlag,
@@ -788,28 +776,34 @@ func createEndpointPickerCommand() *cobra.Command {
 		RunE: func(_ *cobra.Command, _ []string) error {
 			logger := ctlrZap.New().WithName("endpoint-picker-shim")
 			handler := createEndpointPickerHandler(
-				realExtProcClientFactory(endpointPickerEnableTLS, endpointPickerSkipSecureVerify),
+				realExtProcClientFactory(endpointPickerDisableTLS, endpointPickerEnableSecureVerify),
 				logger,
 			)
 			return endpointPickerServer(handler)
 		},
 	}
 
+	addEPPConnectionFlags(cmd)
+
+	return cmd
+}
+
+func addEPPConnectionFlags(cmd *cobra.Command) {
 	cmd.Flags().BoolVar(
-		&endpointPickerEnableTLS,
-		endpointPickerEnableTLSFlag,
-		true,
-		"Enables TLS when connecting to the endpoint picker.",
+		&endpointPickerDisableTLS,
+		endpointPickerDisableTLSFlag,
+		false,
+		"Disables TLS when connecting to the EndpointPicker. "+
+			"Set to true only for development/testing or when using a service mesh for encryption.",
 	)
 
 	cmd.Flags().BoolVar(
-		&endpointPickerSkipSecureVerify,
-		endpointPickerSkipSecureVerifyFlag,
-		true,
-		"Disables server certificate verification when connecting to the endpoint picker, if TLS is enabled",
+		&endpointPickerEnableSecureVerify,
+		endpointPickerEnableSecureVerifyFlag,
+		false,
+		"Enables server certificate verification when connecting to the EndpointPicker, if TLS is enabled. "+
+			"REQUIRED: Must be false until Gateway API Inference Extension EndpointPicker supports mounting certificates.",
 	)
-
-	return cmd
 }
 
 func parseFlags(flags *pflag.FlagSet) ([]string, []string) {

cmd/gateway/commands_test.go

@@ -161,8 +161,8 @@ func TestControllerCmdFlagValidation(t *testing.T) {
 				"--nginx-one-telemetry-endpoint-host=telemetry-endpoint-host",
 				"--nginx-one-telemetry-endpoint-port=443",
 				"--nginx-one-tls-skip-verify",
-				"--endpoint-picker-enable-tls",
-				"--endpoint-picker-skip-secure-verify",
+				"--endpoint-picker-disable-tls",
+				"--endpoint-picker-enable-secure-verify",
 			},
 			wantErr: false,
 		},
@@ -931,29 +931,37 @@ func TestEndpointPickerFlags(t *testing.T) {
 	t.Parallel()
 	tests := []flagTestCase{
 		{
-			name: "valid flags",
+			name: "valid flags with default values",
+			args: []string{
+				"--endpoint-picker-disable-tls=false",
+				"--endpoint-picker-enable-secure-verify=false",
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid flags with changed values",
 			args: []string{
-				"--endpoint-picker-enable-tls=true",
-				"--endpoint-picker-skip-secure-verify=false",
+				"--endpoint-picker-disable-tls=true",
+				"--endpoint-picker-enable-secure-verify=true",
 			},
 			wantErr: false,
 		},
 		{
-			name: "endpoint-picker-enable-tls is not a bool",
+			name: "endpoint-picker-disable-tls is not a bool",
 			args: []string{
-				"--endpoint-picker-enable-tls=not-a-bool",
+				"--endpoint-picker-disable-tls=not-a-bool",
 			},
 			wantErr: true,
-			expectedErrPrefix: `invalid argument "not-a-bool" for "--endpoint-picker-enable-tls" flag:` +
+			expectedErrPrefix: `invalid argument "not-a-bool" for "--endpoint-picker-disable-tls" flag:` +
 				` strconv.ParseBool: parsing "not-a-bool": invalid syntax`,
 		},
 		{
-			name: "endpoint-picker-skip-secure-verify is not a bool",
+			name: "endpoint-picker-enable-secure-verify is not a bool",
 			args: []string{
-				"--endpoint-picker-skip-secure-verify=not-a-bool",
+				"--endpoint-picker-enable-secure-verify=not-a-bool",
 			},
 			wantErr: true,
-			expectedErrPrefix: `invalid argument "not-a-bool" for "--endpoint-picker-skip-secure-verify" flag:` +
+			expectedErrPrefix: `invalid argument "not-a-bool" for "--endpoint-picker-enable-secure-verify" flag:` +
 				` strconv.ParseBool: parsing "not-a-bool": invalid syntax`,
 		},
 	}

cmd/gateway/endpoint_picker.go

@@ -35,15 +35,15 @@ func endpointPickerServer(handler http.Handler) error {
 }
 
 // realExtProcClientFactory returns a factory that creates a new gRPC connection and client per request.
-func realExtProcClientFactory(enableTLS, skipSecureVerify bool) extProcClientFactory {
+func realExtProcClientFactory(disableTLS, enableSecureVerify bool) extProcClientFactory {
 	return func(target string) (extprocv3.ExternalProcessorClient, func() error, error) {
 		var opts []grpc.DialOption
 
-		if !enableTLS {
+		if disableTLS {
 			opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
 		} else {
 			creds := credentials.NewTLS(&tls.Config{
-				InsecureSkipVerify: skipSecureVerify, //nolint:gosec
+				InsecureSkipVerify: !enableSecureVerify, //nolint:gosec
 			})
 			opts = append(opts, grpc.WithTransportCredentials(creds))
 		}

internal/controller/config/config.go

@@ -52,10 +52,10 @@ type Config struct {
 	InferenceExtension bool
 	// SnippetsFilters indicates if SnippetsFilters are enabled.
 	SnippetsFilters bool
-	// EndpointPickerEnableTLS indicates if TLS is enabled for EndpointPicker communication.
-	EndpointPickerEnableTLS bool
-	// EndpointPickerSkipSecureVerify indicates if secure verification is skipped for EndpointPicker communication.
-	EndpointPickerSkipSecureVerify bool
+	// EndpointPickerDisableTLS indicates if TLS is disabled for EndpointPicker communication.
+	EndpointPickerDisableTLS bool
+	// EndpointPickerEnableSecureVerify indicates if secure verification is enabled for EndpointPicker communication.
+	EndpointPickerEnableSecureVerify bool
 }
 
 // GatewayPodConfig contains information about this Pod.

internal/controller/manager.go

@@ -208,21 +208,21 @@ func StartManager(cfg config.Config) error {
 		ctx,
 		mgr,
 		provisioner.Config{
-			DeploymentStore:                nginxUpdater.NginxDeployments,
-			StatusQueue:                    statusQueue,
-			Logger:                         cfg.Logger.WithName("provisioner"),
-			EventRecorder:                  recorder,
-			GatewayPodConfig:               &cfg.GatewayPodConfig,
-			GCName:                         cfg.GatewayClassName,
-			AgentTLSSecretName:             cfg.AgentTLSSecretName,
-			NGINXSCCName:                   cfg.NGINXSCCName,
-			Plus:                           cfg.Plus,
-			NginxDockerSecretNames:         cfg.NginxDockerSecretNames,
-			PlusUsageConfig:                &cfg.UsageReportConfig,
-			NginxOneConsoleTelemetryConfig: cfg.NginxOneConsoleTelemetryConfig,
-			InferenceExtension:             cfg.InferenceExtension,
-			EndpointPickerEnableTLS:        cfg.EndpointPickerEnableTLS,
-			EndpointPickerSkipSecureVerify: cfg.EndpointPickerSkipSecureVerify,
+			DeploymentStore:                  nginxUpdater.NginxDeployments,
+			StatusQueue:                      statusQueue,
+			Logger:                           cfg.Logger.WithName("provisioner"),
+			EventRecorder:                    recorder,
+			GatewayPodConfig:                 &cfg.GatewayPodConfig,
+			GCName:                           cfg.GatewayClassName,
+			AgentTLSSecretName:               cfg.AgentTLSSecretName,
+			NGINXSCCName:                     cfg.NGINXSCCName,
+			Plus:                             cfg.Plus,
+			NginxDockerSecretNames:           cfg.NginxDockerSecretNames,
+			PlusUsageConfig:                  &cfg.UsageReportConfig,
+			NginxOneConsoleTelemetryConfig:   cfg.NginxOneConsoleTelemetryConfig,
+			InferenceExtension:               cfg.InferenceExtension,
+			EndpointPickerDisableTLS:         cfg.EndpointPickerDisableTLS,
+			EndpointPickerEnableSecureVerify: cfg.EndpointPickerEnableSecureVerify,
 		},
 	)
 	if err != nil {

internal/controller/provisioner/objects.go

@@ -1126,11 +1126,11 @@ func (p *NginxProvisioner) buildNginxPodTemplateSpec(
 			"endpoint-picker",
 		}
 
-		if p.cfg.EndpointPickerEnableTLS {
-			command = append(command, "--endpoint-picker-enable-tls")
+		if p.cfg.EndpointPickerDisableTLS {
+			command = append(command, "--endpoint-picker-disable-tls")
 		}
-		if p.cfg.EndpointPickerSkipSecureVerify {
-			command = append(command, "--endpoint-picker-skip-secure-verify")
+		if p.cfg.EndpointPickerEnableSecureVerify {
+			command = append(command, "--endpoint-picker-enable-secure-verify")
 		}
 
 		spec.Spec.Containers = append(spec.Spec.Containers, corev1.Container{