From e0c3f4b69aa0ab44d961d7ecc0f25cd1d14e00f1 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Fri, 13 Dec 2024 10:45:07 -0700 Subject: [PATCH] Omit empty deployment context fields 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. --- cmd/gateway/initialize_test.go | 9 +++++---- internal/mode/static/handler_test.go | 6 +++--- internal/mode/static/licensing/collector.go | 6 +++--- internal/mode/static/licensing/collector_test.go | 9 +++++---- internal/mode/static/nginx/config/generator_test.go | 7 ++++--- internal/mode/static/state/dataplane/types.go | 10 +++++----- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/cmd/gateway/initialize_test.go b/cmd/gateway/initialize_test.go index 708675b1e7..507acc6fa0 100644 --- 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,9 +81,9 @@ 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), }, }, { @@ -90,7 +91,7 @@ func TestInitialize_Plus(t *testing.T) { 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 index c24f5d27d2..8d8ef9a8d3 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -754,9 +754,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 index 7883b0ce58..4d4d1a945d 100644 --- a/internal/mode/static/licensing/collector.go +++ b/internal/mode/static/licensing/collector.go @@ -51,7 +51,7 @@ 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) @@ -59,8 +59,8 @@ func (c *DeploymentContextCollector) Collect(ctx context.Context) (dataplane.Dep 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 index dcb5f6e793..c01b169615 100644 --- 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 index f343e3dc42..a0a5670c26 100644 --- 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 index 274897c007..f9ed79b762 100644 --- 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"` }