CP/DP Split: provision NGINX Plus (#3148)

Continuation from the previous commit to add support for provisioning with NGINX Plus. This adds support for duplicating any NGINX Plus or docker registry secrets into the Gateway namespace.

Added unit tests.

Author: sjberman

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

@@ -36,6 +36,14 @@ spec:
         - --gatewayclass={{ .Values.nginxGateway.gatewayClassName }}
         - --config={{ include "nginx-gateway.config-name" . }}
         - --service={{ include "nginx-gateway.fullname" . }}
+        {{- if .Values.nginx.imagePullSecret }}
+        - --nginx-docker-secret={{ .Values.nginx.imagePullSecret }}
+        {{- end }}
+        {{- if .Values.nginx.imagePullSecrets }}
+          {{- range .Values.nginx.imagePullSecrets }}
+        - --nginx-docker-secret={{ . }}
+          {{- end }}
+        {{- end }}
         {{- if .Values.nginx.plus }}
         - --nginx-plus
           {{- if .Values.nginx.usage.secretName }}

cmd/gateway/commands.go

@@ -69,6 +69,7 @@ func createStaticModeCommand() *cobra.Command {
 		leaderElectionLockNameFlag     = "leader-election-lock-name"
 		productTelemetryDisableFlag    = "product-telemetry-disable"
 		gwAPIExperimentalFlag          = "gateway-api-experimental-features"
+		nginxDockerSecretFlag          = "nginx-docker-secret" //nolint:gosec // not credentials
 		usageReportSecretFlag          = "usage-report-secret"
 		usageReportEndpointFlag        = "usage-report-endpoint"
 		usageReportResolverFlag        = "usage-report-resolver"
@@ -120,7 +121,10 @@ func createStaticModeCommand() *cobra.Command {
 
 		snippetsFilters bool
 
-		plus                  bool
+		plus               bool
+		nginxDockerSecrets = stringSliceValidatingValue{
+			validator: validateResourceName,
+		}
 		usageReportSkipVerify bool
 		usageReportSecretName = stringValidatingValue{
 			validator: validateResourceName,
@@ -249,7 +253,8 @@ func createStaticModeCommand() *cobra.Command {
 					Names:  flagKeys,
 					Values: flagValues,
 				},
-				SnippetsFilters: snippetsFilters,
+				SnippetsFilters:        snippetsFilters,
+				NginxDockerSecretNames: nginxDockerSecrets.values,
 			}
 
 			if err := static.StartManager(conf); err != nil {
@@ -378,6 +383,13 @@ func createStaticModeCommand() *cobra.Command {
 			"Requires the Gateway APIs installed from the experimental channel.",
 	)
 
+	cmd.Flags().Var(
+		&nginxDockerSecrets,
+		nginxDockerSecretFlag,
+		"The name of the NGINX docker registry Secret(s). Must exist in the same namespace "+
+			"that the NGINX Gateway Fabric control plane is running in (default namespace: nginx-gateway).",
+	)
+
 	cmd.Flags().Var(
 		&usageReportSecretName,
 		usageReportSecretFlag,

cmd/gateway/commands_test.go

@@ -153,6 +153,8 @@ func TestStaticModeCmdFlagValidation(t *testing.T) {
 				"--leader-election-lock-name=my-lock",
 				"--leader-election-disable=false",
 				"--nginx-plus",
+				"--nginx-docker-secret=secret1",
+				"--nginx-docker-secret=secret2",
 				"--usage-report-secret=my-secret",
 				"--usage-report-endpoint=example.com",
 				"--usage-report-resolver=resolver.com",
@@ -314,6 +316,31 @@ func TestStaticModeCmdFlagValidation(t *testing.T) {
 			wantErr:           true,
 			expectedErrPrefix: `invalid argument "" for "--leader-election-disable" flag: strconv.ParseBool`,
 		},
+		{
+			name: "nginx-docker-secret is set to empty string",
+			args: []string{
+				"--nginx-docker-secret=",
+			},
+			wantErr:           true,
+			expectedErrPrefix: `invalid argument "" for "--nginx-docker-secret" flag: must be set`,
+		},
+		{
+			name: "nginx-docker-secret is invalid",
+			args: []string{
+				"--nginx-docker-secret=!@#$",
+			},
+			wantErr:           true,
+			expectedErrPrefix: `invalid argument "!@#$" for "--nginx-docker-secret" flag: invalid format: `,
+		},
+		{
+			name: "one nginx-docker-secret is invalid",
+			args: []string{
+				"--nginx-docker-secret=valid",
+				"--nginx-docker-secret=!@#$",
+			},
+			wantErr:           true,
+			expectedErrPrefix: `invalid argument "!@#$" for "--nginx-docker-secret" flag: invalid format: `,
+		},
 		{
 			name: "usage-report-secret is set to empty string",
 			args: []string{

cmd/gateway/validating_types.go

@@ -1,8 +1,11 @@
 package main
 
 import (
+	"bytes"
+	"encoding/csv"
 	"fmt"
 	"strconv"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -30,6 +33,53 @@ func (v *stringValidatingValue) Type() string {
 	return "string"
 }
 
+// stringSliceValidatingValue is a string slice flag value with custom validation logic.
+// it implements the pflag.Value interface.
+type stringSliceValidatingValue struct {
+	validator func(v string) error
+	values    []string
+	changed   bool
+}
+
+func (v *stringSliceValidatingValue) String() string {
+	b := &bytes.Buffer{}
+	w := csv.NewWriter(b)
+	err := w.Write(v.values)
+	if err != nil {
+		return ""
+	}
+
+	w.Flush()
+	str := strings.TrimSuffix(b.String(), "\n")
+	return "[" + str + "]"
+}
+
+func (v *stringSliceValidatingValue) Set(param string) error {
+	if err := v.validator(param); err != nil {
+		return err
+	}
+
+	stringReader := strings.NewReader(param)
+	csvReader := csv.NewReader(stringReader)
+	str, err := csvReader.Read()
+	if err != nil {
+		return err
+	}
+
+	if !v.changed {
+		v.values = str
+	} else {
+		v.values = append(v.values, str...)
+	}
+	v.changed = true
+
+	return nil
+}
+
+func (v *stringSliceValidatingValue) Type() string {
+	return "stringSlice"
+}
+
 type intValidatingValue struct {
 	validator func(v int) error
 	value     int

deploy/experimental-nginx-plus/deploy.yaml

@@ -209,6 +209,7 @@ spec:
         - --gatewayclass=nginx
         - --config=nginx-gateway-config
         - --service=nginx-gateway
+        - --nginx-docker-secret=nginx-plus-registry-secret
         - --nginx-plus
         - --usage-report-secret=nplus-license
         - --metrics-port=9113

deploy/nginx-plus/deploy.yaml

@@ -205,6 +205,7 @@ spec:
         - --gatewayclass=nginx
         - --config=nginx-gateway-config
         - --service=nginx-gateway
+        - --nginx-docker-secret=nginx-plus-registry-secret
         - --nginx-plus
         - --usage-report-secret=nplus-license
         - --metrics-port=9113

deploy/snippets-filters-nginx-plus/deploy.yaml

@@ -207,6 +207,7 @@ spec:
         - --gatewayclass=nginx
         - --config=nginx-gateway-config
         - --service=nginx-gateway
+        - --nginx-docker-secret=nginx-plus-registry-secret
         - --nginx-plus
         - --usage-report-secret=nplus-license
         - --metrics-port=9113

internal/framework/controller/resource.go

@@ -4,6 +4,6 @@ import "fmt"
 
 // CreateNginxResourceName creates the base resource name for all nginx resources
 // created by the control plane.
-func CreateNginxResourceName(gatewayName, gatewayClassName string) string {
-	return fmt.Sprintf("%s-%s", gatewayName, gatewayClassName)
+func CreateNginxResourceName(prefix, suffix string) string {
+	return fmt.Sprintf("%s-%s", prefix, suffix)
 }

internal/mode/static/config/config.go

@@ -32,6 +32,8 @@ type Config struct {
 	ConfigName string
 	// GatewayClassName is the name of the GatewayClass resource that the Gateway will use.
 	GatewayClassName string
+	// NginxDockerSecretNames are the names of any Docker registry Secrets for the NGINX container.
+	NginxDockerSecretNames []string
 	// LeaderElection contains the configuration for leader election.
 	LeaderElection LeaderElectionConfig
 	// ProductTelemetryConfig contains the configuration for collecting product telemetry.

internal/mode/static/handler.go

@@ -169,14 +169,19 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log
 	h.sendNginxConfig(ctx, logger, gr, changeType)
 }
 
+// enable is called when the pod becomes leader to ensure the provisioner has
+// the latest configuration.
+func (h *eventHandlerImpl) enable(ctx context.Context) {
+	h.sendNginxConfig(ctx, h.cfg.logger, h.cfg.processor.GetLatestGraph(), state.ClusterStateChange)
+}
+
 func (h *eventHandlerImpl) sendNginxConfig(
 	ctx context.Context,
 	logger logr.Logger,
 	gr *graph.Graph,
 	changeType state.ChangeType,
 ) {
 	if gr == nil {
-		logger.Info("Handling events didn't result into NGINX configuration changes")
 		return
 	}
 
@@ -246,13 +251,13 @@ func (h *eventHandlerImpl) processStateAndBuildConfig(
 
 		h.setLatestConfiguration(&cfg)
 
-		deployment.Lock.Lock()
+		deployment.FileLock.Lock()
 		if h.cfg.plus {
 			configApplied = h.cfg.nginxUpdater.UpdateUpstreamServers(deployment, cfg)
 		} else {
 			configApplied = h.updateNginxConf(deployment, cfg)
 		}
-		deployment.Lock.Unlock()
+		deployment.FileLock.Unlock()
 	case state.ClusterStateChange:
 		h.version++
 		cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version, h.cfg.plus)
@@ -264,9 +269,9 @@ func (h *eventHandlerImpl) processStateAndBuildConfig(
 
 		h.setLatestConfiguration(&cfg)
 
-		deployment.Lock.Lock()
+		deployment.FileLock.Lock()
 		configApplied = h.updateNginxConf(deployment, cfg)
-		deployment.Lock.Unlock()
+		deployment.FileLock.Unlock()
 	}
 
 	return configApplied

internal/mode/static/manager.go

@@ -201,13 +201,15 @@ 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,
-			Plus:             cfg.Plus,
+			DeploymentStore:        nginxUpdater.NginxDeployments,
+			StatusQueue:            statusQueue,
+			Logger:                 cfg.Logger.WithName("provisioner"),
+			EventRecorder:          recorder,
+			GatewayPodConfig:       &cfg.GatewayPodConfig,
+			GCName:                 cfg.GatewayClassName,
+			Plus:                   cfg.Plus,
+			NginxDockerSecretNames: cfg.NginxDockerSecretNames,
+			PlusUsageConfig:        &cfg.UsageReportConfig,
 		},
 	)
 	if err != nil {
@@ -265,6 +267,7 @@ func StartManager(cfg config.Config) error {
 	if err = mgr.Add(runnables.NewCallFunctionsAfterBecameLeader([]func(context.Context){
 		groupStatusUpdater.Enable,
 		nginxProvisioner.Enable,
+		eventHandler.enable,
 	})); err != nil {
 		return fmt.Errorf("cannot register functions that get called after Pod becomes leader: %w", err)
 	}