Omit empty deployment context fields (#2903) (#2910)

Problem: If an optional deployment context field wasn't set, an empty value would still be sent and cause the reporting server to return a 400. Solution: Use pointers on the optional fields to omit them from the context if they're empty.
nginxinc · Dec 13, 2024 · 9c2deea · 9c2deea
1 parent e153640
commit 9c2deea
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 22 deletions.
diff --git a/cmd/gateway/initialize_test.go b/cmd/gateway/initialize_test.go
@@ -11,6 +11,7 @@ import (
 	. "github.com/onsi/gomega"
 	"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/licensingfakes"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/configfakes"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
@@ -80,17 +81,17 @@ func TestInitialize_Plus(t *testing.T) {
 			collectErr: nil,
 			depCtx: dataplane.DeploymentContext{
 				Integration:      "ngf",
-				ClusterID:        "cluster-id",
-				InstallationID:   "install-id",
-				ClusterNodeCount: 2,
+				ClusterID:        helpers.GetPointer("cluster-id"),
+				InstallationID:   helpers.GetPointer("install-id"),
+				ClusterNodeCount: helpers.GetPointer(2),
 			},
 		},
 		{
 			name:       "collecting deployment context errors",
 			collectErr: errors.New("collect error"),
 			depCtx: dataplane.DeploymentContext{
 				Integration:    "ngf",
-				InstallationID: "install-id",
+				InstallationID: helpers.GetPointer("install-id"),
 			},
 		},
 	}

diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go
@@ -714,9 +714,9 @@ var _ = Describe("getDeploymentContext", func() {
 		It("returns deployment context", func() {
 			expDepCtx := dataplane.DeploymentContext{
 				Integration:      "ngf",
-				ClusterID:        "cluster-id",
-				InstallationID:   "installation-id",
-				ClusterNodeCount: 1,
+				ClusterID:        helpers.GetPointer("cluster-id"),
+				InstallationID:   helpers.GetPointer("installation-id"),
+				ClusterNodeCount: helpers.GetPointer(1),
 			}
 
 			handler := newEventHandlerImpl(eventHandlerConfig{

diff --git a/internal/mode/static/licensing/collector.go b/internal/mode/static/licensing/collector.go
@@ -51,16 +51,16 @@ func NewDeploymentContextCollector(
 func (c *DeploymentContextCollector) Collect(ctx context.Context) (dataplane.DeploymentContext, error) {
 	depCtx := dataplane.DeploymentContext{
 		Integration:    integrationID,
-		InstallationID: c.cfg.PodUID,
+		InstallationID: &c.cfg.PodUID,
 	}
 
 	clusterInfo, err := telemetry.CollectClusterInformation(ctx, c.cfg.K8sClientReader)
 	if err != nil {
 		return depCtx, fmt.Errorf("error collecting cluster ID and cluster node count: %w", err)
 	}
 
-	depCtx.ClusterID = clusterInfo.ClusterID
-	depCtx.ClusterNodeCount = clusterInfo.NodeCount
+	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
@@ -9,6 +9,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"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"
 )
@@ -37,9 +38,9 @@ var _ = Describe("DeploymentContextCollector", func() {
 
 		expCtx := dataplane.DeploymentContext{
 			Integration:      "ngf",
-			ClusterID:        clusterID,
-			InstallationID:   "pod-uid",
-			ClusterNodeCount: 1,
+			ClusterID:        &clusterID,
+			InstallationID:   helpers.GetPointer("pod-uid"),
+			ClusterNodeCount: helpers.GetPointer(1),
 		}
 
 		depCtx, err := collector.Collect(context.Background())
@@ -55,7 +56,7 @@ var _ = Describe("DeploymentContextCollector", func() {
 
 		expCtx := dataplane.DeploymentContext{
 			Integration:    "ngf",
-			InstallationID: "pod-uid",
+			InstallationID: helpers.GetPointer("pod-uid"),
 		}
 
 		depCtx, err := collector.Collect(context.Background())

diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go
@@ -9,6 +9,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap"
 
+	"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
 	ngfConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
@@ -121,9 +122,9 @@ func TestGenerate(t *testing.T) {
 		},
 		DeploymentContext: dataplane.DeploymentContext{
 			Integration:      "ngf",
-			ClusterID:        "test-uid",
-			InstallationID:   "test-uid-replicaSet",
-			ClusterNodeCount: 1,
+			ClusterID:        helpers.GetPointer("test-uid"),
+			InstallationID:   helpers.GetPointer("test-uid-replicaSet"),
+			ClusterNodeCount: helpers.GetPointer(1),
 		},
 		AuxiliarySecrets: map[graph.SecretFileType][]byte{
 			graph.PlusReportJWTToken:             []byte("license"),

diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go
@@ -399,12 +399,12 @@ type Logging struct {
 // DeploymentContext contains metadata about NGF and the cluster.
 // This is JSON marshaled into a file created by the generator, hence the json tags.
 type DeploymentContext struct {
-	// Integration is "ngf".
-	Integration string `json:"integration"`
 	// ClusterID is the ID of the kube-system namespace.
-	ClusterID string `json:"cluster_id"`
+	ClusterID *string `json:"cluster_id,omitempty"`
 	// InstallationID is the ID of the NGF deployment.
-	InstallationID string `json:"installation_id"`
+	InstallationID *string `json:"installation_id,omitempty"`
 	// ClusterNodeCount is the count of nodes in the cluster.
-	ClusterNodeCount int `json:"cluster_node_count"`
+	ClusterNodeCount *int `json:"cluster_node_count,omitempty"`
+	// Integration is "ngf".
+	Integration string `json:"integration"`
 }