Support both new fluxcd and original annotations

The new prefix for annotations is `fluxcd.io`; but, many files will use `flux.weave.works`, and we don't want to suddenly not understand the policies in those. This commit adds support for interpreting and updating policies as annotations with either prefix. When updating, if an annotation uses the old prefix, that will be retained; otherwise, the new prefix is used.
fluxcd · Jul 3, 2019 · 4d5d195 · 4d5d195
1 parent 91793bc
commit 4d5d195
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 31 deletions.
diff --git a/cluster/kubernetes/policies.go b/cluster/kubernetes/policies.go
@@ -3,8 +3,6 @@ package kubernetes
 import (
 	"fmt"
 
-	"github.com/pkg/errors"
-
 	kresource "github.com/weaveworks/flux/cluster/kubernetes/resource"
 	"github.com/weaveworks/flux/resource"
 )
@@ -16,11 +14,16 @@ func (m *manifests) UpdateWorkloadPolicies(def []byte, id resource.ID, update re
 	}
 	res, ok := resources[id.String()]
 	if !ok {
-		return nil, errors.New("resource " + id.String() + " not found")
+		return nil, fmt.Errorf("resource %s not found", id.String())
 	}
+
+	// This is the Kubernetes manifests implementation; panic if it's
+	// not returning `KubeManifest`s.
+	kres := res.(kresource.KubeManifest)
+
 	workload, ok := res.(resource.Workload)
 	if !ok {
-		return nil, errors.New("resource " + id.String() + " does not have containers")
+		return nil, fmt.Errorf("resource %s does not have containers", id.String())
 	}
 	if err != nil {
 		return nil, err
@@ -33,7 +36,11 @@ func (m *manifests) UpdateWorkloadPolicies(def []byte, id resource.ID, update re
 
 	var args []string
 	for k, v := range changes {
-		args = append(args, fmt.Sprintf("%s%s=%s", kresource.PolicyPrefix, k, v))
+		annotation, ok := kres.PolicyAnnotationKey(k)
+		if !ok {
+			annotation = fmt.Sprintf("%s%s", kresource.PolicyPrefix, k)
+		}
+		args = append(args, fmt.Sprintf("%s=%s", annotation, v))
 	}
 
 	ns, kind, name := id.Components()

diff --git a/cluster/kubernetes/policies_test.go b/cluster/kubernetes/policies_test.go
@@ -23,13 +23,13 @@ func TestUpdatePolicies(t *testing.T) {
 		{
 			name: "adding annotation with others existing",
 			in:   []string{"prometheus.io.scrape", "'false'"},
-			out:  []string{"prometheus.io.scrape", "'false'", "flux.weave.works/automated", "'true'"},
+			out:  []string{"prometheus.io.scrape", "'false'", "fluxcd.io/automated", "'true'"},
 			update: resource.PolicyUpdate{
 				Add: policy.Set{policy.Automated: "true"},
 			},
 		},
 		{
-			name: "adding annotation when already has annotation",
+			name: "adding annotation when already has annotation does not change prefix",
 			in:   []string{"flux.weave.works/automated", "'true'"},
 			out:  []string{"flux.weave.works/automated", "'true'"},
 			update: resource.PolicyUpdate{
@@ -45,17 +45,17 @@ func TestUpdatePolicies(t *testing.T) {
 			},
 		},
 		{
-			name: "adding first annotation",
+			name: "adding first annotation (uses new prefix)",
 			in:   nil,
-			out:  []string{"flux.weave.works/automated", "'true'"},
+			out:  []string{"fluxcd.io/automated", "'true'"},
 			update: resource.PolicyUpdate{
 				Add: policy.Set{policy.Automated: "true"},
 			},
 		},
 		{
 			name: "add and remove different annotations at the same time",
 			in:   []string{"flux.weave.works/automated", "'true'", "prometheus.io.scrape", "'false'"},
-			out:  []string{"prometheus.io.scrape", "'false'", "flux.weave.works/locked", "'true'"},
+			out:  []string{"prometheus.io.scrape", "'false'", "fluxcd.io/locked", "'true'"},
 			update: resource.PolicyUpdate{
 				Add:    policy.Set{policy.Locked: "true"},
 				Remove: policy.Set{policy.Automated: "true"},
@@ -72,15 +72,15 @@ func TestUpdatePolicies(t *testing.T) {
 		},
 		{
 			name: "remove annotation with others existing",
-			in:   []string{"flux.weave.works/automated", "true", "prometheus.io.scrape", "false"},
+			in:   []string{"fluxcd.io/automated", "true", "prometheus.io.scrape", "false"},
 			out:  []string{"prometheus.io.scrape", "false"},
 			update: resource.PolicyUpdate{
 				Remove: policy.Set{policy.Automated: "true"},
 			},
 		},
 		{
 			name: "remove last annotation",
-			in:   []string{"flux.weave.works/automated", "true"},
+			in:   []string{"fluxcd.io/automated", "true"},
 			out:  nil,
 			update: resource.PolicyUpdate{
 				Remove: policy.Set{policy.Automated: "true"},
@@ -104,15 +104,15 @@ func TestUpdatePolicies(t *testing.T) {
 		},
 		{
 			name: "multiline",
-			in:   []string{"flux.weave.works/locked_msg", "|-\n      first\n      second"},
+			in:   []string{"fluxcd.io/locked_msg", "|-\n      first\n      second"},
 			out:  nil,
 			update: resource.PolicyUpdate{
 				Remove: policy.Set{policy.LockedMsg: "foo"},
 			},
 		},
 		{
 			name: "multiline with empty line",
-			in:   []string{"flux.weave.works/locked_msg", "|-\n      first\n\n      third"},
+			in:   []string{"fluxcd.io/locked_msg", "|-\n      first\n\n      third"},
 			out:  nil,
 			update: resource.PolicyUpdate{
 				Remove: policy.Set{policy.LockedMsg: "foo"},
@@ -121,31 +121,31 @@ func TestUpdatePolicies(t *testing.T) {
 		{
 			name: "add tag policy",
 			in:   nil,
-			out:  []string{"flux.weave.works/tag.nginx", "glob:*"},
+			out:  []string{"fluxcd.io/tag.nginx", "glob:*"},
 			update: resource.PolicyUpdate{
 				Add: policy.Set{policy.TagPrefix("nginx"): "glob:*"},
 			},
 		},
 		{
 			name: "add non-glob tag policy",
 			in:   nil,
-			out:  []string{"flux.weave.works/tag.nginx", "foo"},
+			out:  []string{"fluxcd.io/tag.nginx", "foo"},
 			update: resource.PolicyUpdate{
 				Add: policy.Set{policy.TagPrefix("nginx"): "foo"},
 			},
 		},
 		{
 			name: "add semver tag policy",
 			in:   nil,
-			out:  []string{"flux.weave.works/tag.nginx", "semver:*"},
+			out:  []string{"fluxcd.io/tag.nginx", "semver:*"},
 			update: resource.PolicyUpdate{
 				Add: policy.Set{policy.TagPrefix("nginx"): "semver:*"},
 			},
 		},
 		{
 			name: "add invalid semver tag policy",
 			in:   nil,
-			out:  []string{"flux.weave.works/tag.nginx", "semver:*"},
+			out:  []string{"fluxcd.io/tag.nginx", "semver:*"},
 			update: resource.PolicyUpdate{
 				Add: policy.Set{policy.TagPrefix("nginx"): "semver:invalid"},
 			},
@@ -154,15 +154,15 @@ func TestUpdatePolicies(t *testing.T) {
 		{
 			name: "add regexp tag policy",
 			in:   nil,
-			out:  []string{"flux.weave.works/tag.nginx", "regexp:(.*?)"},
+			out:  []string{"fluxcd.io/tag.nginx", "regexp:(.*?)"},
 			update: resource.PolicyUpdate{
 				Add: policy.Set{policy.TagPrefix("nginx"): "regexp:(.*?)"},
 			},
 		},
 		{
 			name: "add invalid regexp tag policy",
 			in:   nil,
-			out:  []string{"flux.weave.works/tag.nginx", "regexp:(.*?)"},
+			out:  []string{"fluxcd.io/tag.nginx", "regexp:(.*?)"},
 			update: resource.PolicyUpdate{
 				Add: policy.Set{policy.TagPrefix("nginx"): "regexp:*"},
 			},
@@ -171,7 +171,7 @@ func TestUpdatePolicies(t *testing.T) {
 		{
 			name: "set tag to all containers",
 			in:   nil,
-			out:  []string{"flux.weave.works/tag.nginx", "semver:*"},
+			out:  []string{"fluxcd.io/tag.nginx", "semver:*"},
 			update: resource.PolicyUpdate{
 				Add: policy.Set{policy.TagAll: "semver:*"},
 			},

diff --git a/cluster/kubernetes/resource/resource.go b/cluster/kubernetes/resource/resource.go
@@ -11,8 +11,12 @@ import (
 )
 
 const (
-	PolicyPrefix = "flux.weave.works/"
-	ClusterScope = "<cluster>"
+	PolicyPrefix = "fluxcd.io/"
+	// This is the previously-used prefix for annotations; many
+	// manifests in the wild will still be using it, so it's included
+	// here for backward-compatibility.
+	AlternatePolicyPrefix = "flux.weave.works/"
+	ClusterScope          = "<cluster>"
 )
 
 // KubeManifest represents a manifest for a Kubernetes resource. For
@@ -25,6 +29,8 @@ type KubeManifest interface {
 	GetName() string
 	GetNamespace() string
 	SetNamespace(string)
+
+	PolicyAnnotationKey(string) (string, bool)
 }
 
 // -- unmarshaling code for specific object and field types
@@ -87,13 +93,20 @@ func (o *baseObject) debyte() {
 func PoliciesFromAnnotations(annotations map[string]string) policy.Set {
 	set := policy.Set{}
 	for k, v := range annotations {
-		if strings.HasPrefix(k, PolicyPrefix) {
-			p := strings.TrimPrefix(k, PolicyPrefix)
-			if v == "true" {
-				set = set.Add(policy.Policy(p))
-			} else {
-				set = set.Set(policy.Policy(p), v)
-			}
+		var p string
+		switch {
+		case strings.HasPrefix(k, PolicyPrefix):
+			p = strings.TrimPrefix(k, PolicyPrefix)
+		case strings.HasPrefix(k, AlternatePolicyPrefix):
+			p = strings.TrimPrefix(k, AlternatePolicyPrefix)
+		default:
+			continue
+		}
+
+		if v == "true" {
+			set = set.Add(policy.Policy(p))
+		} else {
+			set = set.Set(policy.Policy(p), v)
 		}
 	}
 	return set
@@ -103,6 +116,20 @@ func (o baseObject) Policies() policy.Set {
 	return PoliciesFromAnnotations(o.Meta.Annotations)
 }
 
+// PolicyAnnotationKey returns the key used in this resource to
+// indicate a particular policy; this is to aid in supporting more
+// than one way of using annotations for policy. If the policy is not
+// present, returns `"", false`.
+func (o baseObject) PolicyAnnotationKey(p string) (string, bool) {
+	for _, prefix := range []string{PolicyPrefix, AlternatePolicyPrefix} {
+		key := prefix + p
+		if _, ok := o.Meta.Annotations[key]; ok {
+			return key, true
+		}
+	}
+	return "", false
+}
+
 func (o baseObject) Source() string {
 	return o.source
 }

diff --git a/manifests/configaware_test.go b/manifests/configaware_test.go
@@ -207,7 +207,7 @@ apiVersion: extensions/v1beta1
 kind: Deployment
 metadata:
   annotations:
-    flux.weave.works/tag.greeter: glob:master-*
+    fluxcd.io/tag.greeter: glob:master-*
   name: helloworld
 spec:
   template: