Repository: fluxcd/flagger
Pull request #939 (2021-06-17 14:50:29)

Remove the GitOps Toolkit metadata from generated objects
Fix: #928
Followup: #936


Patch:
diff --git a/pkg/canary/daemonset_controller.go b/pkg/canary/daemonset_controller.go
index 89494ea3..f2f1a192 100644
--- a/pkg/canary/daemonset_controller.go
+++ b/pkg/canary/daemonset_controller.go
@@ -262,7 +262,7 @@ func (c *DaemonSetController) createPrimaryDaemonSet(cd *flaggerv1.Canary, inclu
 				Name:        primaryName,
 				Namespace:   cd.Namespace,
 				Labels:      makePrimaryLabels(labels, primaryLabelValue, label),
-				Annotations: canaryDae.Annotations,
+				Annotations: filterMetadata(canaryDae.Annotations),
 				OwnerReferences: []metav1.OwnerReference{
 					*metav1.NewControllerRef(cd, schema.GroupVersionKind{
 						Group:   flaggerv1.SchemeGroupVersion.Group,
diff --git a/pkg/canary/deployment_controller.go b/pkg/canary/deployment_controller.go
index c78a53f7..a4e03d62 100644
--- a/pkg/canary/deployment_controller.go
+++ b/pkg/canary/deployment_controller.go
@@ -263,7 +263,7 @@ func (c *DeploymentController) createPrimaryDeployment(cd *flaggerv1.Canary, inc
 				Name:        primaryName,
 				Namespace:   cd.Namespace,
 				Labels:      makePrimaryLabels(labels, primaryLabelValue, label),
-				Annotations: canaryDep.Annotations,
+				Annotations: filterMetadata(canaryDep.Annotations),
 				OwnerReferences: []metav1.OwnerReference{
 					*metav1.NewControllerRef(cd, schema.GroupVersionKind{
 						Group:   flaggerv1.SchemeGroupVersion.Group,
@@ -335,7 +335,7 @@ func (c *DeploymentController) reconcilePrimaryHpa(cd *flaggerv1.Canary, init bo
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      primaryHpaName,
 				Namespace: cd.Namespace,
-				Labels:    hpa.Labels,
+				Labels:    filterMetadata(hpa.Labels),
 				OwnerReferences: []metav1.OwnerReference{
 					*metav1.NewControllerRef(cd, schema.GroupVersionKind{
 						Group:   flaggerv1.SchemeGroupVersion.Group,
diff --git a/pkg/canary/util.go b/pkg/canary/util.go
index 8ebc0615..07bb50c2 100644
--- a/pkg/canary/util.go
+++ b/pkg/canary/util.go
@@ -69,6 +69,8 @@ func getPorts(cd *flaggerv1.Canary, cs []corev1.Container) map[string]int32 {
 	return ports
 }
 
+const toolkitMarker = "toolkit.fluxcd.io"
+
 // makeAnnotations appends an unique ID to annotations map
 func makeAnnotations(annotations map[string]string) (map[string]string, error) {
 	idKey := "flagger-id"
@@ -83,8 +85,7 @@ func makeAnnotations(annotations map[string]string) (map[string]string, error) {
 	id := fmt.Sprintf("%x-%x-%x-%x-%x", uuid[0:4], uuid[4:6], uuid[6:8], uuid[8:10], uuid[10:])
 
 	for k, v := range annotations {
-		// skip Flux GC markers
-		if strings.Contains(k, "/checksum") {
+		if strings.Contains(k, toolkitMarker) {
 			continue
 		}
 		if k != idKey {
@@ -96,9 +97,23 @@ func makeAnnotations(annotations map[string]string) (map[string]string, error) {
 	return res, nil
 }
 
+func filterMetadata(meta map[string]string) map[string]string {
+	res := make(map[string]string)
+	for k, v := range meta {
+		if strings.Contains(k, toolkitMarker) {
+			continue
+		}
+		res[k] = v
+	}
+	return res
+}
+
 func includeLabelsByPrefix(labels map[string]string, includeLabelPrefixes []string) map[string]string {
 	filteredLabels := make(map[string]string)
 	for key, value := range labels {
+		if strings.Contains(key, toolkitMarker) {
+			continue
+		}
 		for _, includeLabelPrefix := range includeLabelPrefixes {
 			if includeLabelPrefix == "*" || strings.HasPrefix(key, includeLabelPrefix) {
 				filteredLabels[key] = value
@@ -113,6 +128,9 @@ func includeLabelsByPrefix(labels map[string]string, includeLabelPrefixes []stri
 func makePrimaryLabels(labels map[string]string, labelValue string, label string) map[string]string {
 	res := make(map[string]string)
 	for k, v := range labels {
+		if strings.Contains(k, toolkitMarker) {
+			continue
+		}
 		if k != label {
 			res[k] = v
 		}
diff --git a/pkg/router/ingress.go b/pkg/router/ingress.go
index 2f605d8b..4890eea4 100644
--- a/pkg/router/ingress.go
+++ b/pkg/router/ingress.go
@@ -211,7 +211,7 @@ func (i *IngressRouter) SetRoutes(
 
 func (i *IngressRouter) makeAnnotations(annotations map[string]string) map[string]string {
 	res := make(map[string]string)
-	for k, v := range makeAnnotations(annotations) {
+	for k, v := range filterMetadata(annotations) {
 		if !strings.Contains(k, i.GetAnnotationWithPrefix("canary")) &&
 			!strings.Contains(k, "kubectl.kubernetes.io/last-applied-configuration") &&
 			!strings.Contains(k, i.GetAnnotationWithPrefix("canary-weight")) &&
@@ -232,7 +232,7 @@ func (i *IngressRouter) makeAnnotations(annotations map[string]string) map[strin
 func (i *IngressRouter) makeHeaderAnnotations(annotations map[string]string,
 	header string, headerValue string, headerRegex string, cookie string) map[string]string {
 	res := make(map[string]string)
-	for k, v := range makeAnnotations(annotations) {
+	for k, v := range filterMetadata(annotations) {
 		if !strings.Contains(v, i.GetAnnotationWithPrefix("canary")) {
 			res[k] = v
 		}
diff --git a/pkg/router/istio.go b/pkg/router/istio.go
index 6794b28a..dd5ae7d6 100644
--- a/pkg/router/istio.go
+++ b/pkg/router/istio.go
@@ -260,7 +260,7 @@ func (ir *IstioRouter) reconcileVirtualService(canary *flaggerv1.Canary) error {
 				if vtClone.ObjectMeta.Annotations == nil {
 					vtClone.ObjectMeta.Annotations = make(map[string]string)
 				} else {
-					vtClone.ObjectMeta.Annotations = makeAnnotations(vtClone.ObjectMeta.Annotations)
+					vtClone.ObjectMeta.Annotations = filterMetadata(vtClone.ObjectMeta.Annotations)
 				}
 
 				vtClone.ObjectMeta.Annotations[configAnnotation] = string(b)
diff --git a/pkg/router/kubernetes_default.go b/pkg/router/kubernetes_default.go
index 3f79bf2b..ad0b63a5 100644
--- a/pkg/router/kubernetes_default.go
+++ b/pkg/router/kubernetes_default.go
@@ -206,8 +206,8 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam
 			if svc.ObjectMeta.Annotations == nil {
 				svc.ObjectMeta.Annotations = make(map[string]string)
 			}
-			if diff := cmp.Diff(makeAnnotations(metadata.Annotations), svc.ObjectMeta.Annotations); diff != "" {
-				svcClone.ObjectMeta.Annotations = makeAnnotations(metadata.Annotations)
+			if diff := cmp.Diff(filterMetadata(metadata.Annotations), svc.ObjectMeta.Annotations); diff != "" {
+				svcClone.ObjectMeta.Annotations = filterMetadata(metadata.Annotations)
 				updateService = true
 			}
 			if diff := cmp.Diff(metadata.Labels, svc.ObjectMeta.Labels); diff != "" {
diff --git a/pkg/router/skipper.go b/pkg/router/skipper.go
index 50d06bf5..29c45989 100644
--- a/pkg/router/skipper.go
+++ b/pkg/router/skipper.go
@@ -129,7 +129,7 @@ func (skp *SkipperRouter) Reconcile(canary *flaggerv1.Canary) error {
 	if cmp.Diff(iClone.Spec, canaryIngress.Spec) != "" {
 		ingressClone := canaryIngress.DeepCopy()
 		ingressClone.Spec = iClone.Spec
-		ingressClone.Annotations = makeAnnotations(iClone.Annotations)
+		ingressClone.Annotations = filterMetadata(iClone.Annotations)
 
 		_, err := skp.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Update(context.TODO(), ingressClone, metav1.UpdateOptions{})
 		if err != nil {
diff --git a/pkg/router/traefik.go b/pkg/router/traefik.go
index ab3a79cd..af7c69bb 100644
--- a/pkg/router/traefik.go
+++ b/pkg/router/traefik.go
@@ -72,7 +72,7 @@ func (tr *TraefikRouter) Reconcile(canary *flaggerv1.Canary) error {
 				Name:        apexName,
 				Namespace:   canary.Namespace,
 				Labels:      tsMetadata.Labels,
-				Annotations: makeAnnotations(tsMetadata.Annotations),
+				Annotations: filterMetadata(tsMetadata.Annotations),
 				OwnerReferences: []metav1.OwnerReference{
 					*metav1.NewControllerRef(canary, schema.GroupVersionKind{
 						Group:   flaggerv1.SchemeGroupVersion.Group,
diff --git a/pkg/router/util.go b/pkg/router/util.go
index d6748968..48e0d1ae 100644
--- a/pkg/router/util.go
+++ b/pkg/router/util.go
@@ -4,9 +4,14 @@ import (
 	"strings"
 )
 
+const toolkitMarker = "toolkit.fluxcd.io"
+
 func includeLabelsByPrefix(labels map[string]string, includeLabelPrefixes []string) map[string]string {
 	filteredLabels := make(map[string]string)
 	for key, value := range labels {
+		if strings.Contains(key, toolkitMarker) {
+			continue
+		}
 		for _, includeLabelPrefix := range includeLabelPrefixes {
 			if includeLabelPrefix == "*" || strings.HasPrefix(key, includeLabelPrefix) {
 				filteredLabels[key] = value
@@ -18,14 +23,13 @@ func includeLabelsByPrefix(labels map[string]string, includeLabelPrefixes []stri
 	return filteredLabels
 }
 
-func makeAnnotations(in map[string]string) map[string]string {
-	out := make(map[string]string)
-	for key, value := range in {
-		// skip Flux GC markers
-		if strings.Contains(key, "/checksum") {
+func filterMetadata(meta map[string]string) map[string]string {
+	res := make(map[string]string)
+	for k, v := range meta {
+		if strings.Contains(k, toolkitMarker) {
 			continue
 		}
-		out[key] = value
+		res[k] = v
 	}
-	return out
+	return res
 }


Test patch:
diff --git a/pkg/canary/daemonset_controller_test.go b/pkg/canary/daemonset_controller_test.go
index e5e8effd..f27ce684 100644
--- a/pkg/canary/daemonset_controller_test.go
+++ b/pkg/canary/daemonset_controller_test.go
@@ -48,6 +48,9 @@ func TestDaemonSetController_Sync_ConsistentNaming(t *testing.T) {
 	primarySelectorValue := daePrimary.Spec.Selector.MatchLabels[dc.label]
 	sourceSelectorValue := dae.Spec.Selector.MatchLabels[dc.label]
 	assert.Equal(t, primarySelectorValue, fmt.Sprintf("%s-primary", sourceSelectorValue))
+
+	annotation := daePrimary.Annotations["kustomize.toolkit.fluxcd.io/checksum"]
+	assert.Equal(t, "", annotation)
 }
 
 func TestDaemonSetController_Sync_InconsistentNaming(t *testing.T) {
diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go
index e516df2e..f8af8cf2 100644
--- a/pkg/canary/deployment_controller_test.go
+++ b/pkg/canary/deployment_controller_test.go
@@ -48,6 +48,9 @@ func TestDeploymentController_Sync_ConsistentNaming(t *testing.T) {
 	primarySelectorValue := depPrimary.Spec.Selector.MatchLabels[dc.label]
 	assert.Equal(t, primarySelectorValue, fmt.Sprintf("%s-primary", dc.labelValue))
 
+	annotation := depPrimary.Annotations["kustomize.toolkit.fluxcd.io/checksum"]
+	assert.Equal(t, "", annotation)
+
 	hpaPrimary, err := mocks.kubeClient.AutoscalingV2beta2().HorizontalPodAutoscalers("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
 	require.NoError(t, err)
 	assert.Equal(t, depPrimary.Name, hpaPrimary.Spec.ScaleTargetRef.Name)
diff --git a/pkg/canary/deployment_fixture_test.go b/pkg/canary/deployment_fixture_test.go
index 45c317c5..718b76d3 100644
--- a/pkg/canary/deployment_fixture_test.go
+++ b/pkg/canary/deployment_fixture_test.go
@@ -419,6 +419,9 @@ func newDeploymentControllerTest(dc deploymentConfigs) *appsv1.Deployment {
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: "default",
 			Name:      dc.name,
+			Annotations: map[string]string{
+				"kustomize.toolkit.fluxcd.io/checksum": "0a40893bfdc545d62125bd3e74eeb2ebaa7097c2",
+			},
 		},
 		Spec: appsv1.DeploymentSpec{
 			Selector: &metav1.LabelSelector{
diff --git a/pkg/router/ingress_test.go b/pkg/router/ingress_test.go
index 53c2df50..803595be 100644
--- a/pkg/router/ingress_test.go
+++ b/pkg/router/ingress_test.go
@@ -48,6 +48,9 @@ func TestIngressRouter_Reconcile(t *testing.T) {
 	inCanary, err := router.kubeClient.NetworkingV1().Ingresses("default").Get(context.TODO(), canaryName, metav1.GetOptions{})
 	require.NoError(t, err)
 
+	annotation := inCanary.Annotations["kustomize.toolkit.fluxcd.io/checksum"]
+	assert.Equal(t, "", annotation)
+
 	// test initialisation
 	assert.Equal(t, "true", inCanary.Annotations[canaryAn])
 	assert.Equal(t, "0", inCanary.Annotations[canaryWeightAn])
@@ -181,6 +184,6 @@ func TestIngressRouter_ABTest(t *testing.T) {
 		// test initialisation
 		assert.Equal(t, "true", inCanary.Annotations[canaryAn])
 		assert.Equal(t, "test", inCanary.Annotations[table.annotation])
+		assert.Equal(t, "", inCanary.Annotations["kustomize.toolkit.fluxcd.io/checksum"])
 	}
-
 }
diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go
index ee3cb24f..a215c82d 100644
--- a/pkg/router/router_test.go
+++ b/pkg/router/router_test.go
@@ -406,6 +406,9 @@ func newTestCanaryIngress() *flaggerv1.Canary {
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: "default",
 			Name:      "nginx",
+			Annotations: map[string]string{
+				"kustomize.toolkit.fluxcd.io/checksum": "0a40893bfdc545d62125bd3e74eeb2ebaa7097c2",
+			},
 		},
 		Spec: flaggerv1.CanarySpec{
 			TargetRef: flaggerv1.CrossNamespaceObjectReference{
@@ -444,7 +447,8 @@ func newTestIngress() *netv1.Ingress {
 			Namespace: "default",
 			Name:      "podinfo",
 			Annotations: map[string]string{
-				"kubernetes.io/ingress.class": "nginx",
+				"kubernetes.io/ingress.class":          "nginx",
+				"kustomize.toolkit.fluxcd.io/checksum": "0a40893bfdc545d62125bd3e74eeb2ebaa7097c2",
 			},
 		},
 		Spec: netv1.IngressSpec{
diff --git a/pkg/router/util_test.go b/pkg/router/util_test.go
index 2069810e..7f44436b 100644
--- a/pkg/router/util_test.go
+++ b/pkg/router/util_test.go
@@ -41,9 +41,10 @@ func TestIncludeLabelsByPrefix(t *testing.T) {
 
 func TestIncludeLabelsByPrefixWithWildcard(t *testing.T) {
 	labels := map[string]string{
-		"foo":   "foo-value",
-		"bar":   "bar-value",
-		"lorem": "ipsum",
+		"foo":                                  "foo-value",
+		"bar":                                  "bar-value",
+		"lorem":                                "ipsum",
+		"kustomize.toolkit.fluxcd.io/checksum": "some",
 	}
 	includeLabelPrefix := []string{"*"}