Simplify target customisations for namespace labels and annotations

e2e/assets/single-cluster/namespaceLabels_targetCustomization.yaml

@@ -4,7 +4,7 @@ metadata:
   name: test
   namespace: fleet-local
 spec:
-  repo: https://github.com/Tommy12789/fleet-examples-tommy
-  branch: test
+  repo: https://github.com/rancher/fleet-test-data
+  branch: master
   paths:
-    - namespaceLabels_targetCustomization
+    - target-customization-namespace-labels

e2e/single-cluster/targetCustomization_test.go

@@ -33,14 +33,23 @@ var _ = Describe("Helm deploy options", func() {
 		BeforeEach(func() {
 			asset = "single-cluster/namespaceLabels_targetCustomization.yaml"
 		})
-		When("namespaceLabels set as targetCustomization ", func() {
-			It("deploy the bundledeployment with the merged namespaceLabels", func() {
+		When("namespaceLabels and namespaceAnnotations are set as targetCustomization ", func() {
+			It("deploys the bundledeployment with the merged namespaceLabels and namespaceAnnotations", func() {
+				out, err := k.Get("cluster", "local", "-o", "jsonpath={.status.namespace}")
+				Expect(err).ToNot(HaveOccurred(), out)
+
+				clusterNS := strings.TrimSpace(out)
+				clusterK := k.Namespace(clusterNS)
 				Eventually(func() string {
-					bundleDeploymentNames, _ := k.Namespace("cluster-fleet-local-local-1a3d67d0a899").Get("bundledeployments", "-o", "jsonpath={.items[*].metadata.name}")
+					bundleDeploymentNames, _ := clusterK.Get(
+						"bundledeployments",
+						"-o",
+						"jsonpath={.items[*].metadata.name}",
+					)
 
 					var bundleDeploymentName string
 					for _, podName := range strings.Split(bundleDeploymentNames, " ") {
-						if strings.HasPrefix(podName, "test-namespacelabels-targetcustomization") {
+						if strings.HasPrefix(podName, "test-target-customization-namespace-labels") {
 							bundleDeploymentName = podName
 							break
 						}
@@ -49,16 +58,25 @@ var _ = Describe("Helm deploy options", func() {
 						return "nil"
 					}
 
-					bundleDeploymentNamespacesLabels, _ := k.Namespace("cluster-fleet-local-local-1a3d67d0a899").Get("bundledeployments", bundleDeploymentName, "-o", "jsonpath={.spec.options.namespaceLabels}")
+					bundleDeploymentNamespacesLabels, _ := clusterK.Get(
+						"bundledeployments",
+						bundleDeploymentName,
+						"-o",
+						"jsonpath={.spec.options.namespaceLabels}",
+					)
 					return bundleDeploymentNamespacesLabels
 				}).Should(Equal(`{"foo":"bar","this.is/a":"test"}`))
 
 				Eventually(func() string {
-					bundleDeploymentNames, _ := k.Namespace("cluster-fleet-local-local-1a3d67d0a899").Get("bundledeployments", "-o", "jsonpath={.items[*].metadata.name}")
+					bundleDeploymentNames, _ := clusterK.Get(
+						"bundledeployments",
+						"-o",
+						"jsonpath={.items[*].metadata.name}",
+					)
 
 					var bundleDeploymentName string
 					for _, podName := range strings.Split(bundleDeploymentNames, " ") {
-						if strings.HasPrefix(podName, "test-namespacelabels-targetcustomization") {
+						if strings.HasPrefix(podName, "test-target-customization-namespace-labels") {
 							bundleDeploymentName = podName
 							break
 						}
@@ -67,7 +85,12 @@ var _ = Describe("Helm deploy options", func() {
 						return "nil"
 					}
 
-					bundleDeploymentNamespacesLabels, _ := k.Namespace("cluster-fleet-local-local-1a3d67d0a899").Get("bundledeployments", bundleDeploymentName, "-o", "jsonpath={.spec.options.namespaceAnnotations}")
+					bundleDeploymentNamespacesLabels, _ := clusterK.Get(
+						"bundledeployments",
+						bundleDeploymentName,
+						"-o",
+						"jsonpath={.spec.options.namespaceAnnotations}",
+					)
 					return bundleDeploymentNamespacesLabels
 				}).Should(Equal(`{"foo":"bar","this.is/a":"test"}`))
 

internal/cmd/agent/deployer/deployer.go

@@ -176,13 +176,13 @@ func (d *Deployer) setNamespaceLabelsAndAnnotations(ctx context.Context, bd *fle
 	}
 
 	if bd.Spec.Options.NamespaceLabels != nil {
-		addLabelsFromOptions(ns.Labels, *bd.Spec.Options.NamespaceLabels)
+		addLabelsFromOptions(ns.Labels, bd.Spec.Options.NamespaceLabels)
 	}
 	if bd.Spec.Options.NamespaceAnnotations != nil {
 		if ns.Annotations == nil {
 			ns.Annotations = map[string]string{}
 		}
-		addAnnotationsFromOptions(ns.Annotations, *bd.Spec.Options.NamespaceAnnotations)
+		addAnnotationsFromOptions(ns.Annotations, bd.Spec.Options.NamespaceAnnotations)
 	}
 	err = d.updateNamespace(ctx, ns)
 	if err != nil {

internal/cmd/agent/deployer/deployer_test.go

@@ -23,11 +23,34 @@ func TestSetNamespaceLabelsAndAnnotations(t *testing.T) {
 		release    string
 		expectedNs corev1.Namespace
 	}{
+		"Empty sets of NamespaceLabels and NamespaceAnnotations are supported": {
+			bd: &fleet.BundleDeployment{Spec: fleet.BundleDeploymentSpec{
+				Options: fleet.BundleDeploymentOptions{
+					NamespaceLabels:      nil, // equivalent to map[string]string{}
+					NamespaceAnnotations: nil,
+				},
+			}},
+			ns: corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "namespace",
+					Labels: map[string]string{"kubernetes.io/metadata.name": "namespace"},
+				},
+			},
+			release: "namespace/foo/bar",
+			expectedNs: corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "namespace",
+					Labels:      map[string]string{"kubernetes.io/metadata.name": "namespace"},
+					Annotations: nil,
+				},
+			},
+		},
+
 		"NamespaceLabels and NamespaceAnnotations are appended": {
 			bd: &fleet.BundleDeployment{Spec: fleet.BundleDeploymentSpec{
 				Options: fleet.BundleDeploymentOptions{
-					NamespaceLabels:      &map[string]string{"optLabel1": "optValue1", "optLabel2": "optValue2"},
-					NamespaceAnnotations: &map[string]string{"optAnn1": "optValue1"},
+					NamespaceLabels:      map[string]string{"optLabel1": "optValue1", "optLabel2": "optValue2"},
+					NamespaceAnnotations: map[string]string{"optAnn1": "optValue1"},
 				},
 			}},
 			ns: corev1.Namespace{
@@ -49,8 +72,8 @@ func TestSetNamespaceLabelsAndAnnotations(t *testing.T) {
 		"NamespaceLabels and NamespaceAnnotations removes entries that are not in the options, except the name label": {
 			bd: &fleet.BundleDeployment{Spec: fleet.BundleDeploymentSpec{
 				Options: fleet.BundleDeploymentOptions{
-					NamespaceLabels:      &map[string]string{"optLabel": "optValue"},
-					NamespaceAnnotations: &map[string]string{},
+					NamespaceLabels:      map[string]string{"optLabel": "optValue"},
+					NamespaceAnnotations: map[string]string{},
 				},
 			}},
 			ns: corev1.Namespace{
@@ -73,8 +96,8 @@ func TestSetNamespaceLabelsAndAnnotations(t *testing.T) {
 		"NamespaceLabels and NamespaceAnnotations updates existing values": {
 			bd: &fleet.BundleDeployment{Spec: fleet.BundleDeploymentSpec{
 				Options: fleet.BundleDeploymentOptions{
-					NamespaceLabels:      &map[string]string{"bdLabel": "labelUpdated"},
-					NamespaceAnnotations: &map[string]string{"bdAnn": "annUpdated"},
+					NamespaceLabels:      map[string]string{"bdLabel": "labelUpdated"},
+					NamespaceAnnotations: map[string]string{"bdAnn": "annUpdated"},
 				},
 			}},
 			ns: corev1.Namespace{
@@ -130,8 +153,8 @@ func TestSetNamespaceLabelsAndAnnotations(t *testing.T) {
 func TestSetNamespaceLabelsAndAnnotationsError(t *testing.T) {
 	bd := &fleet.BundleDeployment{Spec: fleet.BundleDeploymentSpec{
 		Options: fleet.BundleDeploymentOptions{
-			NamespaceLabels:      &map[string]string{"optLabel1": "optValue1", "optLabel2": "optValue2"},
-			NamespaceAnnotations: &map[string]string{"optAnn1": "optValue1"},
+			NamespaceLabels:      map[string]string{"optLabel1": "optValue1", "optLabel2": "optValue2"},
+			NamespaceAnnotations: map[string]string{"optAnn1": "optValue1"},
 		},
 	}}
 	release := "test/foo/bar"

internal/cmd/controller/target/target.go

@@ -58,22 +58,19 @@ func (t *Target) BundleDeployment() *fleet.BundleDeployment {
 		Spec: t.Deployment.Spec,
 	}
 	bd.Spec.Paused = t.IsPaused()
+
 	for _, bundleTarget := range t.Bundle.Spec.Targets {
-		if bundleTarget.NamespaceLabels != nil {
-			for key, value := range *bundleTarget.NamespaceLabels {
-				(*bd.Spec.Options.NamespaceLabels)[key] = value
-				(*bd.Spec.StagedOptions.NamespaceLabels)[key] = value
-			}
+		for key, value := range bundleTarget.NamespaceLabels {
+			bd.Spec.Options.NamespaceLabels[key] = value
+			bd.Spec.StagedOptions.NamespaceLabels[key] = value
 		}
-	}
-	for _, bundleTargets := range t.Bundle.Spec.Targets {
-		if bundleTargets.NamespaceAnnotations != nil {
-			for key, value := range *bundleTargets.NamespaceAnnotations {
-				(*bd.Spec.Options.NamespaceAnnotations)[key] = value
-				(*bd.Spec.StagedOptions.NamespaceAnnotations)[key] = value
-			}
+
+		for key, value := range bundleTarget.NamespaceAnnotations {
+			bd.Spec.Options.NamespaceAnnotations[key] = value
+			bd.Spec.StagedOptions.NamespaceAnnotations[key] = value
 		}
 	}
+
 	bd.Spec.DependsOn = t.Bundle.Spec.DependsOn
 	bd.Spec.CorrectDrift = t.Options.CorrectDrift
 	return bd

pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go

@@ -229,10 +229,10 @@ type BundleTarget struct {
 	DoNotDeploy bool `json:"doNotDeploy,omitempty"`
 	// NamespaceLabels are labels that will be appended to the namespace created by Fleet.
 	// +nullable
-	NamespaceLabels *map[string]string `json:"namespaceLabels,omitempty"`
+	NamespaceLabels map[string]string `json:"namespaceLabels,omitempty"`
 	// NamespaceAnnotations are annotations that will be appended to the namespace created by Fleet.
 	// +nullable
-	NamespaceAnnotations *map[string]string `json:"namespaceAnnotations,omitempty"`
+	NamespaceAnnotations map[string]string `json:"namespaceAnnotations,omitempty"`
 }
 
 // BundleSummary contains the number of bundle deployments in each state and a

pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go

@@ -103,11 +103,11 @@ type BundleDeploymentOptions struct {
 
 	// NamespaceLabels are labels that will be appended to the namespace created by Fleet.
 	// +nullable
-	NamespaceLabels *map[string]string `json:"namespaceLabels,omitempty"`
+	NamespaceLabels map[string]string `json:"namespaceLabels,omitempty"`
 
 	// NamespaceAnnotations are annotations that will be appended to the namespace created by Fleet.
 	// +nullable
-	NamespaceAnnotations *map[string]string `json:"namespaceAnnotations,omitempty"`
+	NamespaceAnnotations map[string]string `json:"namespaceAnnotations,omitempty"`
 
 	// DeleteCRDResources deletes CRDs. Warning! this will also delete all your Custom Resources.
 	DeleteCRDResources bool `json:"deleteCRDResources,omitempty"`