From c5d2e656a465127b54f89f33c88002ffb61c5ed1 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Thu, 5 Dec 2024 16:52:47 -0700 Subject: [PATCH 01/11] Write deployment context in init container --- .../templates/configmap.yaml | 1 + .../templates/deployment.yaml | 14 +- cmd/gateway/commands.go | 104 +++++++----- cmd/gateway/commands_test.go | 24 +-- cmd/gateway/initialize.go | 106 ++++++++++++ cmd/gateway/initialize_test.go | 84 ++++++++++ cmd/gateway/main.go | 2 +- cmd/gateway/validation.go | 4 +- cmd/gateway/validation_test.go | 2 +- config/tests/static-deployment.yaml | 13 +- deploy/aws-nlb/deploy.yaml | 13 +- deploy/azure/deploy.yaml | 13 +- deploy/default/deploy.yaml | 13 +- deploy/experimental-nginx-plus/deploy.yaml | 15 +- deploy/experimental/deploy.yaml | 13 +- deploy/nginx-plus/deploy.yaml | 15 +- deploy/nodeport/deploy.yaml | 13 +- deploy/openshift/deploy.yaml | 13 +- .../snippets-filters-nginx-plus/deploy.yaml | 15 +- deploy/snippets-filters/deploy.yaml | 13 +- internal/mode/static/handler.go | 64 +++---- internal/mode/static/handler_test.go | 117 +------------ internal/mode/static/licensing/collector.go | 70 ++++++++ .../static/licensing/collector_suite_test.go | 14 ++ .../mode/static/licensing/collector_test.go | 126 ++++++++++++++ internal/mode/static/manager.go | 26 +-- .../mode/static/nginx/config/main_config.go | 28 +++- .../nginx/config/main_config_template.go | 4 +- .../file/filefakes/fake_osfile_manager.go | 156 ++++++++++++++++++ internal/mode/static/nginx/file/folders.go | 10 +- .../mode/static/nginx/file/folders_test.go | 37 +++++ internal/mode/static/nginx/file/manager.go | 9 +- .../mode/static/nginx/file/os_filemanager.go | 10 ++ 33 files changed, 886 insertions(+), 275 deletions(-) create mode 100644 cmd/gateway/initialize.go create mode 100644 cmd/gateway/initialize_test.go create mode 100644 internal/mode/static/licensing/collector.go create mode 100644 internal/mode/static/licensing/collector_suite_test.go create mode 100644 internal/mode/static/licensing/collector_test.go diff --git a/charts/nginx-gateway-fabric/templates/configmap.yaml b/charts/nginx-gateway-fabric/templates/configmap.yaml index 69586d5ec3..8b99c60650 100644 --- a/charts/nginx-gateway-fabric/templates/configmap.yaml +++ b/charts/nginx-gateway-fabric/templates/configmap.yaml @@ -29,5 +29,6 @@ data: ssl_certificate_key /etc/nginx/certs-bootstrap/tls.key; {{- end }} enforce_initial_report off; + deployment_context /etc/nginx/main-includes/deployment_ctx.json; } {{- end }} diff --git a/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index 0985a5bc21..94832a43c2 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -34,20 +34,30 @@ spec: {{- toYaml .Values.topologySpreadConstraints | nindent 8 }} {{- end }} initContainers: - - name: copy-nginx-config + - name: init image: {{ .Values.nginxGateway.image.repository }}:{{ default .Chart.AppVersion .Values.nginxGateway.image.tag }} imagePullPolicy: {{ .Values.nginxGateway.image.pullPolicy }} command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf {{- if .Values.nginx.plus }} - --source - /includes/mgmt.conf + - --nginx-plus {{- end }} - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name securityContext: seccompProfile: type: RuntimeDefault diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 72b64db403..96d08f0b40 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -3,9 +3,7 @@ package main import ( "errors" "fmt" - "io" "os" - "path/filepath" "runtime/debug" "strconv" "time" @@ -22,8 +20,10 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/provisioner" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" ) +// These flags are shared by mutliple commands. const ( domain = "gateway.nginx.org" gatewayClassFlag = "gatewayclass" @@ -32,6 +32,7 @@ const ( gatewayCtlrNameFlag = "gateway-ctlr-name" gatewayCtlrNameUsageFmt = `The name of the Gateway controller. ` + `The controller name must be of the form: DOMAIN/PATH. The controller's domain is '%s'` + plusFlag = "nginx-plus" ) func createRootCommand() *cobra.Command { @@ -47,7 +48,6 @@ func createRootCommand() *cobra.Command { return rootCmd } -//nolint:gocyclo func createStaticModeCommand() *cobra.Command { // flag names const ( @@ -63,7 +63,6 @@ func createStaticModeCommand() *cobra.Command { leaderElectionDisableFlag = "leader-election-disable" leaderElectionLockNameFlag = "leader-election-lock-name" productTelemetryDisableFlag = "product-telemetry-disable" - plusFlag = "nginx-plus" gwAPIExperimentalFlag = "gateway-api-experimental-features" usageReportSecretFlag = "usage-report-secret" usageReportEndpointFlag = "usage-report-endpoint" @@ -164,14 +163,9 @@ func createStaticModeCommand() *cobra.Command { return fmt.Errorf("error validating POD_IP environment variable: %w", err) } - namespace := os.Getenv("POD_NAMESPACE") - if namespace == "" { - return errors.New("POD_NAMESPACE environment variable must be set") - } - - podName := os.Getenv("POD_NAME") - if podName == "" { - return errors.New("POD_NAME environment variable must be set") + podNsName, err := getPodNsName() + if err != nil { + return fmt.Errorf("could not get pod nsname: %w", err) } imageSource := os.Getenv("BUILD_AGENT") @@ -229,8 +223,8 @@ func createStaticModeCommand() *cobra.Command { GatewayPodConfig: config.GatewayPodConfig{ PodIP: podIP, ServiceName: serviceName.value, - Namespace: namespace, - Name: podName, + Namespace: podNsName.Namespace, + Name: podNsName.Name, }, HealthConfig: config.HealthConfig{ Enabled: !disableHealth, @@ -244,7 +238,7 @@ func createStaticModeCommand() *cobra.Command { LeaderElection: config.LeaderElectionConfig{ Enabled: !disableLeaderElection, LockName: leaderElectionLockName.String(), - Identity: podName, + Identity: podNsName.Name, }, UsageReportConfig: usageReportConfig, ProductTelemetryConfig: config.ProductTelemetryConfig{ @@ -524,29 +518,50 @@ func createSleepCommand() *cobra.Command { return cmd } -func createCopyCommand() *cobra.Command { +func createInitializeCommand() *cobra.Command { // flag names const srcFlag = "source" const destFlag = "destination" + // flag values var srcFiles []string var dest string + var plus bool cmd := &cobra.Command{ - Use: "copy", - Short: "Copy files to another directory", + Use: "initialize", + Short: "Write initial configuration files", RunE: func(_ *cobra.Command, _ []string) error { - if err := validateSleepArgs(srcFiles, dest); err != nil { + if err := validateCopyArgs(srcFiles, dest); err != nil { return err } - for _, src := range srcFiles { - if err := copyFile(src, dest); err != nil { - return err - } + podNsName, err := getPodNsName() + if err != nil { + return fmt.Errorf("could not get pod nsname: %w", err) } - return nil + logger := ctlrZap.New() + klog.SetLogger(logger) + logger.Info( + "Starting init container", + "source filenames to copy", srcFiles, + "destination directory", dest, + "nginx-plus", + plus, + ) + log.SetLogger(logger) + + return initialize(initializeConfig{ + controllerPodNSName: podNsName, + fileManager: file.NewStdLibOSFileManager(), + logger: logger, + plus: plus, + copy: copyFiles{ + srcFileNames: srcFiles, + destDirName: dest, + }, + }) }, } @@ -564,31 +579,18 @@ func createCopyCommand() *cobra.Command { "The destination directory for the source files to be copied to", ) + cmd.Flags().BoolVar( + &plus, + plusFlag, + false, + "Use NGINX Plus", + ) + cmd.MarkFlagsRequiredTogether(srcFlag, destFlag) return cmd } -func copyFile(src, dest string) error { - srcFile, err := os.Open(src) - if err != nil { - return fmt.Errorf("error opening source file: %w", err) - } - defer srcFile.Close() - - destFile, err := os.Create(filepath.Join(dest, filepath.Base(src))) - if err != nil { - return fmt.Errorf("error creating destination file: %w", err) - } - defer destFile.Close() - - if _, err := io.Copy(destFile, srcFile); err != nil { - return fmt.Errorf("error copying file contents: %w", err) - } - - return nil -} - func parseFlags(flags *pflag.FlagSet) ([]string, []string) { var flagKeys, flagValues []string @@ -634,3 +636,17 @@ func getBuildInfo() (commitHash string, commitTime string, dirtyBuild string) { return } + +func getPodNsName() (types.NamespacedName, error) { + namespace := os.Getenv("POD_NAMESPACE") + if namespace == "" { + return types.NamespacedName{}, errors.New("POD_NAMESPACE environment variable must be set") + } + + podName := os.Getenv("POD_NAME") + if podName == "" { + return types.NamespacedName{}, errors.New("POD_NAME environment variable must be set") + } + + return types.NamespacedName{Namespace: namespace, Name: podName}, nil +} diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 832c44358e..245d8f57c9 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -2,8 +2,6 @@ package main import ( "io" - "os" - "path/filepath" "testing" . "github.com/onsi/gomega" @@ -474,7 +472,7 @@ func TestSleepCmdFlagValidation(t *testing.T) { } } -func TestCopyCmdFlagValidation(t *testing.T) { +func TestInitializeCmdFlagValidation(t *testing.T) { t.Parallel() tests := []flagTestCase{ { @@ -482,6 +480,7 @@ func TestCopyCmdFlagValidation(t *testing.T) { args: []string{ "--source=/my/file", "--destination=dest/file", + "--nginx-plus", }, wantErr: false, }, @@ -513,29 +512,12 @@ func TestCopyCmdFlagValidation(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { t.Parallel() - cmd := createCopyCommand() + cmd := createInitializeCommand() testFlag(t, cmd, test) }) } } -func TestCopyFile(t *testing.T) { - t.Parallel() - g := NewWithT(t) - - src, err := os.CreateTemp(os.TempDir(), "testfile") - g.Expect(err).ToNot(HaveOccurred()) - defer os.Remove(src.Name()) - - dest, err := os.MkdirTemp(os.TempDir(), "testdir") - g.Expect(err).ToNot(HaveOccurred()) - defer os.RemoveAll(dest) - - g.Expect(copyFile(src.Name(), dest)).To(Succeed()) - _, err = os.Stat(filepath.Join(dest, filepath.Base(src.Name()))) - g.Expect(err).ToNot(HaveOccurred()) -} - func TestParseFlags(t *testing.T) { t.Parallel() g := NewWithT(t) diff --git a/cmd/gateway/initialize.go b/cmd/gateway/initialize.go new file mode 100644 index 0000000000..01e56bd328 --- /dev/null +++ b/cmd/gateway/initialize.go @@ -0,0 +1,106 @@ +package main + +import ( + "context" + "fmt" + "path/filepath" + "time" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" + ctlr "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +type copyFiles struct { + destDirName string + srcFileNames []string +} + +type initializeConfig struct { + controllerPodNSName types.NamespacedName + fileManager file.OSFileManager + logger logr.Logger + copy copyFiles + plus bool +} + +func initialize(cfg initializeConfig) error { + for _, src := range cfg.copy.srcFileNames { + if err := copyFile(cfg.fileManager, src, cfg.copy.destDirName); err != nil { + return err + } + } + + if !cfg.plus { + cfg.logger.Info("Finished initializing configuration") + return nil + } + + clusterCfg := ctlr.GetConfigOrDie() + k8sReader, err := client.New(clusterCfg, client.Options{}) + if err != nil { + return fmt.Errorf("unable to initialize k8s client: %w", err) + } + + dcc := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ + K8sClientReader: k8sReader, + PodNSName: cfg.controllerPodNSName, + }) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + depCtx, err := dcc.Collect(ctx, cfg.logger.WithName("deployCtxCollector")) + if err != nil { + return fmt.Errorf("failed to collect deployment context: %w", err) + } + + cfg.logger.Info("Deployment context collected", "deployment context", depCtx) + + if err := writeDeploymentContextFile(cfg.fileManager, depCtx); err != nil { + return fmt.Errorf("failed to write deployment context file: %w", err) + } + + cfg.logger.Info("Finished initializing configuration") + + return nil +} + +func writeDeploymentContextFile(osFileManager file.OSFileManager, depCtx dataplane.DeploymentContext) error { + depCtxFile, err := config.GenerateDeploymentContextFile(depCtx) + if err != nil { + return fmt.Errorf("failed to generate deployment context file: %w", err) + } + + if err := file.WriteFile(osFileManager, depCtxFile); err != nil { + return fmt.Errorf("failed to write deployment context file: %w", err) + } + + return nil +} + +func copyFile(osFileManager file.OSFileManager, src, dest string) error { + srcFile, err := osFileManager.Open(src) + if err != nil { + return fmt.Errorf("error opening source file: %w", err) + } + defer srcFile.Close() + + destFile, err := osFileManager.Create(filepath.Join(dest, filepath.Base(src))) + if err != nil { + return fmt.Errorf("error creating destination file: %w", err) + } + defer destFile.Close() + + if err := osFileManager.Copy(destFile, srcFile); err != nil { + return fmt.Errorf("error copying file contents: %w", err) + } + + return nil +} diff --git a/cmd/gateway/initialize_test.go b/cmd/gateway/initialize_test.go new file mode 100644 index 0000000000..b05f050d9c --- /dev/null +++ b/cmd/gateway/initialize_test.go @@ -0,0 +1,84 @@ +package main + +import ( + "errors" + "io" + "os" + "path/filepath" + "testing" + + . "github.com/onsi/gomega" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file/filefakes" +) + +func TestCopyFile(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + src, err := os.CreateTemp(os.TempDir(), "testfile") + g.Expect(err).ToNot(HaveOccurred()) + defer os.Remove(src.Name()) + + dest, err := os.MkdirTemp(os.TempDir(), "testdir") + g.Expect(err).ToNot(HaveOccurred()) + defer os.RemoveAll(dest) + + g.Expect(copyFile(file.NewStdLibOSFileManager(), src.Name(), dest)).To(Succeed()) + _, err = os.Stat(filepath.Join(dest, filepath.Base(src.Name()))) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestCopyFileErrors(t *testing.T) { + t.Parallel() + + openErr := errors.New("open error") + createErr := errors.New("create error") + copyErr := errors.New("copy error") + + tests := []struct { + fileMgr *filefakes.FakeOSFileManager + expErr error + name string + }{ + { + name: "can't open src file", + fileMgr: &filefakes.FakeOSFileManager{ + OpenStub: func(_ string) (*os.File, error) { + return nil, openErr + }, + }, + expErr: openErr, + }, + { + name: "can't create dest file", + fileMgr: &filefakes.FakeOSFileManager{ + CreateStub: func(_ string) (*os.File, error) { + return nil, createErr + }, + }, + expErr: createErr, + }, + { + name: "can't copy contents", + fileMgr: &filefakes.FakeOSFileManager{ + CopyStub: func(_ io.Writer, _ io.Reader) error { + return copyErr + }, + }, + expErr: copyErr, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + g := NewWithT(t) + err := copyFile(test.fileMgr, "source", "destDir") + + g.Expect(err).To(MatchError(test.expErr)) + }) + } +} diff --git a/cmd/gateway/main.go b/cmd/gateway/main.go index 104bed6673..fc2a5949c7 100644 --- a/cmd/gateway/main.go +++ b/cmd/gateway/main.go @@ -23,7 +23,7 @@ func main() { rootCmd.AddCommand( createStaticModeCommand(), createProvisionerModeCommand(), - createCopyCommand(), + createInitializeCommand(), createSleepCommand(), ) diff --git a/cmd/gateway/validation.go b/cmd/gateway/validation.go index f60c713534..ac6e04fa28 100644 --- a/cmd/gateway/validation.go +++ b/cmd/gateway/validation.go @@ -206,8 +206,8 @@ func ensureNoPortCollisions(ports ...int) error { return nil } -// validateSleepArgs ensures that arguments to the sleep command are set. -func validateSleepArgs(srcFiles []string, dest string) error { +// validateCopyArgs ensures that arguments to the sleep command are set. +func validateCopyArgs(srcFiles []string, dest string) error { if len(srcFiles) == 0 { return errors.New("source must not be empty") } diff --git a/cmd/gateway/validation_test.go b/cmd/gateway/validation_test.go index 28fbfbf3c3..1774f13619 100644 --- a/cmd/gateway/validation_test.go +++ b/cmd/gateway/validation_test.go @@ -588,7 +588,7 @@ func TestValidateSleepArgs(t *testing.T) { t.Parallel() g := NewWithT(t) - err := validateSleepArgs(tc.srcFiles, tc.dest) + err := validateCopyArgs(tc.srcFiles, tc.dest) if !tc.expErr { g.Expect(err).ToNot(HaveOccurred()) } else { diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index 0ae1bcfc0f..63cb16b010 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -22,16 +22,25 @@ spec: app.kubernetes.io/instance: nginx-gateway spec: initContainers: - - name: copy-nginx-config + - name: init image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name securityContext: seccompProfile: type: RuntimeDefault diff --git a/deploy/aws-nlb/deploy.yaml b/deploy/aws-nlb/deploy.yaml index 4f367638b5..91a9aee1da 100644 --- a/deploy/aws-nlb/deploy.yaml +++ b/deploy/aws-nlb/deploy.yaml @@ -306,14 +306,23 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index f4916da3ff..8641af0686 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -303,14 +303,23 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index 88aae1eedd..c5284d27bd 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -303,14 +303,23 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index c2bcbafe09..12f323586b 100644 --- a/deploy/experimental-nginx-plus/deploy.yaml +++ b/deploy/experimental-nginx-plus/deploy.yaml @@ -162,6 +162,7 @@ data: mgmt.conf: | mgmt { enforce_initial_report off; + deployment_context /etc/nginx/main-includes/deployment_ctx.json; } kind: ConfigMap metadata: @@ -328,16 +329,26 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --source - /includes/mgmt.conf + - --nginx-plus - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index be62207472..189a65805b 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -309,14 +309,23 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml index f31b0da07b..87177ca9ea 100644 --- a/deploy/nginx-plus/deploy.yaml +++ b/deploy/nginx-plus/deploy.yaml @@ -157,6 +157,7 @@ data: mgmt.conf: | mgmt { enforce_initial_report off; + deployment_context /etc/nginx/main-includes/deployment_ctx.json; } kind: ConfigMap metadata: @@ -322,16 +323,26 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --source - /includes/mgmt.conf + - --nginx-plus - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index 25b6210ed1..0263036561 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -303,14 +303,23 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml index 8231de661e..7509e76b7a 100644 --- a/deploy/openshift/deploy.yaml +++ b/deploy/openshift/deploy.yaml @@ -311,14 +311,23 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/deploy/snippets-filters-nginx-plus/deploy.yaml b/deploy/snippets-filters-nginx-plus/deploy.yaml index 4a68115cce..f959e99113 100644 --- a/deploy/snippets-filters-nginx-plus/deploy.yaml +++ b/deploy/snippets-filters-nginx-plus/deploy.yaml @@ -159,6 +159,7 @@ data: mgmt.conf: | mgmt { enforce_initial_report off; + deployment_context /etc/nginx/main-includes/deployment_ctx.json; } kind: ConfigMap metadata: @@ -325,16 +326,26 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --source - /includes/mgmt.conf + - --nginx-plus - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/deploy/snippets-filters/deploy.yaml b/deploy/snippets-filters/deploy.yaml index e6fd79ce24..6a0727a206 100644 --- a/deploy/snippets-filters/deploy.yaml +++ b/deploy/snippets-filters/deploy.yaml @@ -306,14 +306,23 @@ spec: initContainers: - command: - /usr/bin/gateway - - copy + - initialize - --source - /includes/main.conf - --destination - /etc/nginx/main-includes + env: + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always - name: copy-nginx-config + name: init securityContext: capabilities: add: diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 637dc378c7..902883ed46 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -19,8 +19,8 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/events" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" frameworkStatus "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" - ngfConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" ngxConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/runtime" @@ -29,7 +29,6 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/status" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/telemetry" ) type handlerMetricsCollector interface { @@ -92,6 +91,11 @@ type objectFilter struct { captureChangeInGraph bool } +// deployCtxCollector collects the deployment context for N+ licensing. +type deployCtxCollector interface { + Collect(ctx context.Context, logger logr.Logger) (dataplane.DeploymentContext, error) +} + // eventHandlerImpl implements EventHandler. // eventHandlerImpl is responsible for: // (1) Reconciling the Gateway API and Kubernetes built-in resources with the NGINX configuration. @@ -107,6 +111,8 @@ type eventHandlerImpl struct { latestReloadResult status.NginxReloadResult + deployCtxCollector deployCtxCollector + cfg eventHandlerConfig lock sync.Mutex @@ -118,6 +124,13 @@ type eventHandlerImpl struct { func newEventHandlerImpl(cfg eventHandlerConfig) *eventHandlerImpl { handler := &eventHandlerImpl{ cfg: cfg, + deployCtxCollector: licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ + K8sClientReader: cfg.k8sReader, + PodNSName: types.NamespacedName{ + Namespace: cfg.gatewayPodConfig.Namespace, + Name: cfg.gatewayPodConfig.Name, + }, + }), } handler.objectFilters = map[filterKey]objectFilter{ @@ -173,9 +186,9 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log case state.EndpointsOnlyChange: h.version++ cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version) - depCtx, setErr := h.setDeploymentCtx(ctx, logger) - if setErr != nil { - logger.Error(setErr, "error setting deployment context for usage reporting") + depCtx, getErr := h.getDeploymentContext(ctx, logger) + if getErr != nil { + logger.Error(getErr, "error getting deployment context for usage reporting") } cfg.DeploymentContext = depCtx @@ -189,9 +202,9 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log case state.ClusterStateChange: h.version++ cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version) - depCtx, setErr := h.setDeploymentCtx(ctx, logger) - if setErr != nil { - logger.Error(setErr, "error setting deployment context for usage reporting") + depCtx, getErr := h.getDeploymentContext(ctx, logger) + if getErr != nil { + logger.Error(getErr, "error getting deployment context for usage reporting") } cfg.DeploymentContext = depCtx @@ -500,8 +513,8 @@ func getGatewayAddresses( return gwAddresses, nil } -// setDeploymentCtx sets the deployment context metadata for nginx plus reporting. -func (h *eventHandlerImpl) setDeploymentCtx( +// getDeploymentContext gets the deployment context metadata for N+ reporting. +func (h *eventHandlerImpl) getDeploymentContext( ctx context.Context, logger logr.Logger, ) (dataplane.DeploymentContext, error) { @@ -509,36 +522,7 @@ func (h *eventHandlerImpl) setDeploymentCtx( return dataplane.DeploymentContext{}, nil } - podNSName := types.NamespacedName{ - Name: h.cfg.gatewayPodConfig.Name, - Namespace: h.cfg.gatewayPodConfig.Namespace, - } - - clusterInfo, err := telemetry.CollectClusterInformation(ctx, h.cfg.k8sReader) - if err != nil { - return dataplane.DeploymentContext{}, fmt.Errorf("error getting cluster information: %w", err) - } - - var installationID string - // InstallationID is not required by the usage API, so if we can't get it, don't return an error - replicaSet, err := telemetry.GetPodReplicaSet(ctx, h.cfg.k8sReader, podNSName) - if err != nil { - logger.Error(err, "failed to get NGF installationID") - } else { - installationID, err = telemetry.GetDeploymentID(replicaSet) - if err != nil { - logger.Error(err, "failed to get NGF installationID") - } - } - - depCtx := dataplane.DeploymentContext{ - Integration: "ngf", - ClusterID: clusterInfo.ClusterID, - ClusterNodeCount: clusterInfo.NodeCount, - InstallationID: installationID, - } - - return depCtx, nil + return h.deployCtxCollector.Collect(ctx, logger.WithName("deployCtxCollector")) } // GetLatestConfiguration gets the latest configuration. diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index 01fc3c0c54..f7f3ffb284 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -8,7 +8,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "go.uber.org/zap" - appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -698,132 +697,26 @@ var _ = Describe("getGatewayAddresses", func() { }) }) -var _ = Describe("setDeploymentCtx", func() { +var _ = Describe("getDeploymentContext", func() { When("nginx plus is false", func() { It("doesn't set the deployment context", func() { handler := eventHandlerImpl{} - depCtx, err := handler.setDeploymentCtx(context.Background(), ctlrZap.New()) + depCtx, err := handler.getDeploymentContext(context.Background(), ctlrZap.New()) Expect(err).ToNot(HaveOccurred()) Expect(depCtx).To(Equal(dataplane.DeploymentContext{})) }) }) When("nginx plus is true", func() { - var ( - clusterID = "test-uid" - ngfPod = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod1", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "replicaset1", - }, - }, - }, - } - - ngfReplicaSet = &appsv1.ReplicaSet{ - Spec: appsv1.ReplicaSetSpec{ - Replicas: helpers.GetPointer[int32](1), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "replicaset1", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "Deployment1", - UID: "test-uid-replicaSet", - }, - }, - }, - } - - kubeNamespace = &v1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: metav1.NamespaceSystem, - UID: "test-uid", - }, - } - - nodeList = &v1.NodeList{ - Items: []v1.Node{{}}, - } - ) - - It("sets the deployment context", func() { - handler := newEventHandlerImpl(eventHandlerConfig{ - plus: true, - k8sReader: fake.NewFakeClient(ngfPod, ngfReplicaSet, kubeNamespace, nodeList), - gatewayPodConfig: config.GatewayPodConfig{ - Name: ngfPod.Name, - }, - }) - - expCtx := dataplane.DeploymentContext{ - Integration: "ngf", - ClusterID: clusterID, - InstallationID: "test-uid-replicaSet", - ClusterNodeCount: 1, - } - - depCtx, err := handler.setDeploymentCtx(context.Background(), ctlrZap.New()) - Expect(err).ToNot(HaveOccurred()) - Expect(depCtx).To(Equal(expCtx)) - }) - - It("returns an error if cluster info isn't found", func() { + It("returns an error on failure ", func() { handler := newEventHandlerImpl(eventHandlerConfig{ plus: true, - k8sReader: fake.NewFakeClient(), + k8sReader: fake.NewFakeClient(), // client with no runtime objects will cause the collector to error }) - _, err := handler.setDeploymentCtx(context.Background(), ctlrZap.New()) + _, err := handler.getDeploymentContext(context.Background(), ctlrZap.New()) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("error getting cluster information")) - }) - - It("sets the deployment context when the replicaset isn't found", func() { - handler := newEventHandlerImpl(eventHandlerConfig{ - plus: true, - k8sReader: fake.NewFakeClient(ngfPod, kubeNamespace, nodeList), - gatewayPodConfig: config.GatewayPodConfig{ - Name: ngfPod.Name, - }, - }) - - expCtx := dataplane.DeploymentContext{ - Integration: "ngf", - ClusterID: clusterID, - ClusterNodeCount: 1, - } - - depCtx, err := handler.setDeploymentCtx(context.Background(), ctlrZap.New()) - Expect(err).ToNot(HaveOccurred()) - Expect(depCtx).To(Equal(expCtx)) - }) - - It("sets the deployment context when the replicaset doesn't have a uid", func() { - ngfReplicaSet.ObjectMeta.OwnerReferences[0].UID = "" - - handler := newEventHandlerImpl(eventHandlerConfig{ - plus: true, - k8sReader: fake.NewFakeClient(ngfPod, ngfReplicaSet, kubeNamespace, nodeList), - gatewayPodConfig: config.GatewayPodConfig{ - Name: ngfPod.Name, - }, - }) - - expCtx := dataplane.DeploymentContext{ - Integration: "ngf", - ClusterID: clusterID, - ClusterNodeCount: 1, - } - - depCtx, err := handler.setDeploymentCtx(context.Background(), ctlrZap.New()) - Expect(err).ToNot(HaveOccurred()) - Expect(depCtx).To(Equal(expCtx)) }) }) }) diff --git a/internal/mode/static/licensing/collector.go b/internal/mode/static/licensing/collector.go new file mode 100644 index 0000000000..d8180ccfd1 --- /dev/null +++ b/internal/mode/static/licensing/collector.go @@ -0,0 +1,70 @@ +package licensing + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/telemetry" +) + +const integrationID = "ngf" + +// DeploymentContextCollectorConfig contains the configuration for the DeploymentContextCollector. +type DeploymentContextCollectorConfig struct { + // K8sClientReader is a Kubernetes API client Reader. + K8sClientReader client.Reader + // PodNSName is the NamespacedName of the NGF Pod. + PodNSName types.NamespacedName +} + +// DeploymentContextCollector collects the deployment context information needed for N+ licensing. +type DeploymentContextCollector struct { + cfg DeploymentContextCollectorConfig +} + +// NewDeploymentContextCollector returns a new instance of DeploymentContextCollector. +func NewDeploymentContextCollector( + cfg DeploymentContextCollectorConfig, +) *DeploymentContextCollector { + return &DeploymentContextCollector{ + cfg: cfg, + } +} + +// Collect collects all the information needed to create the deployment context for N+ licensing. +func (c *DeploymentContextCollector) Collect( + ctx context.Context, + logger logr.Logger, +) (dataplane.DeploymentContext, error) { + clusterInfo, err := telemetry.CollectClusterInformation(ctx, c.cfg.K8sClientReader) + if err != nil { + return dataplane.DeploymentContext{}, fmt.Errorf("error getting cluster information: %w", err) + } + + var installationID string + + // InstallationID is not required by the usage API, so if we can't get it, don't return an error + replicaSet, err := telemetry.GetPodReplicaSet(ctx, c.cfg.K8sClientReader, c.cfg.PodNSName) + if err != nil { + logger.Error(err, "failed to get NGF installationID") + } else { + installationID, err = telemetry.GetDeploymentID(replicaSet) + if err != nil { + logger.Error(err, "failed to get NGF installationID") + } + } + + depCtx := dataplane.DeploymentContext{ + Integration: integrationID, + ClusterID: clusterInfo.ClusterID, + ClusterNodeCount: clusterInfo.NodeCount, + InstallationID: installationID, + } + + return depCtx, nil +} diff --git a/internal/mode/static/licensing/collector_suite_test.go b/internal/mode/static/licensing/collector_suite_test.go new file mode 100644 index 0000000000..bae62214d7 --- /dev/null +++ b/internal/mode/static/licensing/collector_suite_test.go @@ -0,0 +1,14 @@ +package licensing + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestDeploymentContextController(t *testing.T) { + t.Parallel() + RegisterFailHandler(Fail) + RunSpecs(t, "Deployment Context Controller Suite") +} diff --git a/internal/mode/static/licensing/collector_test.go b/internal/mode/static/licensing/collector_test.go new file mode 100644 index 0000000000..fea0449040 --- /dev/null +++ b/internal/mode/static/licensing/collector_test.go @@ -0,0 +1,126 @@ +package licensing_test + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" + + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +var _ = Describe("DeploymentContextCollector", func() { + var ( + clusterID = "test-uid" + ngfPod = &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod1", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "ReplicaSet", + Name: "replicaset1", + }, + }, + }, + } + + ngfReplicaSet = &appsv1.ReplicaSet{ + Spec: appsv1.ReplicaSetSpec{ + Replicas: helpers.GetPointer[int32](1), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "replicaset1", + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Deployment", + Name: "Deployment1", + UID: "test-uid-replicaSet", + }, + }, + }, + } + + kubeNamespace = &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: metav1.NamespaceSystem, + UID: "test-uid", + }, + } + + nodeList = &v1.NodeList{ + Items: []v1.Node{{}}, + } + ) + + It("collects the deployment context", func() { + collector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ + K8sClientReader: fake.NewFakeClient(ngfPod, ngfReplicaSet, kubeNamespace, nodeList), + PodNSName: types.NamespacedName{Name: ngfPod.Name}, + }) + + expCtx := dataplane.DeploymentContext{ + Integration: "ngf", + ClusterID: clusterID, + InstallationID: "test-uid-replicaSet", + ClusterNodeCount: 1, + } + + depCtx, err := collector.Collect(context.Background(), ctlrZap.New()) + Expect(err).ToNot(HaveOccurred()) + Expect(depCtx).To(Equal(expCtx)) + }) + + It("returns an error if cluster info isn't found", func() { + collector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ + K8sClientReader: fake.NewFakeClient(), + }) + + _, err := collector.Collect(context.Background(), ctlrZap.New()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("error getting cluster information")) + }) + + It("returns the deployment context when the replicaset isn't found", func() { + collector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ + K8sClientReader: fake.NewFakeClient(ngfPod, kubeNamespace, nodeList), + PodNSName: types.NamespacedName{Name: ngfPod.Name}, + }) + + expCtx := dataplane.DeploymentContext{ + Integration: "ngf", + ClusterID: clusterID, + ClusterNodeCount: 1, + } + + depCtx, err := collector.Collect(context.Background(), ctlrZap.New()) + Expect(err).ToNot(HaveOccurred()) + Expect(depCtx).To(Equal(expCtx)) + }) + + It("returns the deployment context when the replicaset doesn't have a uid", func() { + ngfReplicaSet.ObjectMeta.OwnerReferences[0].UID = "" + + collector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ + K8sClientReader: fake.NewFakeClient(ngfPod, ngfReplicaSet, kubeNamespace, nodeList), + PodNSName: types.NamespacedName{Name: ngfPod.Name}, + }) + + expCtx := dataplane.DeploymentContext{ + Integration: "ngf", + ClusterID: clusterID, + ClusterNodeCount: 1, + } + + depCtx, err := collector.Collect(context.Background(), ctlrZap.New()) + Expect(err).ToNot(HaveOccurred()) + Expect(depCtx).To(Equal(expCtx)) + }) +}) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 4c80fc4260..5d8f630bbd 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -215,20 +215,11 @@ func StartManager(cfg config.Config) error { groupStatusUpdater := status.NewLeaderAwareGroupUpdater(statusUpdater) eventHandler := newEventHandlerImpl(eventHandlerConfig{ - k8sClient: mgr.GetClient(), - k8sReader: mgr.GetAPIReader(), - processor: processor, - serviceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()), - generator: ngxcfg.NewGeneratorImpl( - cfg.Plus, - &cfg.UsageReportConfig, - cfg.Logger.WithName("generator"), - ), - logLevelSetter: logLevelSetter, nginxFileMgr: file.NewManagerImpl( cfg.Logger.WithName("nginxFileManager"), file.NewStdLibOSFileManager(), ), + metricsCollector: handlerCollector, nginxRuntimeMgr: ngxruntime.NewManagerImpl( ngxPlusClient, ngxruntimeCollector, @@ -236,12 +227,21 @@ func StartManager(cfg config.Config) error { processHandler, ngxruntime.NewVerifyClient(ngxruntime.NginxReloadTimeout), ), - statusUpdater: groupStatusUpdater, + statusUpdater: groupStatusUpdater, + processor: processor, + serviceResolver: resolver.NewServiceResolverImpl(mgr.GetClient()), + generator: ngxcfg.NewGeneratorImpl( + cfg.Plus, + &cfg.UsageReportConfig, + cfg.Logger.WithName("generator"), + ), + k8sClient: mgr.GetClient(), + k8sReader: mgr.GetAPIReader(), + logLevelSetter: logLevelSetter, eventRecorder: recorder, nginxConfiguredOnStartChecker: nginxChecker, - controlConfigNSName: controlConfigNSName, gatewayPodConfig: cfg.GatewayPodConfig, - metricsCollector: handlerCollector, + controlConfigNSName: controlConfigNSName, gatewayCtlrName: cfg.GatewayCtlrName, updateGatewayClassStatus: cfg.UpdateGatewayClassStatus, plus: cfg.Plus, diff --git a/internal/mode/static/nginx/config/main_config.go b/internal/mode/static/nginx/config/main_config.go index bec292a86f..cea4262dfa 100644 --- a/internal/mode/static/nginx/config/main_config.go +++ b/internal/mode/static/nginx/config/main_config.go @@ -2,6 +2,7 @@ package config import ( "encoding/json" + "fmt" gotemplate "text/template" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" @@ -21,6 +22,23 @@ type mainConfig struct { Conf dataplane.Configuration } +// GenerateDeploymentContextFile generates the deployment_ctx.json file needed for N+ licensing. +// It's exported since it's used by the init container process. +func GenerateDeploymentContextFile(depCtx dataplane.DeploymentContext) (file.File, error) { + depCtxBytes, err := json.Marshal(depCtx) + if err != nil { + return file.File{}, fmt.Errorf("error building deployment context for mgmt block: %w", err) + } + + deploymentCtxFile := file.File{ + Content: depCtxBytes, + Path: mainIncludesFolder + "/deployment_ctx.json", + Type: file.TypeRegular, + } + + return deploymentCtxFile, nil +} + func executeMainConfig(conf dataplane.Configuration) []executeResult { includes := createIncludesFromSnippets(conf.MainSnippets) @@ -43,7 +61,6 @@ type mgmtConf struct { Endpoint string Resolver string LicenseTokenFile string - DeploymentCtxFile string CACertFile string ClientSSLCertFile string ClientSSLKeyFile string @@ -106,17 +123,10 @@ func (g GeneratorImpl) generateMgmtFiles(conf dataplane.Configuration) []file.Fi files = append(files, keyFile) } - depCtx, err := json.Marshal(conf.DeploymentContext) + deploymentCtxFile, err := GenerateDeploymentContextFile(conf.DeploymentContext) if err != nil { g.logger.Error(err, "error building deployment context for mgmt block") } else { - deploymentCtxFile := file.File{ - Content: depCtx, - Path: mainIncludesFolder + "/deployment_ctx.json", - Type: file.TypeRegular, - } - - cfg.DeploymentCtxFile = deploymentCtxFile.Path files = append(files, deploymentCtxFile) } diff --git a/internal/mode/static/nginx/config/main_config_template.go b/internal/mode/static/nginx/config/main_config_template.go index 895494f7b2..2811a4f77d 100644 --- a/internal/mode/static/nginx/config/main_config_template.go +++ b/internal/mode/static/nginx/config/main_config_template.go @@ -21,9 +21,7 @@ mgmt { resolver {{ .Resolver }}; {{- end }} license_token {{ .LicenseTokenFile }}; - {{- if .DeploymentCtxFile }} - deployment_context {{ .DeploymentCtxFile }}; - {{- end }} + deployment_context /etc/nginx/main-includes/deployment_ctx.json; {{- if .SkipVerify }} ssl_verify off; {{- end }} diff --git a/internal/mode/static/nginx/file/filefakes/fake_osfile_manager.go b/internal/mode/static/nginx/file/filefakes/fake_osfile_manager.go index 7227cf8d4c..733da9a1c4 100644 --- a/internal/mode/static/nginx/file/filefakes/fake_osfile_manager.go +++ b/internal/mode/static/nginx/file/filefakes/fake_osfile_manager.go @@ -2,6 +2,7 @@ package filefakes import ( + "io" "io/fs" "os" "sync" @@ -22,6 +23,18 @@ type FakeOSFileManager struct { chmodReturnsOnCall map[int]struct { result1 error } + CopyStub func(io.Writer, io.Reader) error + copyMutex sync.RWMutex + copyArgsForCall []struct { + arg1 io.Writer + arg2 io.Reader + } + copyReturns struct { + result1 error + } + copyReturnsOnCall map[int]struct { + result1 error + } CreateStub func(string) (*os.File, error) createMutex sync.RWMutex createArgsForCall []struct { @@ -35,6 +48,19 @@ type FakeOSFileManager struct { result1 *os.File result2 error } + OpenStub func(string) (*os.File, error) + openMutex sync.RWMutex + openArgsForCall []struct { + arg1 string + } + openReturns struct { + result1 *os.File + result2 error + } + openReturnsOnCall map[int]struct { + result1 *os.File + result2 error + } ReadDirStub func(string) ([]fs.DirEntry, error) readDirMutex sync.RWMutex readDirArgsForCall []struct { @@ -137,6 +163,68 @@ func (fake *FakeOSFileManager) ChmodReturnsOnCall(i int, result1 error) { }{result1} } +func (fake *FakeOSFileManager) Copy(arg1 io.Writer, arg2 io.Reader) error { + fake.copyMutex.Lock() + ret, specificReturn := fake.copyReturnsOnCall[len(fake.copyArgsForCall)] + fake.copyArgsForCall = append(fake.copyArgsForCall, struct { + arg1 io.Writer + arg2 io.Reader + }{arg1, arg2}) + stub := fake.CopyStub + fakeReturns := fake.copyReturns + fake.recordInvocation("Copy", []interface{}{arg1, arg2}) + fake.copyMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeOSFileManager) CopyCallCount() int { + fake.copyMutex.RLock() + defer fake.copyMutex.RUnlock() + return len(fake.copyArgsForCall) +} + +func (fake *FakeOSFileManager) CopyCalls(stub func(io.Writer, io.Reader) error) { + fake.copyMutex.Lock() + defer fake.copyMutex.Unlock() + fake.CopyStub = stub +} + +func (fake *FakeOSFileManager) CopyArgsForCall(i int) (io.Writer, io.Reader) { + fake.copyMutex.RLock() + defer fake.copyMutex.RUnlock() + argsForCall := fake.copyArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeOSFileManager) CopyReturns(result1 error) { + fake.copyMutex.Lock() + defer fake.copyMutex.Unlock() + fake.CopyStub = nil + fake.copyReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeOSFileManager) CopyReturnsOnCall(i int, result1 error) { + fake.copyMutex.Lock() + defer fake.copyMutex.Unlock() + fake.CopyStub = nil + if fake.copyReturnsOnCall == nil { + fake.copyReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.copyReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeOSFileManager) Create(arg1 string) (*os.File, error) { fake.createMutex.Lock() ret, specificReturn := fake.createReturnsOnCall[len(fake.createArgsForCall)] @@ -201,6 +289,70 @@ func (fake *FakeOSFileManager) CreateReturnsOnCall(i int, result1 *os.File, resu }{result1, result2} } +func (fake *FakeOSFileManager) Open(arg1 string) (*os.File, error) { + fake.openMutex.Lock() + ret, specificReturn := fake.openReturnsOnCall[len(fake.openArgsForCall)] + fake.openArgsForCall = append(fake.openArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.OpenStub + fakeReturns := fake.openReturns + fake.recordInvocation("Open", []interface{}{arg1}) + fake.openMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeOSFileManager) OpenCallCount() int { + fake.openMutex.RLock() + defer fake.openMutex.RUnlock() + return len(fake.openArgsForCall) +} + +func (fake *FakeOSFileManager) OpenCalls(stub func(string) (*os.File, error)) { + fake.openMutex.Lock() + defer fake.openMutex.Unlock() + fake.OpenStub = stub +} + +func (fake *FakeOSFileManager) OpenArgsForCall(i int) string { + fake.openMutex.RLock() + defer fake.openMutex.RUnlock() + argsForCall := fake.openArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeOSFileManager) OpenReturns(result1 *os.File, result2 error) { + fake.openMutex.Lock() + defer fake.openMutex.Unlock() + fake.OpenStub = nil + fake.openReturns = struct { + result1 *os.File + result2 error + }{result1, result2} +} + +func (fake *FakeOSFileManager) OpenReturnsOnCall(i int, result1 *os.File, result2 error) { + fake.openMutex.Lock() + defer fake.openMutex.Unlock() + fake.OpenStub = nil + if fake.openReturnsOnCall == nil { + fake.openReturnsOnCall = make(map[int]struct { + result1 *os.File + result2 error + }) + } + fake.openReturnsOnCall[i] = struct { + result1 *os.File + result2 error + }{result1, result2} +} + func (fake *FakeOSFileManager) ReadDir(arg1 string) ([]fs.DirEntry, error) { fake.readDirMutex.Lock() ret, specificReturn := fake.readDirReturnsOnCall[len(fake.readDirArgsForCall)] @@ -398,8 +550,12 @@ func (fake *FakeOSFileManager) Invocations() map[string][][]interface{} { defer fake.invocationsMutex.RUnlock() fake.chmodMutex.RLock() defer fake.chmodMutex.RUnlock() + fake.copyMutex.RLock() + defer fake.copyMutex.RUnlock() fake.createMutex.RLock() defer fake.createMutex.RUnlock() + fake.openMutex.RLock() + defer fake.openMutex.RUnlock() fake.readDirMutex.RLock() defer fake.readDirMutex.RUnlock() fake.removeMutex.RLock() diff --git a/internal/mode/static/nginx/file/folders.go b/internal/mode/static/nginx/file/folders.go index a7b296e1d1..847ca6312a 100644 --- a/internal/mode/static/nginx/file/folders.go +++ b/internal/mode/static/nginx/file/folders.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "slices" ) //counterfeiter:generate io/fs.DirEntry @@ -21,10 +22,13 @@ type ClearFoldersOSFileManager interface { // These files are needed on startup, so skip deleting them. const ( - mainConf = "/etc/nginx/main-includes/main.conf" - mgmtConf = "/etc/nginx/main-includes/mgmt.conf" + mainConf = "/etc/nginx/main-includes/main.conf" + mgmtConf = "/etc/nginx/main-includes/mgmt.conf" + deployCtx = "/etc/nginx/main-includes/deployment_ctx.json" ) +var ignoreFilePaths = []string{mainConf, mgmtConf, deployCtx} + // ClearFolders removes all files in the given folders and returns the removed files' full paths. func ClearFolders(fileMgr ClearFoldersOSFileManager, paths []string) (removedFiles []string, e error) { for _, path := range paths { @@ -36,7 +40,7 @@ func ClearFolders(fileMgr ClearFoldersOSFileManager, paths []string) (removedFil for _, entry := range entries { entryPath := filepath.Join(path, entry.Name()) - if entryPath == mainConf || entryPath == mgmtConf { + if slices.Contains(ignoreFilePaths, entryPath) { continue } diff --git a/internal/mode/static/nginx/file/folders_test.go b/internal/mode/static/nginx/file/folders_test.go index 8a63b12106..beefd76dd1 100644 --- a/internal/mode/static/nginx/file/folders_test.go +++ b/internal/mode/static/nginx/file/folders_test.go @@ -41,6 +41,43 @@ func TestClearFoldersRemoves(t *testing.T) { g.Expect(entries).To(BeEmpty()) } +func TestClearFoldersIgnoresPaths(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + fakeFileMgr := &filefakes.FakeClearFoldersOSFileManager{ + ReadDirStub: func(_ string) ([]os.DirEntry, error) { + return []os.DirEntry{ + &filefakes.FakeDirEntry{ + NameStub: func() string { + return "deployment_ctx.json" + }, + }, + &filefakes.FakeDirEntry{ + NameStub: func() string { + return "mgmt.conf" + }, + }, + &filefakes.FakeDirEntry{ + NameStub: func() string { + return "main.conf" + }, + }, + &filefakes.FakeDirEntry{ + NameStub: func() string { + return "can-be-removed.conf" + }, + }, + }, nil + }, + } + + removed, err := file.ClearFolders(fakeFileMgr, []string{"/etc/nginx/main-includes"}) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(removed).To(HaveLen(1)) + g.Expect(removed[0]).To(Equal("/etc/nginx/main-includes/can-be-removed.conf")) +} + func TestClearFoldersFails(t *testing.T) { t.Parallel() files := []string{"file"} diff --git a/internal/mode/static/nginx/file/manager.go b/internal/mode/static/nginx/file/manager.go index b2726c5b54..6c535ddfc4 100644 --- a/internal/mode/static/nginx/file/manager.go +++ b/internal/mode/static/nginx/file/manager.go @@ -3,6 +3,7 @@ package file import ( "errors" "fmt" + "io" "io/fs" "os" @@ -61,6 +62,10 @@ type OSFileManager interface { Chmod(file *os.File, mode os.FileMode) error // Write writes contents to the file. Write(file *os.File, contents []byte) error + // Open opens the file. + Open(name string) (*os.File, error) + // Copy copies from src to dst. + Copy(dst io.Writer, src io.Reader) error } //counterfeiter:generate . Manager @@ -113,7 +118,7 @@ func (m *ManagerImpl) ReplaceFiles(files []File) error { m.lastWrittenPaths = make([]string, 0, len(files)) for _, file := range files { - if err := writeFile(m.osFileManager, file); err != nil { + if err := WriteFile(m.osFileManager, file); err != nil { return fmt.Errorf("failed to write file %q of type %v: %w", file.Path, file.Type, err) } @@ -124,7 +129,7 @@ func (m *ManagerImpl) ReplaceFiles(files []File) error { return nil } -func writeFile(fileMgr OSFileManager, file File) error { +func WriteFile(fileMgr OSFileManager, file File) error { ensureType(file.Type) f, err := fileMgr.Create(file.Path) diff --git a/internal/mode/static/nginx/file/os_filemanager.go b/internal/mode/static/nginx/file/os_filemanager.go index 05547cb046..4b89768b80 100644 --- a/internal/mode/static/nginx/file/os_filemanager.go +++ b/internal/mode/static/nginx/file/os_filemanager.go @@ -1,6 +1,7 @@ package file import ( + "io" "io/fs" "os" ) @@ -41,3 +42,12 @@ func (s *StdLibOSFileManager) Create(name string) (*os.File, error) { func (s *StdLibOSFileManager) Chmod(file *os.File, mode os.FileMode) error { return file.Chmod(mode) } + +// Open wraps os.Open. +func (s *StdLibOSFileManager) Open(name string) (*os.File, error) { return os.Open(name) } + +// Copy wraps io.Copy. +func (s *StdLibOSFileManager) Copy(dst io.Writer, src io.Reader) error { + _, err := io.Copy(dst, src) + return err +} From a235361a334c87f8bd26eaf6f496da3a9d6b492e Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Thu, 5 Dec 2024 17:10:02 -0700 Subject: [PATCH 02/11] Fix lint --- cmd/gateway/commands.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 96d08f0b40..d287ef74fc 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -23,7 +23,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" ) -// These flags are shared by mutliple commands. +// These flags are shared by multiple commands. const ( domain = "gateway.nginx.org" gatewayClassFlag = "gatewayclass" From 4c468a45e1a986816eb07195a046026a5219a0d1 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Thu, 5 Dec 2024 17:55:57 -0700 Subject: [PATCH 03/11] more tests --- cmd/gateway/commands.go | 22 +++- cmd/gateway/commands_test.go | 23 ++++ cmd/gateway/initialize.go | 26 +--- cmd/gateway/initialize_test.go | 114 +++++++++++++++++ internal/mode/static/licensing/collector.go | 9 ++ .../licensingfakes/fake_collector.go | 121 ++++++++++++++++++ 6 files changed, 291 insertions(+), 24 deletions(-) create mode 100644 internal/mode/static/licensing/licensingfakes/fake_collector.go diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index d287ef74fc..0626882ef6 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -14,12 +14,15 @@ import ( "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/klog/v2" + ctlr "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/provisioner" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" ) @@ -541,6 +544,17 @@ func createInitializeCommand() *cobra.Command { return fmt.Errorf("could not get pod nsname: %w", err) } + clusterCfg := ctlr.GetConfigOrDie() + k8sReader, err := client.New(clusterCfg, client.Options{}) + if err != nil { + return fmt.Errorf("unable to initialize k8s client: %w", err) + } + + dcc := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ + K8sClientReader: k8sReader, + PodNSName: podNsName, + }) + logger := ctlrZap.New() klog.SetLogger(logger) logger.Info( @@ -553,10 +567,10 @@ func createInitializeCommand() *cobra.Command { log.SetLogger(logger) return initialize(initializeConfig{ - controllerPodNSName: podNsName, - fileManager: file.NewStdLibOSFileManager(), - logger: logger, - plus: plus, + fileManager: file.NewStdLibOSFileManager(), + logger: logger, + plus: plus, + collector: dcc, copy: copyFiles{ srcFileNames: srcFiles, destDirName: dest, diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 245d8f57c9..55edcc185d 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -2,6 +2,7 @@ package main import ( "io" + "os" "testing" . "github.com/onsi/gomega" @@ -658,3 +659,25 @@ func TestGetBuildInfo(t *testing.T) { g.Expect(commitTime).To(Not(Equal("unknown"))) g.Expect(dirtyBuild).To(Not(Equal("unknown"))) } + +func TestGetPodNsName(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + g.Expect(os.Setenv("POD_NAMESPACE", "default")).To(Succeed()) + g.Expect(os.Setenv("POD_NAME", "my-pod")).To(Succeed()) + + nsname, err := getPodNsName() + g.Expect(err).To(Not(HaveOccurred())) + g.Expect(nsname).To(Equal(types.NamespacedName{Name: "my-pod", Namespace: "default"})) + + g.Expect(os.Unsetenv("POD_NAMESPACE")).To(Succeed()) + nsname, err = getPodNsName() + g.Expect(err).To(HaveOccurred()) + g.Expect(nsname).To(Equal(types.NamespacedName{})) + + g.Expect(os.Unsetenv("POD_NAME")).To(Succeed()) + nsname, err = getPodNsName() + g.Expect(err).To(HaveOccurred()) + g.Expect(nsname).To(Equal(types.NamespacedName{})) +} diff --git a/cmd/gateway/initialize.go b/cmd/gateway/initialize.go index 01e56bd328..812d87e463 100644 --- a/cmd/gateway/initialize.go +++ b/cmd/gateway/initialize.go @@ -7,9 +7,6 @@ import ( "time" "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/types" - ctlr "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" @@ -23,11 +20,11 @@ type copyFiles struct { } type initializeConfig struct { - controllerPodNSName types.NamespacedName - fileManager file.OSFileManager - logger logr.Logger - copy copyFiles - plus bool + collector licensing.Collector + fileManager file.OSFileManager + logger logr.Logger + copy copyFiles + plus bool } func initialize(cfg initializeConfig) error { @@ -42,21 +39,10 @@ func initialize(cfg initializeConfig) error { return nil } - clusterCfg := ctlr.GetConfigOrDie() - k8sReader, err := client.New(clusterCfg, client.Options{}) - if err != nil { - return fmt.Errorf("unable to initialize k8s client: %w", err) - } - - dcc := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ - K8sClientReader: k8sReader, - PodNSName: cfg.controllerPodNSName, - }) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - depCtx, err := dcc.Collect(ctx, cfg.logger.WithName("deployCtxCollector")) + depCtx, err := cfg.collector.Collect(ctx, cfg.logger.WithName("deployCtxCollector")) if err != nil { return fmt.Errorf("failed to collect deployment context: %w", err) } diff --git a/cmd/gateway/initialize_test.go b/cmd/gateway/initialize_test.go index b05f050d9c..34d811682a 100644 --- a/cmd/gateway/initialize_test.go +++ b/cmd/gateway/initialize_test.go @@ -1,18 +1,132 @@ package main import ( + "context" "errors" "io" "os" "path/filepath" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing/licensingfakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file/filefakes" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) +func TestInitialize_OSS(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + + fakeFileMgr := &filefakes.FakeOSFileManager{} + + ic := initializeConfig{ + fileManager: fakeFileMgr, + logger: zap.New(), + copy: copyFiles{ + destDirName: "destDir", + srcFileNames: []string{"src1", "src2"}, + }, + plus: false, + } + + err := initialize(ic) + g.Expect(err).To(BeNil()) + g.Expect(fakeFileMgr.CreateCallCount()).To(Equal(2)) + g.Expect(fakeFileMgr.OpenCallCount()).To(Equal(2)) + g.Expect(fakeFileMgr.CopyCallCount()).To(Equal(2)) +} + +func TestInitialize_OSS_Error(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + + openErr := errors.New("open error") + fakeFileMgr := &filefakes.FakeOSFileManager{ + OpenStub: func(_ string) (*os.File, error) { + return nil, openErr + }, + } + + ic := initializeConfig{ + fileManager: fakeFileMgr, + logger: zap.New(), + copy: copyFiles{ + destDirName: "destDir", + srcFileNames: []string{"src1", "src2"}, + }, + plus: false, + } + + err := initialize(ic) + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(MatchError(openErr)) +} + +func TestInitialize_Plus(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + + fakeFileMgr := &filefakes.FakeOSFileManager{} + fakeCollector := &licensingfakes.FakeCollector{} + + ic := initializeConfig{ + fileManager: fakeFileMgr, + logger: zap.New(), + collector: fakeCollector, + copy: copyFiles{ + destDirName: "destDir", + srcFileNames: []string{"src1", "src2"}, + }, + plus: true, + } + + err := initialize(ic) + g.Expect(err).ToNot(HaveOccurred()) + // copies + g.Expect(fakeFileMgr.OpenCallCount()).To(Equal(2)) + g.Expect(fakeFileMgr.CopyCallCount()).To(Equal(2)) + + // 2 copies, 1 write deploy ctx + g.Expect(fakeFileMgr.CreateCallCount()).To(Equal(3)) + // write deploy ctx + g.Expect(fakeCollector.CollectCallCount()).To(Equal(1)) + g.Expect(fakeFileMgr.WriteCallCount()).To(Equal(1)) + g.Expect(fakeFileMgr.ChmodCallCount()).To(Equal(1)) +} + +func TestInitialize_Plus_Error(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + + collectErr := errors.New("collect error") + fakeFileMgr := &filefakes.FakeOSFileManager{} + fakeCollector := &licensingfakes.FakeCollector{ + CollectStub: func(_ context.Context, _ logr.Logger) (dataplane.DeploymentContext, error) { + return dataplane.DeploymentContext{}, collectErr + }, + } + + ic := initializeConfig{ + fileManager: fakeFileMgr, + logger: zap.New(), + collector: fakeCollector, + copy: copyFiles{ + destDirName: "destDir", + srcFileNames: []string{"src1", "src2"}, + }, + plus: true, + } + + err := initialize(ic) + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(MatchError(collectErr)) +} + func TestCopyFile(t *testing.T) { t.Parallel() g := NewWithT(t) diff --git a/internal/mode/static/licensing/collector.go b/internal/mode/static/licensing/collector.go index d8180ccfd1..d3180ffc25 100644 --- a/internal/mode/static/licensing/collector.go +++ b/internal/mode/static/licensing/collector.go @@ -12,6 +12,15 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/telemetry" ) +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate + +//counterfeiter:generate . Collector + +// Collector collects licensing information for N+. +type Collector interface { + Collect(ctx context.Context, log logr.Logger) (dataplane.DeploymentContext, error) +} + const integrationID = "ngf" // DeploymentContextCollectorConfig contains the configuration for the DeploymentContextCollector. diff --git a/internal/mode/static/licensing/licensingfakes/fake_collector.go b/internal/mode/static/licensing/licensingfakes/fake_collector.go new file mode 100644 index 0000000000..f5f4efc8a2 --- /dev/null +++ b/internal/mode/static/licensing/licensingfakes/fake_collector.go @@ -0,0 +1,121 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package licensingfakes + +import ( + "context" + "sync" + + "github.com/go-logr/logr" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +type FakeCollector struct { + CollectStub func(context.Context, logr.Logger) (dataplane.DeploymentContext, error) + collectMutex sync.RWMutex + collectArgsForCall []struct { + arg1 context.Context + arg2 logr.Logger + } + collectReturns struct { + result1 dataplane.DeploymentContext + result2 error + } + collectReturnsOnCall map[int]struct { + result1 dataplane.DeploymentContext + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeCollector) Collect(arg1 context.Context, arg2 logr.Logger) (dataplane.DeploymentContext, error) { + fake.collectMutex.Lock() + ret, specificReturn := fake.collectReturnsOnCall[len(fake.collectArgsForCall)] + fake.collectArgsForCall = append(fake.collectArgsForCall, struct { + arg1 context.Context + arg2 logr.Logger + }{arg1, arg2}) + stub := fake.CollectStub + fakeReturns := fake.collectReturns + fake.recordInvocation("Collect", []interface{}{arg1, arg2}) + fake.collectMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeCollector) CollectCallCount() int { + fake.collectMutex.RLock() + defer fake.collectMutex.RUnlock() + return len(fake.collectArgsForCall) +} + +func (fake *FakeCollector) CollectCalls(stub func(context.Context, logr.Logger) (dataplane.DeploymentContext, error)) { + fake.collectMutex.Lock() + defer fake.collectMutex.Unlock() + fake.CollectStub = stub +} + +func (fake *FakeCollector) CollectArgsForCall(i int) (context.Context, logr.Logger) { + fake.collectMutex.RLock() + defer fake.collectMutex.RUnlock() + argsForCall := fake.collectArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeCollector) CollectReturns(result1 dataplane.DeploymentContext, result2 error) { + fake.collectMutex.Lock() + defer fake.collectMutex.Unlock() + fake.CollectStub = nil + fake.collectReturns = struct { + result1 dataplane.DeploymentContext + result2 error + }{result1, result2} +} + +func (fake *FakeCollector) CollectReturnsOnCall(i int, result1 dataplane.DeploymentContext, result2 error) { + fake.collectMutex.Lock() + defer fake.collectMutex.Unlock() + fake.CollectStub = nil + if fake.collectReturnsOnCall == nil { + fake.collectReturnsOnCall = make(map[int]struct { + result1 dataplane.DeploymentContext + result2 error + }) + } + fake.collectReturnsOnCall[i] = struct { + result1 dataplane.DeploymentContext + result2 error + }{result1, result2} +} + +func (fake *FakeCollector) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.collectMutex.RLock() + defer fake.collectMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeCollector) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ licensing.Collector = new(FakeCollector) From cfc5b4fddb7fc52b1334fa5b94322650495e21e4 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 6 Dec 2024 09:04:25 -0700 Subject: [PATCH 04/11] Even more tests --- cmd/gateway/commands.go | 11 ++--- cmd/gateway/initialize.go | 2 +- cmd/gateway/initialize_test.go | 5 +-- internal/mode/static/handler.go | 27 +++--------- internal/mode/static/handler_test.go | 42 ++++++++++++++++--- internal/mode/static/licensing/collector.go | 13 +++--- .../mode/static/licensing/collector_test.go | 9 ++-- .../licensingfakes/fake_collector.go | 19 ++++----- internal/mode/static/manager.go | 10 +++++ 9 files changed, 79 insertions(+), 59 deletions(-) diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 0626882ef6..824ecf5a1a 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -550,11 +550,6 @@ func createInitializeCommand() *cobra.Command { return fmt.Errorf("unable to initialize k8s client: %w", err) } - dcc := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ - K8sClientReader: k8sReader, - PodNSName: podNsName, - }) - logger := ctlrZap.New() klog.SetLogger(logger) logger.Info( @@ -566,6 +561,12 @@ func createInitializeCommand() *cobra.Command { ) log.SetLogger(logger) + dcc := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ + K8sClientReader: k8sReader, + PodNSName: podNsName, + Logger: logger.WithName("deployCtxCollector"), + }) + return initialize(initializeConfig{ fileManager: file.NewStdLibOSFileManager(), logger: logger, diff --git a/cmd/gateway/initialize.go b/cmd/gateway/initialize.go index 812d87e463..45c2999ae9 100644 --- a/cmd/gateway/initialize.go +++ b/cmd/gateway/initialize.go @@ -42,7 +42,7 @@ func initialize(cfg initializeConfig) error { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - depCtx, err := cfg.collector.Collect(ctx, cfg.logger.WithName("deployCtxCollector")) + depCtx, err := cfg.collector.Collect(ctx) if err != nil { return fmt.Errorf("failed to collect deployment context: %w", err) } diff --git a/cmd/gateway/initialize_test.go b/cmd/gateway/initialize_test.go index 34d811682a..c4c282e940 100644 --- a/cmd/gateway/initialize_test.go +++ b/cmd/gateway/initialize_test.go @@ -8,7 +8,6 @@ import ( "path/filepath" "testing" - "github.com/go-logr/logr" . "github.com/onsi/gomega" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -35,7 +34,7 @@ func TestInitialize_OSS(t *testing.T) { } err := initialize(ic) - g.Expect(err).To(BeNil()) + g.Expect(err).ToNot(HaveOccurred()) g.Expect(fakeFileMgr.CreateCallCount()).To(Equal(2)) g.Expect(fakeFileMgr.OpenCallCount()).To(Equal(2)) g.Expect(fakeFileMgr.CopyCallCount()).To(Equal(2)) @@ -106,7 +105,7 @@ func TestInitialize_Plus_Error(t *testing.T) { collectErr := errors.New("collect error") fakeFileMgr := &filefakes.FakeOSFileManager{} fakeCollector := &licensingfakes.FakeCollector{ - CollectStub: func(_ context.Context, _ logr.Logger) (dataplane.DeploymentContext, error) { + CollectStub: func(_ context.Context) (dataplane.DeploymentContext, error) { return dataplane.DeploymentContext{}, collectErr }, } diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 902883ed46..e09732e7f0 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -59,6 +59,8 @@ type eventHandlerConfig struct { logLevelSetter logLevelSetter // eventRecorder records events for Kubernetes resources. eventRecorder record.EventRecorder + // deployCtxCollector collects the deployment context for N+ licensing + deployCtxCollector licensing.Collector // nginxConfiguredOnStartChecker sets the health of the Pod to Ready once we've written out our initial config. nginxConfiguredOnStartChecker *nginxConfiguredOnStartChecker // gatewayPodConfig contains information about this Pod. @@ -91,11 +93,6 @@ type objectFilter struct { captureChangeInGraph bool } -// deployCtxCollector collects the deployment context for N+ licensing. -type deployCtxCollector interface { - Collect(ctx context.Context, logger logr.Logger) (dataplane.DeploymentContext, error) -} - // eventHandlerImpl implements EventHandler. // eventHandlerImpl is responsible for: // (1) Reconciling the Gateway API and Kubernetes built-in resources with the NGINX configuration. @@ -111,8 +108,6 @@ type eventHandlerImpl struct { latestReloadResult status.NginxReloadResult - deployCtxCollector deployCtxCollector - cfg eventHandlerConfig lock sync.Mutex @@ -124,13 +119,6 @@ type eventHandlerImpl struct { func newEventHandlerImpl(cfg eventHandlerConfig) *eventHandlerImpl { handler := &eventHandlerImpl{ cfg: cfg, - deployCtxCollector: licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ - K8sClientReader: cfg.k8sReader, - PodNSName: types.NamespacedName{ - Namespace: cfg.gatewayPodConfig.Namespace, - Name: cfg.gatewayPodConfig.Name, - }, - }), } handler.objectFilters = map[filterKey]objectFilter{ @@ -186,7 +174,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log case state.EndpointsOnlyChange: h.version++ cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version) - depCtx, getErr := h.getDeploymentContext(ctx, logger) + depCtx, getErr := h.getDeploymentContext(ctx) if getErr != nil { logger.Error(getErr, "error getting deployment context for usage reporting") } @@ -202,7 +190,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log case state.ClusterStateChange: h.version++ cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version) - depCtx, getErr := h.getDeploymentContext(ctx, logger) + depCtx, getErr := h.getDeploymentContext(ctx) if getErr != nil { logger.Error(getErr, "error getting deployment context for usage reporting") } @@ -514,15 +502,12 @@ func getGatewayAddresses( } // getDeploymentContext gets the deployment context metadata for N+ reporting. -func (h *eventHandlerImpl) getDeploymentContext( - ctx context.Context, - logger logr.Logger, -) (dataplane.DeploymentContext, error) { +func (h *eventHandlerImpl) getDeploymentContext(ctx context.Context) (dataplane.DeploymentContext, error) { if !h.cfg.plus { return dataplane.DeploymentContext{}, nil } - return h.deployCtxCollector.Collect(ctx, logger.WithName("deployCtxCollector")) + return h.cfg.deployCtxCollector.Collect(ctx) } // GetLatestConfiguration gets the latest configuration. diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index f7f3ffb284..67bf0e8e0e 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -23,6 +23,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status/statusfakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing/licensingfakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/metrics/collectors" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/configfakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" @@ -105,6 +106,7 @@ var _ = Describe("eventHandler", func() { nginxRuntimeMgr: fakeNginxRuntimeMgr, statusUpdater: fakeStatusUpdater, eventRecorder: fakeEventRecorder, + deployCtxCollector: &licensingfakes.FakeCollector{}, nginxConfiguredOnStartChecker: newNginxConfiguredOnStartChecker(), controlConfigNSName: types.NamespacedName{Namespace: namespace, Name: configName}, gatewayPodConfig: config.GatewayPodConfig{ @@ -702,21 +704,49 @@ var _ = Describe("getDeploymentContext", func() { It("doesn't set the deployment context", func() { handler := eventHandlerImpl{} - depCtx, err := handler.getDeploymentContext(context.Background(), ctlrZap.New()) + depCtx, err := handler.getDeploymentContext(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(depCtx).To(Equal(dataplane.DeploymentContext{})) }) }) When("nginx plus is true", func() { - It("returns an error on failure ", func() { + It("returns deployment context", func() { + expDepCtx := dataplane.DeploymentContext{ + Integration: "ngf", + ClusterID: "cluster-id", + InstallationID: "installation-id", + ClusterNodeCount: 1, + } + + handler := newEventHandlerImpl(eventHandlerConfig{ + plus: true, + deployCtxCollector: &licensingfakes.FakeCollector{ + CollectStub: func(_ context.Context) (dataplane.DeploymentContext, error) { + return expDepCtx, nil + }, + }, + }) + + dc, err := handler.getDeploymentContext(context.Background()) + Expect(err).ToNot(HaveOccurred()) + Expect(dc).To(Equal(expDepCtx)) + }) + It("returns error if it occurs", func() { + expErr := errors.New("collect error") + handler := newEventHandlerImpl(eventHandlerConfig{ - plus: true, - k8sReader: fake.NewFakeClient(), // client with no runtime objects will cause the collector to error + plus: true, + deployCtxCollector: &licensingfakes.FakeCollector{ + CollectStub: func(_ context.Context) (dataplane.DeploymentContext, error) { + return dataplane.DeploymentContext{}, expErr + }, + }, }) - _, err := handler.getDeploymentContext(context.Background(), ctlrZap.New()) - Expect(err).To(HaveOccurred()) + dc, err := handler.getDeploymentContext(context.Background()) + Expect(err).To(MatchError(expErr)) + Expect(dc).To(Equal(dataplane.DeploymentContext{})) }) }) }) diff --git a/internal/mode/static/licensing/collector.go b/internal/mode/static/licensing/collector.go index d3180ffc25..1c566ba608 100644 --- a/internal/mode/static/licensing/collector.go +++ b/internal/mode/static/licensing/collector.go @@ -18,7 +18,7 @@ import ( // Collector collects licensing information for N+. type Collector interface { - Collect(ctx context.Context, log logr.Logger) (dataplane.DeploymentContext, error) + Collect(ctx context.Context) (dataplane.DeploymentContext, error) } const integrationID = "ngf" @@ -29,6 +29,8 @@ type DeploymentContextCollectorConfig struct { K8sClientReader client.Reader // PodNSName is the NamespacedName of the NGF Pod. PodNSName types.NamespacedName + // Logger is the logger. + Logger logr.Logger } // DeploymentContextCollector collects the deployment context information needed for N+ licensing. @@ -46,10 +48,7 @@ func NewDeploymentContextCollector( } // Collect collects all the information needed to create the deployment context for N+ licensing. -func (c *DeploymentContextCollector) Collect( - ctx context.Context, - logger logr.Logger, -) (dataplane.DeploymentContext, error) { +func (c *DeploymentContextCollector) Collect(ctx context.Context) (dataplane.DeploymentContext, error) { clusterInfo, err := telemetry.CollectClusterInformation(ctx, c.cfg.K8sClientReader) if err != nil { return dataplane.DeploymentContext{}, fmt.Errorf("error getting cluster information: %w", err) @@ -60,11 +59,11 @@ func (c *DeploymentContextCollector) Collect( // InstallationID is not required by the usage API, so if we can't get it, don't return an error replicaSet, err := telemetry.GetPodReplicaSet(ctx, c.cfg.K8sClientReader, c.cfg.PodNSName) if err != nil { - logger.Error(err, "failed to get NGF installationID") + c.cfg.Logger.Error(err, "failed to get NGF installationID") } else { installationID, err = telemetry.GetDeploymentID(replicaSet) if err != nil { - logger.Error(err, "failed to get NGF installationID") + c.cfg.Logger.Error(err, "failed to get NGF installationID") } } diff --git a/internal/mode/static/licensing/collector_test.go b/internal/mode/static/licensing/collector_test.go index fea0449040..6d895996da 100644 --- a/internal/mode/static/licensing/collector_test.go +++ b/internal/mode/static/licensing/collector_test.go @@ -10,7 +10,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client/fake" - ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" @@ -73,7 +72,7 @@ var _ = Describe("DeploymentContextCollector", func() { ClusterNodeCount: 1, } - depCtx, err := collector.Collect(context.Background(), ctlrZap.New()) + depCtx, err := collector.Collect(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(depCtx).To(Equal(expCtx)) }) @@ -83,7 +82,7 @@ var _ = Describe("DeploymentContextCollector", func() { K8sClientReader: fake.NewFakeClient(), }) - _, err := collector.Collect(context.Background(), ctlrZap.New()) + _, err := collector.Collect(context.Background()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("error getting cluster information")) }) @@ -100,7 +99,7 @@ var _ = Describe("DeploymentContextCollector", func() { ClusterNodeCount: 1, } - depCtx, err := collector.Collect(context.Background(), ctlrZap.New()) + depCtx, err := collector.Collect(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(depCtx).To(Equal(expCtx)) }) @@ -119,7 +118,7 @@ var _ = Describe("DeploymentContextCollector", func() { ClusterNodeCount: 1, } - depCtx, err := collector.Collect(context.Background(), ctlrZap.New()) + depCtx, err := collector.Collect(context.Background()) Expect(err).ToNot(HaveOccurred()) Expect(depCtx).To(Equal(expCtx)) }) diff --git a/internal/mode/static/licensing/licensingfakes/fake_collector.go b/internal/mode/static/licensing/licensingfakes/fake_collector.go index f5f4efc8a2..d801c5a443 100644 --- a/internal/mode/static/licensing/licensingfakes/fake_collector.go +++ b/internal/mode/static/licensing/licensingfakes/fake_collector.go @@ -5,17 +5,15 @@ import ( "context" "sync" - "github.com/go-logr/logr" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) type FakeCollector struct { - CollectStub func(context.Context, logr.Logger) (dataplane.DeploymentContext, error) + CollectStub func(context.Context) (dataplane.DeploymentContext, error) collectMutex sync.RWMutex collectArgsForCall []struct { arg1 context.Context - arg2 logr.Logger } collectReturns struct { result1 dataplane.DeploymentContext @@ -29,19 +27,18 @@ type FakeCollector struct { invocationsMutex sync.RWMutex } -func (fake *FakeCollector) Collect(arg1 context.Context, arg2 logr.Logger) (dataplane.DeploymentContext, error) { +func (fake *FakeCollector) Collect(arg1 context.Context) (dataplane.DeploymentContext, error) { fake.collectMutex.Lock() ret, specificReturn := fake.collectReturnsOnCall[len(fake.collectArgsForCall)] fake.collectArgsForCall = append(fake.collectArgsForCall, struct { arg1 context.Context - arg2 logr.Logger - }{arg1, arg2}) + }{arg1}) stub := fake.CollectStub fakeReturns := fake.collectReturns - fake.recordInvocation("Collect", []interface{}{arg1, arg2}) + fake.recordInvocation("Collect", []interface{}{arg1}) fake.collectMutex.Unlock() if stub != nil { - return stub(arg1, arg2) + return stub(arg1) } if specificReturn { return ret.result1, ret.result2 @@ -55,17 +52,17 @@ func (fake *FakeCollector) CollectCallCount() int { return len(fake.collectArgsForCall) } -func (fake *FakeCollector) CollectCalls(stub func(context.Context, logr.Logger) (dataplane.DeploymentContext, error)) { +func (fake *FakeCollector) CollectCalls(stub func(context.Context) (dataplane.DeploymentContext, error)) { fake.collectMutex.Lock() defer fake.collectMutex.Unlock() fake.CollectStub = stub } -func (fake *FakeCollector) CollectArgsForCall(i int) (context.Context, logr.Logger) { +func (fake *FakeCollector) CollectArgsForCall(i int) context.Context { fake.collectMutex.RLock() defer fake.collectMutex.RUnlock() argsForCall := fake.collectArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 + return argsForCall.arg1 } func (fake *FakeCollector) CollectReturns(result1 dataplane.DeploymentContext, result2 error) { diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 5d8f630bbd..4c32d1ed2d 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -47,6 +47,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" ngftypes "github.com/nginxinc/nginx-gateway-fabric/internal/framework/types" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/metrics/collectors" ngxcfg "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies" @@ -213,6 +214,14 @@ func StartManager(cfg config.Config) error { ) groupStatusUpdater := status.NewLeaderAwareGroupUpdater(statusUpdater) + deployCtxCollector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ + K8sClientReader: mgr.GetAPIReader(), + PodNSName: types.NamespacedName{ + Namespace: cfg.GatewayPodConfig.Namespace, + Name: cfg.GatewayPodConfig.Name, + }, + Logger: cfg.Logger.WithName("deployCtxCollector"), + }) eventHandler := newEventHandlerImpl(eventHandlerConfig{ nginxFileMgr: file.NewManagerImpl( @@ -239,6 +248,7 @@ func StartManager(cfg config.Config) error { k8sReader: mgr.GetAPIReader(), logLevelSetter: logLevelSetter, eventRecorder: recorder, + deployCtxCollector: deployCtxCollector, nginxConfiguredOnStartChecker: nginxChecker, gatewayPodConfig: cfg.GatewayPodConfig, controlConfigNSName: controlConfigNSName, From cd874cf4be8bfdd0fe72fd46d6baea332ed28e84 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 6 Dec 2024 09:09:56 -0700 Subject: [PATCH 05/11] Code review --- cmd/gateway/commands.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index 824ecf5a1a..ea19235610 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -168,7 +168,7 @@ func createStaticModeCommand() *cobra.Command { podNsName, err := getPodNsName() if err != nil { - return fmt.Errorf("could not get pod nsname: %w", err) + return fmt.Errorf("could not get pod namespaced name: %w", err) } imageSource := os.Getenv("BUILD_AGENT") @@ -541,7 +541,7 @@ func createInitializeCommand() *cobra.Command { podNsName, err := getPodNsName() if err != nil { - return fmt.Errorf("could not get pod nsname: %w", err) + return fmt.Errorf("could not get pod namespaced name: %w", err) } clusterCfg := ctlr.GetConfigOrDie() From cb992d3d53fa9ea9275803b9ec8311df13d9122a Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 6 Dec 2024 14:49:07 -0700 Subject: [PATCH 06/11] Use Pod UID for install ID and always send deploy ctx --- .../templates/deployment.yaml | 8 ++ cmd/gateway/commands.go | 83 ++++++++----- cmd/gateway/commands_test.go | 50 ++++++-- cmd/gateway/initialize.go | 30 ++--- cmd/gateway/initialize_test.go | 111 ++++++++++-------- internal/mode/static/config/config.go | 2 + internal/mode/static/licensing/collector.go | 31 ++--- .../mode/static/licensing/collector_test.go | 77 ++---------- internal/mode/static/manager.go | 7 +- .../config/configfakes/fake_generator.go | 79 +++++++++++++ .../mode/static/nginx/config/generator.go | 21 ++++ .../mode/static/nginx/config/main_config.go | 21 +--- 12 files changed, 293 insertions(+), 227 deletions(-) diff --git a/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index 94832a43c2..d2a3e86a29 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -58,6 +58,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid securityContext: seccompProfile: type: RuntimeDefault @@ -142,6 +146,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: {{ .Values.nginxGateway.image.repository }}:{{ default .Chart.AppVersion .Values.nginxGateway.image.tag }} imagePullPolicy: {{ .Values.nginxGateway.image.pullPolicy }} name: nginx-gateway diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index ea19235610..34c595f455 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -23,6 +23,7 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" + ngxConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" ) @@ -161,16 +162,6 @@ func createStaticModeCommand() *cobra.Command { return fmt.Errorf("error validating ports: %w", err) } - podIP := os.Getenv("POD_IP") - if err := validateIP(podIP); err != nil { - return fmt.Errorf("error validating POD_IP environment variable: %w", err) - } - - podNsName, err := getPodNsName() - if err != nil { - return fmt.Errorf("could not get pod namespaced name: %w", err) - } - imageSource := os.Getenv("BUILD_AGENT") if imageSource != "gha" && imageSource != "local" { imageSource = "unknown" @@ -215,6 +206,11 @@ func createStaticModeCommand() *cobra.Command { flagKeys, flagValues := parseFlags(cmd.Flags()) + podConfig, err := createGatewayPodConfig(serviceName.value) + if err != nil { + return fmt.Errorf("error creating gateway pod config: %w", err) + } + conf := config.Config{ GatewayCtlrName: gatewayCtlrName.value, ConfigName: configName.String(), @@ -223,12 +219,7 @@ func createStaticModeCommand() *cobra.Command { GatewayClassName: gatewayClassName.value, GatewayNsName: gwNsName, UpdateGatewayClassStatus: updateGCStatus, - GatewayPodConfig: config.GatewayPodConfig{ - PodIP: podIP, - ServiceName: serviceName.value, - Namespace: podNsName.Namespace, - Name: podNsName.Name, - }, + GatewayPodConfig: podConfig, HealthConfig: config.HealthConfig{ Enabled: !disableHealth, Port: healthListenPort.value, @@ -241,7 +232,7 @@ func createStaticModeCommand() *cobra.Command { LeaderElection: config.LeaderElectionConfig{ Enabled: !disableLeaderElection, LockName: leaderElectionLockName.String(), - Identity: podNsName.Name, + Identity: podConfig.Name, }, UsageReportConfig: usageReportConfig, ProductTelemetryConfig: config.ProductTelemetryConfig{ @@ -539,9 +530,9 @@ func createInitializeCommand() *cobra.Command { return err } - podNsName, err := getPodNsName() + podUID, err := getValueFromEnv("POD_UID") if err != nil { - return fmt.Errorf("could not get pod namespaced name: %w", err) + return fmt.Errorf("could not get pod UID: %w", err) } clusterCfg := ctlr.GetConfigOrDie() @@ -563,15 +554,16 @@ func createInitializeCommand() *cobra.Command { dcc := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ K8sClientReader: k8sReader, - PodNSName: podNsName, + PodUID: podUID, Logger: logger.WithName("deployCtxCollector"), }) return initialize(initializeConfig{ - fileManager: file.NewStdLibOSFileManager(), - logger: logger, - plus: plus, - collector: dcc, + fileManager: file.NewStdLibOSFileManager(), + fileGenerator: ngxConfig.NewGeneratorImpl(plus, nil, logger.WithName("generator")), + logger: logger, + plus: plus, + collector: dcc, copy: copyFiles{ srcFileNames: srcFiles, destDirName: dest, @@ -652,16 +644,43 @@ func getBuildInfo() (commitHash string, commitTime string, dirtyBuild string) { return } -func getPodNsName() (types.NamespacedName, error) { - namespace := os.Getenv("POD_NAMESPACE") - if namespace == "" { - return types.NamespacedName{}, errors.New("POD_NAMESPACE environment variable must be set") +func createGatewayPodConfig(svcName string) (config.GatewayPodConfig, error) { + podIP, err := getValueFromEnv("POD_IP") + if err != nil { + return config.GatewayPodConfig{}, err } - podName := os.Getenv("POD_NAME") - if podName == "" { - return types.NamespacedName{}, errors.New("POD_NAME environment variable must be set") + podUID, err := getValueFromEnv("POD_UID") + if err != nil { + return config.GatewayPodConfig{}, err + } + + ns, err := getValueFromEnv("POD_NAMESPACE") + if err != nil { + return config.GatewayPodConfig{}, err + } + + name, err := getValueFromEnv("POD_NAME") + if err != nil { + return config.GatewayPodConfig{}, err + } + + c := config.GatewayPodConfig{ + PodIP: podIP, + ServiceName: svcName, + Namespace: ns, + Name: name, + UID: podUID, + } + + return c, nil +} + +func getValueFromEnv(key string) (string, error) { + val := os.Getenv(key) + if val == "" { + return "", fmt.Errorf("environment variable %s not set", key) } - return types.NamespacedName{Namespace: namespace, Name: podName}, nil + return val, nil } diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 55edcc185d..4576c54d2b 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -1,6 +1,7 @@ package main import ( + "errors" "io" "os" "testing" @@ -9,6 +10,8 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/types" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" ) type flagTestCase struct { @@ -660,24 +663,49 @@ func TestGetBuildInfo(t *testing.T) { g.Expect(dirtyBuild).To(Not(Equal("unknown"))) } -func TestGetPodNsName(t *testing.T) { +func TestCreateGatewayPodConfig(t *testing.T) { t.Parallel() g := NewWithT(t) + // Order matters here + // We start with all env vars set + g.Expect(os.Setenv("POD_IP", "10.0.0.0")).To(Succeed()) + g.Expect(os.Setenv("POD_UID", "1234")).To(Succeed()) g.Expect(os.Setenv("POD_NAMESPACE", "default")).To(Succeed()) g.Expect(os.Setenv("POD_NAME", "my-pod")).To(Succeed()) - nsname, err := getPodNsName() + expCfg := config.GatewayPodConfig{ + PodIP: "10.0.0.0", + ServiceName: "svc", + Namespace: "default", + Name: "my-pod", + UID: "1234", + } + cfg, err := createGatewayPodConfig("svc") g.Expect(err).To(Not(HaveOccurred())) - g.Expect(nsname).To(Equal(types.NamespacedName{Name: "my-pod", Namespace: "default"})) - - g.Expect(os.Unsetenv("POD_NAMESPACE")).To(Succeed()) - nsname, err = getPodNsName() - g.Expect(err).To(HaveOccurred()) - g.Expect(nsname).To(Equal(types.NamespacedName{})) + g.Expect(cfg).To(Equal(expCfg)) + // unset name g.Expect(os.Unsetenv("POD_NAME")).To(Succeed()) - nsname, err = getPodNsName() - g.Expect(err).To(HaveOccurred()) - g.Expect(nsname).To(Equal(types.NamespacedName{})) + cfg, err = createGatewayPodConfig("svc") + g.Expect(err).To(MatchError(errors.New("environment variable POD_NAME not set"))) + g.Expect(cfg).To(Equal(config.GatewayPodConfig{})) + + // unset namespace + g.Expect(os.Unsetenv("POD_NAMESPACE")).To(Succeed()) + cfg, err = createGatewayPodConfig("svc") + g.Expect(err).To(MatchError(errors.New("environment variable POD_NAMESPACE not set"))) + g.Expect(cfg).To(Equal(config.GatewayPodConfig{})) + + // unset UUID + g.Expect(os.Unsetenv("POD_UID")).To(Succeed()) + cfg, err = createGatewayPodConfig("svc") + g.Expect(err).To(MatchError(errors.New("environment variable POD_UID not set"))) + g.Expect(cfg).To(Equal(config.GatewayPodConfig{})) + + // unset IP + g.Expect(os.Unsetenv("POD_IP")).To(Succeed()) + cfg, err = createGatewayPodConfig("svc") + g.Expect(err).To(MatchError(errors.New("environment variable POD_IP not set"))) + g.Expect(cfg).To(Equal(config.GatewayPodConfig{})) } diff --git a/cmd/gateway/initialize.go b/cmd/gateway/initialize.go index 45c2999ae9..9972612a59 100644 --- a/cmd/gateway/initialize.go +++ b/cmd/gateway/initialize.go @@ -11,7 +11,6 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) type copyFiles struct { @@ -20,11 +19,12 @@ type copyFiles struct { } type initializeConfig struct { - collector licensing.Collector - fileManager file.OSFileManager - logger logr.Logger - copy copyFiles - plus bool + collector licensing.Collector + fileManager file.OSFileManager + fileGenerator config.Generator + logger logr.Logger + copy copyFiles + plus bool } func initialize(cfg initializeConfig) error { @@ -44,30 +44,22 @@ func initialize(cfg initializeConfig) error { depCtx, err := cfg.collector.Collect(ctx) if err != nil { - return fmt.Errorf("failed to collect deployment context: %w", err) + cfg.logger.Error(err, "error collecting deployment context") } cfg.logger.Info("Deployment context collected", "deployment context", depCtx) - if err := writeDeploymentContextFile(cfg.fileManager, depCtx); err != nil { - return fmt.Errorf("failed to write deployment context file: %w", err) - } - - cfg.logger.Info("Finished initializing configuration") - - return nil -} - -func writeDeploymentContextFile(osFileManager file.OSFileManager, depCtx dataplane.DeploymentContext) error { - depCtxFile, err := config.GenerateDeploymentContextFile(depCtx) + depCtxFile, err := cfg.fileGenerator.GenerateDeploymentContext(depCtx) if err != nil { return fmt.Errorf("failed to generate deployment context file: %w", err) } - if err := file.WriteFile(osFileManager, depCtxFile); err != nil { + if err := file.WriteFile(cfg.fileManager, depCtxFile); err != nil { return fmt.Errorf("failed to write deployment context file: %w", err) } + cfg.logger.Info("Finished initializing configuration") + return nil } diff --git a/cmd/gateway/initialize_test.go b/cmd/gateway/initialize_test.go index c4c282e940..708675b1e7 100644 --- a/cmd/gateway/initialize_test.go +++ b/cmd/gateway/initialize_test.go @@ -12,6 +12,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing/licensingfakes" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/configfakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file/filefakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" @@ -68,62 +69,72 @@ func TestInitialize_OSS_Error(t *testing.T) { func TestInitialize_Plus(t *testing.T) { t.Parallel() - g := NewGomegaWithT(t) - - fakeFileMgr := &filefakes.FakeOSFileManager{} - fakeCollector := &licensingfakes.FakeCollector{} - ic := initializeConfig{ - fileManager: fakeFileMgr, - logger: zap.New(), - collector: fakeCollector, - copy: copyFiles{ - destDirName: "destDir", - srcFileNames: []string{"src1", "src2"}, + tests := []struct { + name string + collectErr error + depCtx dataplane.DeploymentContext + }{ + { + name: "normal", + collectErr: nil, + depCtx: dataplane.DeploymentContext{ + Integration: "ngf", + ClusterID: "cluster-id", + InstallationID: "install-id", + ClusterNodeCount: 2, + }, }, - plus: true, - } - - err := initialize(ic) - g.Expect(err).ToNot(HaveOccurred()) - // copies - g.Expect(fakeFileMgr.OpenCallCount()).To(Equal(2)) - g.Expect(fakeFileMgr.CopyCallCount()).To(Equal(2)) - - // 2 copies, 1 write deploy ctx - g.Expect(fakeFileMgr.CreateCallCount()).To(Equal(3)) - // write deploy ctx - g.Expect(fakeCollector.CollectCallCount()).To(Equal(1)) - g.Expect(fakeFileMgr.WriteCallCount()).To(Equal(1)) - g.Expect(fakeFileMgr.ChmodCallCount()).To(Equal(1)) -} - -func TestInitialize_Plus_Error(t *testing.T) { - t.Parallel() - g := NewGomegaWithT(t) - - collectErr := errors.New("collect error") - fakeFileMgr := &filefakes.FakeOSFileManager{} - fakeCollector := &licensingfakes.FakeCollector{ - CollectStub: func(_ context.Context) (dataplane.DeploymentContext, error) { - return dataplane.DeploymentContext{}, collectErr + { + name: "collecting deployment context errors", + collectErr: errors.New("collect error"), + depCtx: dataplane.DeploymentContext{ + Integration: "ngf", + InstallationID: "install-id", + }, }, } - ic := initializeConfig{ - fileManager: fakeFileMgr, - logger: zap.New(), - collector: fakeCollector, - copy: copyFiles{ - destDirName: "destDir", - srcFileNames: []string{"src1", "src2"}, - }, - plus: true, - } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) - err := initialize(ic) - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(collectErr)) + fakeFileMgr := &filefakes.FakeOSFileManager{} + fakeCollector := &licensingfakes.FakeCollector{ + CollectStub: func(_ context.Context) (dataplane.DeploymentContext, error) { + return test.depCtx, test.collectErr + }, + } + fakeGenerator := &configfakes.FakeGenerator{} + + ic := initializeConfig{ + fileManager: fakeFileMgr, + logger: zap.New(), + collector: fakeCollector, + fileGenerator: fakeGenerator, + copy: copyFiles{ + destDirName: "destDir", + srcFileNames: []string{"src1", "src2"}, + }, + plus: true, + } + + g.Expect(initialize(ic)).To(Succeed()) + // copies + g.Expect(fakeFileMgr.OpenCallCount()).To(Equal(2)) + g.Expect(fakeFileMgr.CopyCallCount()).To(Equal(2)) + + // 2 copies, 1 write deploy ctx + g.Expect(fakeFileMgr.CreateCallCount()).To(Equal(3)) + // write deploy ctx + g.Expect(fakeGenerator.GenerateDeploymentContextCallCount()).To(Equal(1)) + g.Expect(fakeGenerator.GenerateDeploymentContextArgsForCall(0)).To(Equal(test.depCtx)) + g.Expect(fakeCollector.CollectCallCount()).To(Equal(1)) + g.Expect(fakeFileMgr.WriteCallCount()).To(Equal(1)) + g.Expect(fakeFileMgr.ChmodCallCount()).To(Equal(1)) + }) + } } func TestCopyFile(t *testing.T) { diff --git a/internal/mode/static/config/config.go b/internal/mode/static/config/config.go index dbcbfa6b81..82b4238836 100644 --- a/internal/mode/static/config/config.go +++ b/internal/mode/static/config/config.go @@ -60,6 +60,8 @@ type GatewayPodConfig struct { Namespace string // Name is the name of the Pod. Name string + // UID is the UID of the Pod. + UID string } // MetricsConfig specifies the metrics config. diff --git a/internal/mode/static/licensing/collector.go b/internal/mode/static/licensing/collector.go index 1c566ba608..002cc6490f 100644 --- a/internal/mode/static/licensing/collector.go +++ b/internal/mode/static/licensing/collector.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" @@ -27,8 +26,8 @@ const integrationID = "ngf" type DeploymentContextCollectorConfig struct { // K8sClientReader is a Kubernetes API client Reader. K8sClientReader client.Reader - // PodNSName is the NamespacedName of the NGF Pod. - PodNSName types.NamespacedName + // PodUID is the UID of the NGF Pod. + PodUID string // Logger is the logger. Logger logr.Logger } @@ -49,30 +48,18 @@ func NewDeploymentContextCollector( // Collect collects all the information needed to create the deployment context for N+ licensing. func (c *DeploymentContextCollector) Collect(ctx context.Context) (dataplane.DeploymentContext, error) { - clusterInfo, err := telemetry.CollectClusterInformation(ctx, c.cfg.K8sClientReader) - if err != nil { - return dataplane.DeploymentContext{}, fmt.Errorf("error getting cluster information: %w", err) + depCtx := dataplane.DeploymentContext{ + Integration: integrationID, + InstallationID: c.cfg.PodUID, } - var installationID string - - // InstallationID is not required by the usage API, so if we can't get it, don't return an error - replicaSet, err := telemetry.GetPodReplicaSet(ctx, c.cfg.K8sClientReader, c.cfg.PodNSName) + clusterInfo, err := telemetry.CollectClusterInformation(ctx, c.cfg.K8sClientReader) if err != nil { - c.cfg.Logger.Error(err, "failed to get NGF installationID") - } else { - installationID, err = telemetry.GetDeploymentID(replicaSet) - if err != nil { - c.cfg.Logger.Error(err, "failed to get NGF installationID") - } + return depCtx, fmt.Errorf("error collecting cluster ID and cluster node count: %w", err) } - depCtx := dataplane.DeploymentContext{ - Integration: integrationID, - ClusterID: clusterInfo.ClusterID, - ClusterNodeCount: clusterInfo.NodeCount, - InstallationID: installationID, - } + depCtx.ClusterID = clusterInfo.ClusterID + depCtx.ClusterNodeCount = clusterInfo.NodeCount return depCtx, nil } diff --git a/internal/mode/static/licensing/collector_test.go b/internal/mode/static/licensing/collector_test.go index 6d895996da..dcb5f6e793 100644 --- a/internal/mode/static/licensing/collector_test.go +++ b/internal/mode/static/licensing/collector_test.go @@ -5,13 +5,10 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/licensing" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) @@ -19,33 +16,6 @@ import ( var _ = Describe("DeploymentContextCollector", func() { var ( clusterID = "test-uid" - ngfPod = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod1", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "replicaset1", - }, - }, - }, - } - - ngfReplicaSet = &appsv1.ReplicaSet{ - Spec: appsv1.ReplicaSetSpec{ - Replicas: helpers.GetPointer[int32](1), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "replicaset1", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "Deployment1", - UID: "test-uid-replicaSet", - }, - }, - }, - } kubeNamespace = &v1.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -61,14 +31,14 @@ var _ = Describe("DeploymentContextCollector", func() { It("collects the deployment context", func() { collector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ - K8sClientReader: fake.NewFakeClient(ngfPod, ngfReplicaSet, kubeNamespace, nodeList), - PodNSName: types.NamespacedName{Name: ngfPod.Name}, + K8sClientReader: fake.NewFakeClient(kubeNamespace, nodeList), + PodUID: "pod-uid", }) expCtx := dataplane.DeploymentContext{ Integration: "ngf", ClusterID: clusterID, - InstallationID: "test-uid-replicaSet", + InstallationID: "pod-uid", ClusterNodeCount: 1, } @@ -77,49 +47,20 @@ var _ = Describe("DeploymentContextCollector", func() { Expect(depCtx).To(Equal(expCtx)) }) - It("returns an error if cluster info isn't found", func() { + It("returns an error and default deployment context if cluster info isn't found", func() { collector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ K8sClientReader: fake.NewFakeClient(), - }) - - _, err := collector.Collect(context.Background()) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("error getting cluster information")) - }) - - It("returns the deployment context when the replicaset isn't found", func() { - collector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ - K8sClientReader: fake.NewFakeClient(ngfPod, kubeNamespace, nodeList), - PodNSName: types.NamespacedName{Name: ngfPod.Name}, - }) - - expCtx := dataplane.DeploymentContext{ - Integration: "ngf", - ClusterID: clusterID, - ClusterNodeCount: 1, - } - - depCtx, err := collector.Collect(context.Background()) - Expect(err).ToNot(HaveOccurred()) - Expect(depCtx).To(Equal(expCtx)) - }) - - It("returns the deployment context when the replicaset doesn't have a uid", func() { - ngfReplicaSet.ObjectMeta.OwnerReferences[0].UID = "" - - collector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ - K8sClientReader: fake.NewFakeClient(ngfPod, ngfReplicaSet, kubeNamespace, nodeList), - PodNSName: types.NamespacedName{Name: ngfPod.Name}, + PodUID: "pod-uid", }) expCtx := dataplane.DeploymentContext{ - Integration: "ngf", - ClusterID: clusterID, - ClusterNodeCount: 1, + Integration: "ngf", + InstallationID: "pod-uid", } depCtx, err := collector.Collect(context.Background()) - Expect(err).ToNot(HaveOccurred()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("error collecting cluster ID and cluster node count")) Expect(depCtx).To(Equal(expCtx)) }) }) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 4c32d1ed2d..bc24210318 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -216,11 +216,8 @@ func StartManager(cfg config.Config) error { groupStatusUpdater := status.NewLeaderAwareGroupUpdater(statusUpdater) deployCtxCollector := licensing.NewDeploymentContextCollector(licensing.DeploymentContextCollectorConfig{ K8sClientReader: mgr.GetAPIReader(), - PodNSName: types.NamespacedName{ - Namespace: cfg.GatewayPodConfig.Namespace, - Name: cfg.GatewayPodConfig.Name, - }, - Logger: cfg.Logger.WithName("deployCtxCollector"), + PodUID: cfg.GatewayPodConfig.UID, + Logger: cfg.Logger.WithName("deployCtxCollector"), }) eventHandler := newEventHandlerImpl(eventHandlerConfig{ diff --git a/internal/mode/static/nginx/config/configfakes/fake_generator.go b/internal/mode/static/nginx/config/configfakes/fake_generator.go index 828e41951b..d2b3a6e7b7 100644 --- a/internal/mode/static/nginx/config/configfakes/fake_generator.go +++ b/internal/mode/static/nginx/config/configfakes/fake_generator.go @@ -21,6 +21,19 @@ type FakeGenerator struct { generateReturnsOnCall map[int]struct { result1 []file.File } + GenerateDeploymentContextStub func(dataplane.DeploymentContext) (file.File, error) + generateDeploymentContextMutex sync.RWMutex + generateDeploymentContextArgsForCall []struct { + arg1 dataplane.DeploymentContext + } + generateDeploymentContextReturns struct { + result1 file.File + result2 error + } + generateDeploymentContextReturnsOnCall map[int]struct { + result1 file.File + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -86,11 +99,77 @@ func (fake *FakeGenerator) GenerateReturnsOnCall(i int, result1 []file.File) { }{result1} } +func (fake *FakeGenerator) GenerateDeploymentContext(arg1 dataplane.DeploymentContext) (file.File, error) { + fake.generateDeploymentContextMutex.Lock() + ret, specificReturn := fake.generateDeploymentContextReturnsOnCall[len(fake.generateDeploymentContextArgsForCall)] + fake.generateDeploymentContextArgsForCall = append(fake.generateDeploymentContextArgsForCall, struct { + arg1 dataplane.DeploymentContext + }{arg1}) + stub := fake.GenerateDeploymentContextStub + fakeReturns := fake.generateDeploymentContextReturns + fake.recordInvocation("GenerateDeploymentContext", []interface{}{arg1}) + fake.generateDeploymentContextMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeGenerator) GenerateDeploymentContextCallCount() int { + fake.generateDeploymentContextMutex.RLock() + defer fake.generateDeploymentContextMutex.RUnlock() + return len(fake.generateDeploymentContextArgsForCall) +} + +func (fake *FakeGenerator) GenerateDeploymentContextCalls(stub func(dataplane.DeploymentContext) (file.File, error)) { + fake.generateDeploymentContextMutex.Lock() + defer fake.generateDeploymentContextMutex.Unlock() + fake.GenerateDeploymentContextStub = stub +} + +func (fake *FakeGenerator) GenerateDeploymentContextArgsForCall(i int) dataplane.DeploymentContext { + fake.generateDeploymentContextMutex.RLock() + defer fake.generateDeploymentContextMutex.RUnlock() + argsForCall := fake.generateDeploymentContextArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeGenerator) GenerateDeploymentContextReturns(result1 file.File, result2 error) { + fake.generateDeploymentContextMutex.Lock() + defer fake.generateDeploymentContextMutex.Unlock() + fake.GenerateDeploymentContextStub = nil + fake.generateDeploymentContextReturns = struct { + result1 file.File + result2 error + }{result1, result2} +} + +func (fake *FakeGenerator) GenerateDeploymentContextReturnsOnCall(i int, result1 file.File, result2 error) { + fake.generateDeploymentContextMutex.Lock() + defer fake.generateDeploymentContextMutex.Unlock() + fake.GenerateDeploymentContextStub = nil + if fake.generateDeploymentContextReturnsOnCall == nil { + fake.generateDeploymentContextReturnsOnCall = make(map[int]struct { + result1 file.File + result2 error + }) + } + fake.generateDeploymentContextReturnsOnCall[i] = struct { + result1 file.File + result2 error + }{result1, result2} +} + func (fake *FakeGenerator) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() fake.generateMutex.RLock() defer fake.generateMutex.RUnlock() + fake.generateDeploymentContextMutex.RLock() + defer fake.generateDeploymentContextMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index 9792eb4d9a..714ade9dd3 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -1,6 +1,8 @@ package config import ( + "encoding/json" + "fmt" "path/filepath" "github.com/go-logr/logr" @@ -65,6 +67,8 @@ var ConfigFolders = []string{httpFolder, secretsFolder, includesFolder, mainIncl type Generator interface { // Generate generates NGINX configuration files from internal representation. Generate(configuration dataplane.Configuration) []file.File + // GenerateDeploymentContext generates the deployment context used for N+ licensing. + GenerateDeploymentContext(depCtx dataplane.DeploymentContext) (file.File, error) } // GeneratorImpl is an implementation of Generator. @@ -125,6 +129,23 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []file.File { return files } +// GenerateDeploymentContext generates the deployment_ctx.json file needed for N+ licensing. +// It's exported since it's used by the init container process. +func (g GeneratorImpl) GenerateDeploymentContext(depCtx dataplane.DeploymentContext) (file.File, error) { + depCtxBytes, err := json.Marshal(depCtx) + if err != nil { + return file.File{}, fmt.Errorf("error building deployment context for mgmt block: %w", err) + } + + deploymentCtxFile := file.File{ + Content: depCtxBytes, + Path: mainIncludesFolder + "/deployment_ctx.json", + Type: file.TypeRegular, + } + + return deploymentCtxFile, nil +} + func (g GeneratorImpl) executeConfigTemplates( conf dataplane.Configuration, generator policies.Generator, diff --git a/internal/mode/static/nginx/config/main_config.go b/internal/mode/static/nginx/config/main_config.go index cea4262dfa..58cdef68cb 100644 --- a/internal/mode/static/nginx/config/main_config.go +++ b/internal/mode/static/nginx/config/main_config.go @@ -1,8 +1,6 @@ package config import ( - "encoding/json" - "fmt" gotemplate "text/template" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" @@ -22,23 +20,6 @@ type mainConfig struct { Conf dataplane.Configuration } -// GenerateDeploymentContextFile generates the deployment_ctx.json file needed for N+ licensing. -// It's exported since it's used by the init container process. -func GenerateDeploymentContextFile(depCtx dataplane.DeploymentContext) (file.File, error) { - depCtxBytes, err := json.Marshal(depCtx) - if err != nil { - return file.File{}, fmt.Errorf("error building deployment context for mgmt block: %w", err) - } - - deploymentCtxFile := file.File{ - Content: depCtxBytes, - Path: mainIncludesFolder + "/deployment_ctx.json", - Type: file.TypeRegular, - } - - return deploymentCtxFile, nil -} - func executeMainConfig(conf dataplane.Configuration) []executeResult { includes := createIncludesFromSnippets(conf.MainSnippets) @@ -123,7 +104,7 @@ func (g GeneratorImpl) generateMgmtFiles(conf dataplane.Configuration) []file.Fi files = append(files, keyFile) } - deploymentCtxFile, err := GenerateDeploymentContextFile(conf.DeploymentContext) + deploymentCtxFile, err := g.GenerateDeploymentContext(conf.DeploymentContext) if err != nil { g.logger.Error(err, "error building deployment context for mgmt block") } else { From a5452bf2cf39d2abd559ab00e88454254d619afc Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Fri, 6 Dec 2024 14:56:39 -0700 Subject: [PATCH 07/11] Add comment and constant --- cmd/gateway/initialize.go | 6 +++++- internal/mode/static/licensing/collector.go | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/gateway/initialize.go b/cmd/gateway/initialize.go index 9972612a59..1aa01f923e 100644 --- a/cmd/gateway/initialize.go +++ b/cmd/gateway/initialize.go @@ -13,6 +13,10 @@ import ( "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file" ) +const ( + collectDeployCtxTimeout = 10 * time.Second +) + type copyFiles struct { destDirName string srcFileNames []string @@ -39,7 +43,7 @@ func initialize(cfg initializeConfig) error { return nil } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), collectDeployCtxTimeout) defer cancel() depCtx, err := cfg.collector.Collect(ctx) diff --git a/internal/mode/static/licensing/collector.go b/internal/mode/static/licensing/collector.go index 002cc6490f..7883b0ce58 100644 --- a/internal/mode/static/licensing/collector.go +++ b/internal/mode/static/licensing/collector.go @@ -17,6 +17,7 @@ import ( // Collector collects licensing information for N+. type Collector interface { + // Collect collects the licensing information for N+ and returns it in the deployment context. Collect(ctx context.Context) (dataplane.DeploymentContext, error) } From 7dda8cf3d8aa988585c8634ebf5e760b190db44c Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Mon, 9 Dec 2024 13:26:13 -0700 Subject: [PATCH 08/11] Unexport telemetry methods --- internal/mode/static/telemetry/collector.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index 0e1eaf06bb..cf58cc3d20 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -148,7 +148,7 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { return Data{}, fmt.Errorf("failed to collect NGF resource counts: %w", err) } - replicaSet, err := GetPodReplicaSet(ctx, c.cfg.K8sClientReader, c.cfg.PodNSName) + replicaSet, err := getPodReplicaSet(ctx, c.cfg.K8sClientReader, c.cfg.PodNSName) if err != nil { return Data{}, fmt.Errorf("failed to get replica set for pod %v: %w", c.cfg.PodNSName, err) } @@ -158,7 +158,7 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) { return Data{}, fmt.Errorf("failed to collect NGF replica count: %w", err) } - deploymentID, err := GetDeploymentID(replicaSet) + deploymentID, err := getDeploymentID(replicaSet) if err != nil { return Data{}, fmt.Errorf("failed to get NGF deploymentID: %w", err) } @@ -280,8 +280,8 @@ func computeRouteCount( } } -// GetPodReplicaSet returns the replicaset for the provided Pod. -func GetPodReplicaSet( +// getPodReplicaSet returns the replicaset for the provided Pod. +func getPodReplicaSet( ctx context.Context, k8sClient client.Reader, podNSName types.NamespacedName, @@ -324,8 +324,8 @@ func getReplicas(replicaSet *appsv1.ReplicaSet) (int, error) { return int(*replicaSet.Spec.Replicas), nil } -// GetDeploymentID gets the deployment ID of the provided ReplicaSet. -func GetDeploymentID(replicaSet *appsv1.ReplicaSet) (string, error) { +// getDeploymentID gets the deployment ID of the provided ReplicaSet. +func getDeploymentID(replicaSet *appsv1.ReplicaSet) (string, error) { replicaOwnerRefs := replicaSet.GetOwnerReferences() if len(replicaOwnerRefs) != 1 { return "", fmt.Errorf("expected one owner reference of the NGF ReplicaSet, got %d", len(replicaOwnerRefs)) From 00e8a4832896b60317b1f4e2e58ea13e87adcf26 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Mon, 9 Dec 2024 14:44:24 -0700 Subject: [PATCH 09/11] Fix UID comment --- cmd/gateway/commands_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gateway/commands_test.go b/cmd/gateway/commands_test.go index 4576c54d2b..a714004bff 100644 --- a/cmd/gateway/commands_test.go +++ b/cmd/gateway/commands_test.go @@ -697,7 +697,7 @@ func TestCreateGatewayPodConfig(t *testing.T) { g.Expect(err).To(MatchError(errors.New("environment variable POD_NAMESPACE not set"))) g.Expect(cfg).To(Equal(config.GatewayPodConfig{})) - // unset UUID + // unset pod UID g.Expect(os.Unsetenv("POD_UID")).To(Succeed()) cfg, err = createGatewayPodConfig("svc") g.Expect(err).To(MatchError(errors.New("environment variable POD_UID not set"))) From eb30efebbd74fc0d272c4a839967bc17625d4386 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Wed, 11 Dec 2024 10:37:44 -0700 Subject: [PATCH 10/11] Generate files --- config/tests/static-deployment.yaml | 8 ++++++++ deploy/aws-nlb/deploy.yaml | 8 ++++++++ deploy/azure/deploy.yaml | 8 ++++++++ deploy/default/deploy.yaml | 8 ++++++++ deploy/experimental-nginx-plus/deploy.yaml | 8 ++++++++ deploy/experimental/deploy.yaml | 8 ++++++++ deploy/nginx-plus/deploy.yaml | 8 ++++++++ deploy/nodeport/deploy.yaml | 8 ++++++++ deploy/openshift/deploy.yaml | 8 ++++++++ deploy/snippets-filters-nginx-plus/deploy.yaml | 8 ++++++++ deploy/snippets-filters/deploy.yaml | 8 ++++++++ 11 files changed, 88 insertions(+) diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index 63cb16b010..114714005f 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -41,6 +41,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid securityContext: seccompProfile: type: RuntimeDefault @@ -81,6 +85,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway diff --git a/deploy/aws-nlb/deploy.yaml b/deploy/aws-nlb/deploy.yaml index 91a9aee1da..8e103f9b4c 100644 --- a/deploy/aws-nlb/deploy.yaml +++ b/deploy/aws-nlb/deploy.yaml @@ -230,6 +230,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -320,6 +324,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index 8641af0686..85be9df807 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -227,6 +227,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -317,6 +321,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index c5284d27bd..c54ebf86ef 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -227,6 +227,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -317,6 +321,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index 12f323586b..b361c85c21 100644 --- a/deploy/experimental-nginx-plus/deploy.yaml +++ b/deploy/experimental-nginx-plus/deploy.yaml @@ -248,6 +248,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -346,6 +350,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index 189a65805b..e08fcda03f 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -233,6 +233,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -323,6 +327,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml index 87177ca9ea..c78ca01634 100644 --- a/deploy/nginx-plus/deploy.yaml +++ b/deploy/nginx-plus/deploy.yaml @@ -242,6 +242,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -340,6 +344,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index 0263036561..d5a3bd0551 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -227,6 +227,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -317,6 +321,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml index 7509e76b7a..abad970041 100644 --- a/deploy/openshift/deploy.yaml +++ b/deploy/openshift/deploy.yaml @@ -235,6 +235,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -325,6 +329,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init diff --git a/deploy/snippets-filters-nginx-plus/deploy.yaml b/deploy/snippets-filters-nginx-plus/deploy.yaml index f959e99113..a228a369b3 100644 --- a/deploy/snippets-filters-nginx-plus/deploy.yaml +++ b/deploy/snippets-filters-nginx-plus/deploy.yaml @@ -245,6 +245,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -343,6 +347,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init diff --git a/deploy/snippets-filters/deploy.yaml b/deploy/snippets-filters/deploy.yaml index 6a0727a206..ade8c672a8 100644 --- a/deploy/snippets-filters/deploy.yaml +++ b/deploy/snippets-filters/deploy.yaml @@ -230,6 +230,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: nginx-gateway @@ -320,6 +324,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_UID + valueFrom: + fieldRef: + fieldPath: metadata.uid image: ghcr.io/nginxinc/nginx-gateway-fabric:edge imagePullPolicy: Always name: init From a8d01cf137023253c0143ae5aad56656ec262f13 Mon Sep 17 00:00:00 2001 From: Kate Osborn Date: Wed, 11 Dec 2024 11:42:34 -0700 Subject: [PATCH 11/11] Remove POD_NAMESPACE and POD_NAME from init container --- charts/nginx-gateway-fabric/templates/deployment.yaml | 8 -------- config/tests/static-deployment.yaml | 8 -------- deploy/aws-nlb/deploy.yaml | 8 -------- deploy/azure/deploy.yaml | 8 -------- deploy/default/deploy.yaml | 8 -------- deploy/experimental-nginx-plus/deploy.yaml | 8 -------- deploy/experimental/deploy.yaml | 8 -------- deploy/nginx-plus/deploy.yaml | 8 -------- deploy/nodeport/deploy.yaml | 8 -------- deploy/openshift/deploy.yaml | 8 -------- deploy/snippets-filters-nginx-plus/deploy.yaml | 8 -------- deploy/snippets-filters/deploy.yaml | 8 -------- 12 files changed, 96 deletions(-) diff --git a/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index d2a3e86a29..4149bdf587 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -50,14 +50,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index 114714005f..e84e3cbfd4 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -33,14 +33,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/aws-nlb/deploy.yaml b/deploy/aws-nlb/deploy.yaml index 8e103f9b4c..6e4ff4865f 100644 --- a/deploy/aws-nlb/deploy.yaml +++ b/deploy/aws-nlb/deploy.yaml @@ -316,14 +316,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index 85be9df807..06ba7a02ae 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -313,14 +313,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index c54ebf86ef..7771ea7a3f 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -313,14 +313,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index b361c85c21..6e37bdbba6 100644 --- a/deploy/experimental-nginx-plus/deploy.yaml +++ b/deploy/experimental-nginx-plus/deploy.yaml @@ -342,14 +342,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index e08fcda03f..e6dd9e8f24 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -319,14 +319,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml index c78ca01634..65d5e68bee 100644 --- a/deploy/nginx-plus/deploy.yaml +++ b/deploy/nginx-plus/deploy.yaml @@ -336,14 +336,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index d5a3bd0551..36d5ff08ec 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -313,14 +313,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml index abad970041..d66e286be5 100644 --- a/deploy/openshift/deploy.yaml +++ b/deploy/openshift/deploy.yaml @@ -321,14 +321,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/snippets-filters-nginx-plus/deploy.yaml b/deploy/snippets-filters-nginx-plus/deploy.yaml index a228a369b3..efc9f843ad 100644 --- a/deploy/snippets-filters-nginx-plus/deploy.yaml +++ b/deploy/snippets-filters-nginx-plus/deploy.yaml @@ -339,14 +339,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: diff --git a/deploy/snippets-filters/deploy.yaml b/deploy/snippets-filters/deploy.yaml index ade8c672a8..743b8f7fdf 100644 --- a/deploy/snippets-filters/deploy.yaml +++ b/deploy/snippets-filters/deploy.yaml @@ -316,14 +316,6 @@ spec: - --destination - /etc/nginx/main-includes env: - - name: POD_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - - name: POD_NAME - valueFrom: - fieldRef: - fieldPath: metadata.name - name: POD_UID valueFrom: fieldRef: