fix: allow prefixes in tags and taints with noprefix (#50)

Signed-off-by: Patrik Cyvoct <[email protected]>

Author: Sh4d1

docs/tags.md

@@ -13,18 +13,29 @@ Once the tag is removed from the instance, it will also be removed as a label on
 
 ### Non prefixed labels
 
-It is possible to add labels nop prefixed with `k8s.scaleway.com`. The downside, is that when you will delete the associated tag, the label won't get removed.
+It is possible to add labels not prefixed with `k8s.scaleway.com`. The downside, is that when you will delete the associated tag, the label won't get removed.
 In order to have non prefixed labels, you should prefix the tag with `noprefix=`.
 
 For intance the tag `noprefix=foo=bar` will yield the `foo=bar` label on the Kubernetes nodes.
 
+This is the only way to add custom prefixed labels like `node.kubernetes.io`.
+
 ## Taints
 
 In order for a tag to be synced to a taint, it needs to be of the form `taint=foo=bar:Effect`, where `Effect` is one of `NoExecute`, `NoSchedule` or `PreferNoSchedule`.
 In this case, the Kubernetes nodes will have the taint `k8s.scaleway.com/foo=bar` with the effect `Effect`.
 
 Once the tag is removed from the instance, it will also be removed as a taint on the node.
 
+### Non prefixed Tains
+
+It is possible to add taints not prefixed with `k8s.scaleway.com`. The downside, is that when you will delete the associated tag, the taint won't get removed.
+In order to have non prefixed taints, you should prefix the taint with `taint=noprefix=`.
+
+For intance the tag `taint=noprefix=foo=bar:Effect` will yield the `foo=bar` taint on the Kubernetes nodes with the `Effect` effect.
+
+This is the only way to add custom prefixed taints like `node.kubernetes.io`.
+
 ## Special Kubernetes Labels
 
 - `node.kubernetes.io/exclude-from-external-load-balancers` can be set on the Kubernetes nodes if this same value is set as a tag on the instance. It will have the value `managed-by-scaleway-ccm` and will be deleted if deleted from the tags.

scaleway/sync.go

@@ -28,6 +28,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
@@ -53,13 +54,20 @@ const (
 	exponentialMaxRetries = 30
 )
 
+// from k8s.io/apimachinery/pkg/util/validation/validation.go
+const qnameCharFmt string = "[A-Za-z0-9]"
+const qnameExtCharFmt string = "[-A-Za-z0-9_.]"
+const qualifiedNameFmt string = "(" + qnameCharFmt + qnameExtCharFmt + "*)?" + qnameCharFmt
+const qualifiedNameErrMsg string = "must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character"
+const qualifiedNameMaxLength int = 63
+
 var (
-	splitRegexp      = regexp.MustCompile(`[:= ]+`)
-	labelsRegexp     = regexp.MustCompile(`^(([A-Za-z0-9][-A-Za-z0-9_\\.]*)?[A-Za-z0-9])?$`)
-	labelsPrefix     = "k8s.scaleway.com/"
-	taintsPrefix     = "k8s.scaleway.com/"
-	labelTaintPrefix = "taint="
-	labelNoPrefix    = "noprefix="
+	splitRegexp         = regexp.MustCompile(`[:= ]+`)
+	qualifiedNameRegexp = regexp.MustCompile("^" + qualifiedNameFmt + "$")
+	labelsPrefix        = "k8s.scaleway.com/"
+	taintsPrefix        = "k8s.scaleway.com/"
+	labelTaintPrefix    = "taint="
+	labelNoPrefix       = "noprefix="
 
 	// K8S labels
 	labelNodeRoleExcludeBalancer      = "node.kubernetes.io/exclude-from-external-load-balancers"
@@ -335,22 +343,50 @@ func (s *syncController) SyncLBTags() {
 
 func tagLabelParser(tag string) (key string, value string) {
 	prefix := labelsPrefix
+	tagValue := tag
 
 	if strings.HasPrefix(tag, labelNoPrefix) {
 		prefix = ""
-		tag = strings.TrimPrefix(tag, labelNoPrefix)
+		tagValue = strings.TrimPrefix(tag, labelNoPrefix)
+		tagSplit := strings.Split(tagValue, "/")
+
+		if len(tagSplit) > 2 {
+			klog.Errorf("tag %s is not valid: %s contains more than 1 '/'", tag, tagValue)
+			return "", ""
+		}
+		if len(tagSplit) == 2 {
+			if tagSplit[0] == "" {
+				klog.Errorf("tag %s is not valid: prefix is empty")
+				return "", ""
+			}
+			if errs := validation.IsDNS1123Subdomain(tagSplit[0]); len(errs) != 0 {
+				klog.Errorf("tag %s is not valid: prefix is not composed of DNS labels: %s", tag, strings.Join(errs, ","))
+				return "", ""
+			}
+			prefix = tagSplit[0] + "/"
+			tagValue = tagSplit[1]
+		}
 	}
 
-	split := splitRegexp.Split(tag, -1)
+	split := splitRegexp.Split(tagValue, -1)
 
-	key = prefix + labelsRegexp.FindString(split[0])
-	if key == prefix || len(key) > 63 {
+	if len(split[0]) == 0 {
+		klog.Errorf("tag %s have an empty key", tag)
 		return "", ""
 	}
-
+	if len(split[0]) > qualifiedNameMaxLength {
+		klog.Errorf("tag %s have a key too long, got %d instead of max %d", tag, len(split[0]), qualifiedNameMaxLength)
+		return "", ""
+	}
+	if !qualifiedNameRegexp.MatchString(split[0]) {
+		klog.Errorf("tag %s key must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character", tag)
+		return "", ""
+	}
+	key = prefix + split[0]
 	if len(split) > 1 {
-		value = labelsRegexp.FindString(split[1])
-		if value == "" || len(value) > 63 {
+		value = split[1]
+		if errs := validation.IsValidLabelValue(value); len(errs) != 0 {
+			klog.Errorf("tag %s value must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character", tag)
 			return "", ""
 		}
 	}

scaleway/sync_test.go

@@ -35,10 +35,12 @@ func TestTagTaintParser(t *testing.T) {
 	tagsTest := map[string][3]string{
 		"taint=":                    {"", "", ""},
 		"taint=word=word:NoExecute": {"k8s.scaleway.com/word", "word", "NoExecute"},
-		"taint=underscore_word=underscore_word:NoSchedule": {"k8s.scaleway.com/underscore_word", "underscore_word", "NoSchedule"},
-		"taint=dash-word=dash-word:PreferNoSchedule":       {"k8s.scaleway.com/dash-word", "dash-word", "PreferNoSchedule"},
-		"taint=dash-word=dash-word:foo":                    {"", "", ""},
-		"taint=dash-word=dash-word":                        {"", "", ""},
+		"taint=underscore_word=underscore_word:NoSchedule":                {"k8s.scaleway.com/underscore_word", "underscore_word", "NoSchedule"},
+		"taint=dash-word=dash-word:PreferNoSchedule":                      {"k8s.scaleway.com/dash-word", "dash-word", "PreferNoSchedule"},
+		"taint=dash-word=dash-word:foo":                                   {"", "", ""},
+		"taint=dash-word=dash-word":                                       {"", "", ""},
+		"taint=noprefix=dash-word=dash-word:PreferNoSchedule":             {"dash-word", "dash-word", "PreferNoSchedule"},
+		"taint=noprefix=node.k8s.io/dash-word=dash-word:PreferNoSchedule": {"node.k8s.io/dash-word", "dash-word", "PreferNoSchedule"},
 	}
 	for tag, expected := range tagsTest {
 		t.Run(tag, func(t *testing.T) {
@@ -59,6 +61,7 @@ func TestTagLabelParser(t *testing.T) {
 		"dash-word":             {"k8s.scaleway.com/dash-word", ""},
 		"equal1=value":          {"k8s.scaleway.com/equal1", "value"},
 		"noprefix=equal1=value": {"equal1", "value"},
+		"noprefix=node.kubernetes.io/role=myrole": {"node.kubernetes.io/role", "myrole"},
 		"equal2 = value":        {"k8s.scaleway.com/equal2", "value"},
 		"equal3 =value":         {"k8s.scaleway.com/equal3", "value"},
 		"equal4= value":         {"k8s.scaleway.com/equal4", "value"},