Merge pull request #1049 from fluxcd/prevent-flux-overrides

Prevent Flux from overriding Flagger managed objects

Author: stefanprodan

pkg/router/kubernetes_default.go

@@ -150,7 +150,7 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam
 				Name:        name,
 				Namespace:   canary.Namespace,
 				Labels:      metadata.Labels,
-				Annotations: metadata.Annotations,
+				Annotations: filterMetadata(metadata.Annotations),
 				OwnerReferences: []metav1.OwnerReference{
 					*metav1.NewControllerRef(canary, schema.GroupVersionKind{
 						Group:   flaggerv1.SchemeGroupVersion.Group,
@@ -217,6 +217,10 @@ func (c *KubernetesDefaultRouter) reconcileService(canary *flaggerv1.Canary, nam
 		}
 
 		if updateService {
+			if svcClone.ObjectMeta.Annotations == nil {
+				svcClone.ObjectMeta.Annotations = make(map[string]string)
+			}
+			svcClone.ObjectMeta.Annotations = filterMetadata(svcClone.ObjectMeta.Annotations)
 			_, err = c.kubeClient.CoreV1().Services(canary.Namespace).Update(context.TODO(), svcClone, metav1.UpdateOptions{})
 			if err != nil {
 				return fmt.Errorf("service %s update error: %w", name, err)

pkg/router/kubernetes_default_test.go

@@ -391,7 +391,7 @@ func TestServiceRouter_InitializeMetadata(t *testing.T) {
 
 	primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
 	require.NoError(t, err)
-	assert.Equal(t, 0, len(primarySvc.Annotations))
+	assert.Equal(t, 1, len(primarySvc.Annotations))
 	assert.Equal(t, "podinfo-primary", primarySvc.Labels["app"])
 }
 
@@ -423,12 +423,12 @@ func TestServiceRouter_ReconcileMetadata(t *testing.T) {
 
 	canarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo-canary", metav1.GetOptions{})
 	require.NoError(t, err)
-	assert.Equal(t, 0, len(canarySvc.Annotations))
+	assert.Equal(t, 1, len(canarySvc.Annotations))
 	assert.Equal(t, "podinfo-canary", canarySvc.Labels["app"])
 
 	primarySvc, err := mocks.kubeClient.CoreV1().Services("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
 	require.NoError(t, err)
-	assert.Equal(t, 0, len(primarySvc.Annotations))
+	assert.Equal(t, 1, len(primarySvc.Annotations))
 	assert.Equal(t, "podinfo-primary", primarySvc.Labels["app"])
 
 	mocks.canary.Spec.Service.Apex = &flaggerv1.CustomMetadata{

pkg/router/traefik_test.go

@@ -52,7 +52,7 @@ func TestTraefikRouter_Reconcile(t *testing.T) {
 	assert.Equal(t, uint(100), services[0].Weight)
 
 	assert.Equal(t, ts.ObjectMeta.Labels, mocks.canary.Spec.Service.Apex.Labels)
-	assert.Equal(t, ts.ObjectMeta.Annotations, mocks.canary.Spec.Service.Apex.Annotations)
+	assert.Equal(t, ts.ObjectMeta.Annotations, filterMetadata(mocks.canary.Spec.Service.Apex.Annotations))
 
 	for _, tt := range []struct {
 		name        string

pkg/router/util.go

@@ -4,7 +4,11 @@ import (
 	"strings"
 )
 
-const toolkitMarker = "toolkit.fluxcd.io"
+const (
+	toolkitMarker         = "toolkit.fluxcd.io"
+	toolkitReconcileKey   = "kustomize.toolkit.fluxcd.io/reconcile"
+	toolkitReconcileValue = "disabled"
+)
 
 func includeLabelsByPrefix(labels map[string]string, includeLabelPrefixes []string) map[string]string {
 	filteredLabels := make(map[string]string)
@@ -26,10 +30,14 @@ func includeLabelsByPrefix(labels map[string]string, includeLabelPrefixes []stri
 func filterMetadata(meta map[string]string) map[string]string {
 	res := make(map[string]string)
 	for k, v := range meta {
+		// remove Flux ownership
 		if strings.Contains(k, toolkitMarker) {
 			continue
 		}
 		res[k] = v
 	}
+
+	// prevent Flux from overriding Flagger managed objects
+	res[toolkitReconcileKey] = toolkitReconcileValue
 	return res
 }

test/kubernetes/test-deployment.sh

@@ -8,6 +8,21 @@ set -o errexit
 REPO_ROOT=$(git rev-parse --show-toplevel)
 
 cat <<EOF | kubectl apply -f -
+apiVersion: v1
+kind: Service
+metadata:
+  name: podinfo-svc
+  namespace: test
+spec:
+  type: ClusterIP
+  selector:
+    app: podinfo
+  ports:
+    - name: http
+      port: 9898
+      protocol: TCP
+      targetPort: http
+---
 apiVersion: flagger.app/v1beta1
 kind: Canary
 metadata:
@@ -72,6 +87,12 @@ until ${ok}; do
     fi
 done
 
+passed=$(kubectl -n test get svc/podinfo-svc -oyaml 2>&1 | { grep 'kustomize.toolkit.fluxcd.io/reconcile' || true; })
+if [ -z "$passed" ]; then
+  echo -e '\u2716 toolkit annotation test failed'
+  kubectl -n test get svc/podinfo-svc -oyaml
+  exit 1
+fi
 passed=$(kubectl -n test get deploy/podinfo-primary -oyaml 2>&1 | { grep test-label-prefix || true; })
 if [ -z "$passed" ]; then
   echo -e '\u2716 primary copy labels by prefix test failed'