Bug 1954152: manifests: populate aws user tags in infrastructure

data/data/install.openshift.io_installconfigs.yaml

@@ -948,6 +948,12 @@ spec:
                           type: string
                         type: array
                     type: object
+                  experimentalPropagateUserTags:
+                    description: ExperimentalPropagateUserTags is an experimental
+                      flag that directs in-cluster operators to include the specified
+                      user tags in the tags of the AWS resources that the operators
+                      create.
+                    type: boolean
                   region:
                     description: Region specifies the AWS region where the cluster
                       will be created.

go.mod

@@ -29,7 +29,7 @@ require (
 	github.com/go-playground/validator/v10 v10.2.0
 	github.com/gobuffalo/flect v0.2.2 // indirect
 	github.com/golang/mock v1.4.4
-	github.com/golang/protobuf v1.4.2
+	github.com/golang/protobuf v1.4.3
 	github.com/google/martian v2.1.1-0.20190517191504-25dcb96d9e51+incompatible // indirect
 	github.com/google/uuid v1.1.2
 	github.com/gophercloud/gophercloud v0.12.1-0.20200827191144-bb4781e9de45
@@ -89,24 +89,22 @@ require (
 	github.com/vincent-petithory/dataurl v0.0.0-20191104211930-d1553a71de50
 	github.com/vmware/govmomi v0.22.2
 	github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca // indirect
-	golang.org/dl v0.0.0-20210204224843-1557c60ec592 // indirect
 	golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad
 	golang.org/x/lint v0.0.0-20200302205851-738671d3881b
 	golang.org/x/mod v0.4.0 // indirect
 	golang.org/x/net v0.0.0-20201202161906-c7110b5ffcbb // indirect
 	golang.org/x/oauth2 v0.0.0-20200902213428-5d25da1a8d43
 	golang.org/x/sys v0.0.0-20201202213521-69691e467435
-	golang.org/x/text v0.3.4 // indirect
 	golang.org/x/tools v0.0.0-20201202200335-bef1c476418a // indirect
 	google.golang.org/api v0.33.0
 	google.golang.org/genproto v0.0.0-20200911024640-645f7a48b24f
 	google.golang.org/grpc v1.32.0
 	gopkg.in/AlecAivazis/survey.v1 v1.8.9-0.20200217094205-6773bdf39b7f
 	gopkg.in/ini.v1 v1.61.0
 	gopkg.in/yaml.v2 v2.4.0
-	k8s.io/api v0.19.4
+	k8s.io/api v0.20.0
 	k8s.io/apiextensions-apiserver v0.19.4
-	k8s.io/apimachinery v0.19.4
+	k8s.io/apimachinery v0.20.0
 	k8s.io/client-go v12.0.0+incompatible
 	k8s.io/klog v1.0.0
 	k8s.io/klog/v2 v2.4.0
@@ -117,7 +115,6 @@ require (
 	sigs.k8s.io/cluster-api-provider-azure v0.0.0
 	sigs.k8s.io/cluster-api-provider-openstack v0.0.0
 	sigs.k8s.io/controller-tools v0.4.1
-	sigs.k8s.io/structured-merge-diff/v4 v4.0.2 // indirect
 )
 
 replace (
@@ -131,7 +128,7 @@ replace (
 	github.com/kubevirt/terraform-provider-kubevirt => github.com/nirarg/terraform-provider-kubevirt v0.0.0-20201222125919-101cee051ed3
 	github.com/metal3-io/baremetal-operator => github.com/openshift/baremetal-operator v0.0.0-20200715132148-0f91f62a41fe // Use OpenShift fork
 	github.com/metal3-io/cluster-api-provider-baremetal => github.com/openshift/cluster-api-provider-baremetal v0.0.0-20190821174549-a2a477909c1d // Pin OpenShift fork
-	github.com/openshift/api => github.com/openshift/api v0.0.0-20201020123828-1e2e1963a41f // Pin API
+	github.com/openshift/api => github.com/openshift/api v0.0.0-20210428205234-a8389931bee7
 	github.com/openshift/client-go => github.com/openshift/client-go v0.0.0-20200929181438-91d71ef2122c // Pin client-go
 	github.com/openshift/machine-config-operator => github.com/openshift/machine-config-operator v0.0.1-0.20201009041932-4fe8559913b8 // Pin MCO so it doesn't get downgraded
 	github.com/terraform-providers/terraform-provider-aws => github.com/openshift/terraform-provider-aws v1.60.1-0.20200630224953-76d1fb4e5699 // Pin to openshift fork with tag v2.67.0-openshift
@@ -140,6 +137,7 @@ replace (
 	github.com/terraform-providers/terraform-provider-vsphere => github.com/openshift/terraform-provider-vsphere v1.18.1-openshift-2
 	github.com/vmware/govmomi => github.com/vmware/govmomi v0.22.2-0.20200420222347-5fceac570f29
 	k8s.io/api => k8s.io/api v0.19.0
+	k8s.io/apimachinery => k8s.io/apimachinery v0.19.0
 	k8s.io/client-go => k8s.io/client-go v0.19.0
 	kubevirt.io/client-go => kubevirt.io/client-go v0.29.0
 	sigs.k8s.io/cluster-api-provider-aws => github.com/openshift/cluster-api-provider-aws v0.2.1-0.20201022175424-d30c7a274820

pkg/asset/manifests/infrastructure.go

@@ -83,9 +83,17 @@ func (i *Infrastructure) Generate(dependencies asset.Parents) error {
 	switch installConfig.Config.Platform.Name() {
 	case aws.Name:
 		config.Spec.PlatformSpec.Type = configv1.AWSPlatformType
-		config.Spec.PlatformSpec.AWS = &configv1.AWSPlatformSpec{}
+
+		var resourceTags []configv1.AWSResourceTag
+		if installConfig.Config.AWS.ExperimentalPropagateUserTag {
+			resourceTags = make([]configv1.AWSResourceTag, 0, len(installConfig.Config.AWS.UserTags))
+			for k, v := range installConfig.Config.AWS.UserTags {
+				resourceTags = append(resourceTags, configv1.AWSResourceTag{Key: k, Value: v})
+			}
+		}
 		config.Status.PlatformStatus.AWS = &configv1.AWSPlatformStatus{
-			Region: installConfig.Config.Platform.AWS.Region,
+			Region:       installConfig.Config.Platform.AWS.Region,
+			ResourceTags: resourceTags,
 		}
 
 		for _, service := range installConfig.Config.Platform.AWS.ServiceEndpoints {

pkg/explain/printer_test.go

@@ -119,6 +119,9 @@ func Test_PrintFields(t *testing.T) {
     defaultMachinePlatform <object>
       DefaultMachinePlatform is the default configuration used when installing on AWS for machine pools which do not define their own platform configuration.
 
+    experimentalPropagateUserTags <boolean>
+      ExperimentalPropagateUserTags is an experimental flag that directs in-cluster operators to include the specified user tags in the tags of the AWS resources that the operators create.
+
     region <string> -required-
       Region specifies the AWS region where the cluster will be created.
 

pkg/types/aws/platform.go

@@ -43,6 +43,12 @@ type Platform struct {
 	// platform configuration.
 	// +optional
 	DefaultMachinePlatform *MachinePool `json:"defaultMachinePlatform,omitempty"`
+
+	// ExperimentalPropagateUserTags is an experimental flag that directs in-cluster
+	// operators to include the specified user tags in the tags of the AWS resources
+	// that the operators create.
+	// +optional
+	ExperimentalPropagateUserTag bool `json:"experimentalPropagateUserTags,omitempty"`
 }
 
 // ServiceEndpoint store the configuration for services to

pkg/types/aws/validation/platform.go

@@ -4,14 +4,22 @@ import (
 	"fmt"
 	"net/url"
 	"regexp"
-	"sort"
 	"strings"
 
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	"github.com/openshift/installer/pkg/types/aws"
 )
 
+// tagRegex is used to check that the keys and values of a tag contain only valid characters.
+var tagRegex = regexp.MustCompile(`^[0-9A-Za-z_.:/=+-@]*$`)
+
+// kubernetesNamespaceRegex is used to check that a tag key is not in the kubernetes.io namespace.
+var kubernetesNamespaceRegex = regexp.MustCompile(`^([^/]*\.)?kubernetes.io/`)
+
+// openshiftNamespaceRegex is used to check that a tag key is not in the openshift.io namespace.
+var openshiftNamespaceRegex = regexp.MustCompile(`^([^/]*\.)?openshift.io/`)
+
 // ValidatePlatform checks that the specified platform is valid.
 func ValidatePlatform(p *aws.Platform, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
@@ -21,35 +29,72 @@ func ValidatePlatform(p *aws.Platform, fldPath *field.Path) field.ErrorList {
 	}
 
 	allErrs = append(allErrs, validateServiceEndpoints(p.ServiceEndpoints, fldPath.Child("serviceEndpoints"))...)
-	allErrs = append(allErrs, validateUserTags(p.UserTags, fldPath.Child("userTags"))...)
+	allErrs = append(allErrs, validateUserTags(p.UserTags, p.ExperimentalPropagateUserTag, fldPath.Child("userTags"))...)
 
 	if p.DefaultMachinePlatform != nil {
 		allErrs = append(allErrs, ValidateMachinePool(p, p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...)
 	}
 	return allErrs
 }
 
-func validateUserTags(tags map[string]string, fldPath *field.Path) field.ErrorList {
+func validateUserTags(tags map[string]string, propagatingTags bool, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 	if len(tags) == 0 {
 		return allErrs
 	}
-	keys := make([]string, 0, len(tags))
-	for k := range tags {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-	for _, key := range keys {
+	for key, value := range tags {
 		if strings.EqualFold(key, "Name") {
 			allErrs = append(allErrs, field.Invalid(fldPath.Key(key), tags[key], "Name key is not allowed for user defined tags"))
 		}
-		if strings.HasPrefix(key, "kubernetes.io/cluster/") {
-			allErrs = append(allErrs, field.Invalid(fldPath.Key(key), tags[key], "Keys with prefix 'kubernetes.io/cluster/' are not allowed for user defined tags"))
+		if propagatingTags {
+			if err := validateTag(key, value); err != nil {
+				allErrs = append(allErrs, field.Invalid(fldPath.Key(key), value, err.Error()))
+			}
+		} else {
+			if strings.HasPrefix(key, "kubernetes.io/cluster/") {
+				allErrs = append(allErrs, field.Invalid(fldPath.Key(key), tags[key], "Keys with prefix 'kubernetes.io/cluster/' are not allowed for user defined tags"))
+			}
 		}
 	}
 	return allErrs
 }
 
+// validateTag checks the following things to ensure that the tag is acceptable as an additional tag.
+// * The key and value contain only valid characters.
+// * The key is not empty and at most 128 characters.
+// * The value is not empty and at most 256 characters. Note that, while many AWS services accept empty tag values,
+//   the additional tags may be applied to resources in services that do not accept empty tag values. Consequently,
+//   OpenShift cannot accept empty tag values.
+// * The key is not in the kubernetes.io namespace.
+// * The key is not in the openshift.io namespace.
+func validateTag(key, value string) error {
+	if !tagRegex.MatchString(key) {
+		return fmt.Errorf("key contains invalid characters")
+	}
+	if !tagRegex.MatchString(value) {
+		return fmt.Errorf("value contains invalid characters")
+	}
+	if len(key) == 0 {
+		return fmt.Errorf("key is empty")
+	}
+	if len(key) > 128 {
+		return fmt.Errorf("key is too long")
+	}
+	if len(value) == 0 {
+		return fmt.Errorf("value is empty")
+	}
+	if len(value) > 256 {
+		return fmt.Errorf("value is too long")
+	}
+	if kubernetesNamespaceRegex.MatchString(key) {
+		return fmt.Errorf("key is in the kubernetes.io namespace")
+	}
+	if openshiftNamespaceRegex.MatchString(key) {
+		return fmt.Errorf("key is in the openshift.io namespace")
+	}
+	return nil
+}
+
 func validateServiceEndpoints(endpoints []aws.ServiceEndpoint, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 	tracker := map[string]int{}

pkg/types/aws/validation/platform_test.go

@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -158,3 +159,79 @@ func TestValidatePlatform(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateTag(t *testing.T) {
+	cases := []struct {
+		name      string
+		key       string
+		value     string
+		expectErr bool
+	}{{
+		name:  "valid",
+		key:   "test-key",
+		value: "test-value",
+	}, {
+		name:      "invalid characters in key",
+		key:       "bad-key***",
+		value:     "test-value",
+		expectErr: true,
+	}, {
+		name:      "invalid characters in value",
+		key:       "test-key",
+		value:     "bad-value***",
+		expectErr: true,
+	}, {
+		name:      "empty key",
+		key:       "",
+		value:     "test-value",
+		expectErr: true,
+	}, {
+		name:      "empty value",
+		key:       "test-key",
+		value:     "",
+		expectErr: true,
+	}, {
+		name:      "key too long",
+		key:       strings.Repeat("a", 129),
+		value:     "test-value",
+		expectErr: true,
+	}, {
+		name:      "value too long",
+		key:       "test-key",
+		value:     strings.Repeat("a", 257),
+		expectErr: true,
+	}, {
+		name:      "key in kubernetes.io namespace",
+		key:       "kubernetes.io/cluster/some-cluster",
+		value:     "owned",
+		expectErr: true,
+	}, {
+		name:      "key in openshift.io namespace",
+		key:       "openshift.io/some-key",
+		value:     "some-value",
+		expectErr: true,
+	}, {
+		name:      "key in openshift.io subdomain namespace",
+		key:       "other.openshift.io/some-key",
+		value:     "some-value",
+		expectErr: true,
+	}, {
+		name:  "key in namespace similar to openshift.io",
+		key:   "otheropenshift.io/some-key",
+		value: "some-value",
+	}, {
+		name:  "key with openshift.io in path",
+		key:   "some-domain/openshift.io/some-key",
+		value: "some-value",
+	}}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := validateTag(tc.key, tc.value)
+			if tc.expectErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}