From 77d81fee4d32a55090d2a5344f6b5d0d07baea5e Mon Sep 17 00:00:00 2001 From: staebler Date: Tue, 20 Apr 2021 21:40:37 -0400 Subject: [PATCH 1/2] vendor: bump openshift/api for resourceTags in infrastructure Bump to the latest github.com/openshift/api to pull in the resourceTags fields added to infrastructure.config.openshift.io. --- go.mod | 2 + go.sum | 6 +- ...config-operator_01_infrastructure.crd.yaml | 62 +++++++++++++++++++ .../api/config/v1/types_infrastructure.go | 36 +++++++++++ .../api/config/v1/zz_generated.deepcopy.go | 26 ++++++++ .../v1/zz_generated.swagger_doc_generated.go | 12 ++++ vendor/modules.txt | 2 +- 7 files changed, 141 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index ac54c29d3..4f8119ccb 100644 --- a/go.mod +++ b/go.mod @@ -26,3 +26,5 @@ require ( // points to temporary-watch-reduction-patch-1.21 to pick up k/k/pull/101102 - please remove it once the pr merges and a new Z release is cut replace k8s.io/apiserver => github.com/openshift/kubernetes-apiserver v0.0.0-20210419140141-620426e63a99 + +replace github.com/openshift/api => github.com/staebler/api v0.0.0-20210421013259-487a3ae5573d diff --git a/go.sum b/go.sum index 1049341b8..095904db3 100644 --- a/go.sum +++ b/go.sum @@ -416,10 +416,6 @@ github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1Cpa github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= github.com/opencontainers/runc v0.0.0-20191031171055-b133feaeeb2e/go.mod h1:qT5XzbpPznkRYVz/mWwUaVBUv2rmF59PVA73FjuZG0U= -github.com/openshift/api v0.0.0-20201214114959-164a2fb63b5f/go.mod h1:aqU5Cq+kqKKPbDMqxo9FojgDeSpNJI7iuskjXjtojDg= -github.com/openshift/api v0.0.0-20210105115604-44119421ec6b/go.mod h1:aqU5Cq+kqKKPbDMqxo9FojgDeSpNJI7iuskjXjtojDg= -github.com/openshift/api v0.0.0-20210415092137-8c78458f83d9 h1:TudJ23vtDe4zyFep9xdw1baNkZ6AyEGSkrzUzws9C0c= -github.com/openshift/api v0.0.0-20210415092137-8c78458f83d9/go.mod h1:dZ4kytOo3svxJHNYd0J55hwe/6IQG5gAUHUE0F3Jkio= github.com/openshift/build-machinery-go v0.0.0-20200917070002-f171684f77ab/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE= github.com/openshift/build-machinery-go v0.0.0-20210209125900-0da259a2c359 h1:ehSDsWQiUVzJZrSEXMC7ceV9JIPEyTYqrpqu3m4Wa08= github.com/openshift/build-machinery-go v0.0.0-20210209125900-0da259a2c359/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE= @@ -507,6 +503,8 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s= github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg= +github.com/staebler/api v0.0.0-20210421013259-487a3ae5573d h1:O7vGIbmd+jDXsyIt5zY4szBgScDK12EG1l+dyYlLA/U= +github.com/staebler/api v0.0.0-20210421013259-487a3ae5573d/go.mod h1:dZ4kytOo3svxJHNYd0J55hwe/6IQG5gAUHUE0F3Jkio= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= diff --git a/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure.crd.yaml b/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure.crd.yaml index 212c1e21f..1f5d0847a 100644 --- a/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure.crd.yaml +++ b/vendor/github.com/openshift/api/config/v1/0000_10_config-operator_01_infrastructure.crd.yaml @@ -76,6 +76,37 @@ spec: Services infrastructure provider. type: object properties: + resourceTags: + description: resourceTags is a list of additional tags to + apply to AWS resources created for the cluster. See https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html + for information on tagging AWS resources. AWS supports a + maximum of 50 tags per resource. OpenShift reserves 25 tags + for its use, leaving 25 tags available for the user. + type: array + maxItems: 25 + items: + description: AWSResourceTag is a tag to apply to AWS resources + created for the cluster. + type: object + required: + - key + - value + properties: + key: + description: key is the key of the tag + type: string + maxLength: 128 + minLength: 1 + pattern: ^[0-9A-Za-z_.:/=+-@]+$ + value: + description: value is the value of the tag. Some AWS + service do not support empty values. Since tags are + added to resources in many services, the length of + the tag value must meet the requirements of all services. + type: string + maxLength: 256 + minLength: 1 + pattern: ^[0-9A-Za-z_.:/=+-@]+$ serviceEndpoints: description: serviceEndpoints list contains custom endpoints which will override default service endpoint of AWS Services. @@ -248,6 +279,37 @@ spec: description: region holds the default AWS region for new AWS resources created by the cluster. type: string + resourceTags: + description: resourceTags is a list of additional tags to + apply to AWS resources created for the cluster. See https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html + for information on tagging AWS resources. AWS supports a + maximum of 50 tags per resource. OpenShift reserves 25 tags + for its use, leaving 25 tags available for the user. + type: array + maxItems: 25 + items: + description: AWSResourceTag is a tag to apply to AWS resources + created for the cluster. + type: object + required: + - key + - value + properties: + key: + description: key is the key of the tag + type: string + maxLength: 128 + minLength: 1 + pattern: ^[0-9A-Za-z_.:/=+-@]+$ + value: + description: value is the value of the tag. Some AWS + service do not support empty values. Since tags are + added to resources in many services, the length of + the tag value must meet the requirements of all services. + type: string + maxLength: 256 + minLength: 1 + pattern: ^[0-9A-Za-z_.:/=+-@]+$ serviceEndpoints: description: ServiceEndpoints list contains custom endpoints which will override default service endpoint of AWS Services. diff --git a/vendor/github.com/openshift/api/config/v1/types_infrastructure.go b/vendor/github.com/openshift/api/config/v1/types_infrastructure.go index d5ebcc91c..a843d5cf3 100644 --- a/vendor/github.com/openshift/api/config/v1/types_infrastructure.go +++ b/vendor/github.com/openshift/api/config/v1/types_infrastructure.go @@ -301,6 +301,14 @@ type AWSPlatformSpec struct { // There must be only one ServiceEndpoint for a service. // +optional ServiceEndpoints []AWSServiceEndpoint `json:"serviceEndpoints,omitempty"` + + // resourceTags is a list of additional tags to apply to AWS resources created for the cluster. + // See https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html for information on tagging AWS resources. + // AWS supports a maximum of 50 tags per resource. OpenShift reserves 25 tags for its use, leaving 25 tags + // available for the user. + // +kubebuilder:validation:MaxItems=25 + // +optional + ResourceTags []AWSResourceTag `json:"resourceTags,omitempty"` } // AWSPlatformStatus holds the current status of the Amazon Web Services infrastructure provider. @@ -313,6 +321,34 @@ type AWSPlatformStatus struct { // There must be only one ServiceEndpoint for a service. // +optional ServiceEndpoints []AWSServiceEndpoint `json:"serviceEndpoints,omitempty"` + + // resourceTags is a list of additional tags to apply to AWS resources created for the cluster. + // See https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html for information on tagging AWS resources. + // AWS supports a maximum of 50 tags per resource. OpenShift reserves 25 tags for its use, leaving 25 tags + // available for the user. + // +kubebuilder:validation:MaxItems=25 + // +optional + ResourceTags []AWSResourceTag `json:"resourceTags,omitempty"` +} + +// AWSResourceTag is a tag to apply to AWS resources created for the cluster. +type AWSResourceTag struct { + // key is the key of the tag + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=128 + // +kubebuilder:validation:Pattern=`^[0-9A-Za-z_.:/=+-@]+$` + // +required + Key string `json:"key"` + // value is the value of the tag. + // Some AWS service do not support empty values. Since tags are added to resources in many services, the + // length of the tag value must meet the requirements of all services. + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=256 + // +kubebuilder:validation:Pattern=`^[0-9A-Za-z_.:/=+-@]+$` + // +required + Value string `json:"value"` } // AzurePlatformSpec holds the desired state of the Azure infrastructure provider. diff --git a/vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.go b/vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.go index e6012e04e..fc71794cd 100644 --- a/vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.go +++ b/vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.go @@ -186,6 +186,11 @@ func (in *AWSPlatformSpec) DeepCopyInto(out *AWSPlatformSpec) { *out = make([]AWSServiceEndpoint, len(*in)) copy(*out, *in) } + if in.ResourceTags != nil { + in, out := &in.ResourceTags, &out.ResourceTags + *out = make([]AWSResourceTag, len(*in)) + copy(*out, *in) + } return } @@ -207,6 +212,11 @@ func (in *AWSPlatformStatus) DeepCopyInto(out *AWSPlatformStatus) { *out = make([]AWSServiceEndpoint, len(*in)) copy(*out, *in) } + if in.ResourceTags != nil { + in, out := &in.ResourceTags, &out.ResourceTags + *out = make([]AWSResourceTag, len(*in)) + copy(*out, *in) + } return } @@ -220,6 +230,22 @@ func (in *AWSPlatformStatus) DeepCopy() *AWSPlatformStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AWSResourceTag) DeepCopyInto(out *AWSResourceTag) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AWSResourceTag. +func (in *AWSResourceTag) DeepCopy() *AWSResourceTag { + if in == nil { + return nil + } + out := new(AWSResourceTag) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AWSServiceEndpoint) DeepCopyInto(out *AWSServiceEndpoint) { *out = *in diff --git a/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go b/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go index 6cc78bc37..2c6f9db13 100644 --- a/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go +++ b/vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go @@ -715,6 +715,7 @@ func (RegistrySources) SwaggerDoc() map[string]string { var map_AWSPlatformSpec = map[string]string{ "": "AWSPlatformSpec holds the desired state of the Amazon Web Services infrastructure provider. This only includes fields that can be modified in the cluster.", "serviceEndpoints": "serviceEndpoints list contains custom endpoints which will override default service endpoint of AWS Services. There must be only one ServiceEndpoint for a service.", + "resourceTags": "resourceTags is a list of additional tags to apply to AWS resources created for the cluster. See https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html for information on tagging AWS resources. AWS supports a maximum of 50 tags per resource. OpenShift reserves 25 tags for its use, leaving 25 tags available for the user.", } func (AWSPlatformSpec) SwaggerDoc() map[string]string { @@ -725,12 +726,23 @@ var map_AWSPlatformStatus = map[string]string{ "": "AWSPlatformStatus holds the current status of the Amazon Web Services infrastructure provider.", "region": "region holds the default AWS region for new AWS resources created by the cluster.", "serviceEndpoints": "ServiceEndpoints list contains custom endpoints which will override default service endpoint of AWS Services. There must be only one ServiceEndpoint for a service.", + "resourceTags": "resourceTags is a list of additional tags to apply to AWS resources created for the cluster. See https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html for information on tagging AWS resources. AWS supports a maximum of 50 tags per resource. OpenShift reserves 25 tags for its use, leaving 25 tags available for the user.", } func (AWSPlatformStatus) SwaggerDoc() map[string]string { return map_AWSPlatformStatus } +var map_AWSResourceTag = map[string]string{ + "": "AWSResourceTag is a tag to apply to AWS resources created for the cluster.", + "key": "key is the key of the tag", + "value": "value is the value of the tag. Some AWS service do not support empty values. Since tags are added to resources in many services, the length of the tag value must meet the requirements of all services.", +} + +func (AWSResourceTag) SwaggerDoc() map[string]string { + return map_AWSResourceTag +} + var map_AWSServiceEndpoint = map[string]string{ "": "AWSServiceEndpoint store the configuration of a custom url to override existing defaults of AWS Services.", "name": "name is the name of the AWS service. The list of all the service names can be found at https://docs.aws.amazon.com/general/latest/gr/aws-service-information.html This must be provided and cannot be empty.", diff --git a/vendor/modules.txt b/vendor/modules.txt index 2e41ef053..c278061c8 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -92,7 +92,7 @@ github.com/modern-go/concurrent github.com/modern-go/reflect2 # github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 github.com/munnerz/goautoneg -# github.com/openshift/api v0.0.0-20210415092137-8c78458f83d9 +# github.com/openshift/api v0.0.0-20210415092137-8c78458f83d9 => github.com/staebler/api v0.0.0-20210421013259-487a3ae5573d github.com/openshift/api github.com/openshift/api/apiserver github.com/openshift/api/apiserver/v1 From 7417f249f42310040615f5ade959edf172985d6a Mon Sep 17 00:00:00 2001 From: staebler Date: Tue, 20 Apr 2021 21:37:51 -0400 Subject: [PATCH 2/2] add controller to sync aws resource tags Add aws_resource_tags controller. The controller will sync AWS resource tags from the spec of the infrastructure resource to the status of the resource. --- pkg/operator/aws_resource_tags/OWNERS | 9 + pkg/operator/aws_resource_tags/controller.go | 149 +++++++++ .../aws_resource_tags/controller_test.go | 287 ++++++++++++++++++ pkg/operator/starter.go | 10 + 4 files changed, 455 insertions(+) create mode 100644 pkg/operator/aws_resource_tags/OWNERS create mode 100644 pkg/operator/aws_resource_tags/controller.go create mode 100644 pkg/operator/aws_resource_tags/controller_test.go diff --git a/pkg/operator/aws_resource_tags/OWNERS b/pkg/operator/aws_resource_tags/OWNERS new file mode 100644 index 000000000..735dc632a --- /dev/null +++ b/pkg/operator/aws_resource_tags/OWNERS @@ -0,0 +1,9 @@ +reviewers: +- e-tienne +- jhixson74 +- jstuever +- mtnbikenc +- patrickdillon +- rna-afk +- staebler +approvers: diff --git a/pkg/operator/aws_resource_tags/controller.go b/pkg/operator/aws_resource_tags/controller.go new file mode 100644 index 000000000..933e5bd1a --- /dev/null +++ b/pkg/operator/aws_resource_tags/controller.go @@ -0,0 +1,149 @@ +package aws_resource_tags + +import ( + "context" + "fmt" + "regexp" + "time" + + configv1 "github.com/openshift/api/config/v1" + configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + operatorv1helpers "github.com/openshift/library-go/pkg/operator/v1helpers" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/cache" +) + +// 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/`) + +// AWSResourceTagsController syncs the AWS resource tags from the spec of the `infrastructure.config.openshift.io/v1` +// `cluster` object to the status. +type AWSResourceTagsController struct { + infraClient configv1client.InfrastructureInterface + infraLister configv1listers.InfrastructureLister +} + +// NewController returns a AWSResourceTagsController +func NewController(operatorClient operatorv1helpers.OperatorClient, + infraClient configv1client.InfrastructuresGetter, infraLister configv1listers.InfrastructureLister, infraInformer cache.SharedIndexInformer, + recorder events.Recorder) factory.Controller { + c := &AWSResourceTagsController{ + infraClient: infraClient.Infrastructures(), + infraLister: infraLister, + } + return factory.New(). + WithInformers( + operatorClient.Informer(), + infraInformer, + ). + WithSync(c.sync). + WithSyncDegradedOnError(operatorClient). + ResyncEvery(time.Minute). + ToController("AWSResourceTagsController", recorder) +} + +func (c AWSResourceTagsController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + obji, err := c.infraLister.Get("cluster") + if errors.IsNotFound(err) { + syncCtx.Recorder().Warningf("AWSResourceTagsController", "Required infrastructures.%s/cluster not found", configv1.GroupName) + return nil + } + if err != nil { + return err + } + + currentInfra := obji.DeepCopy() + + var desiredResourceTags []configv1.AWSResourceTag + if awsSpec := currentInfra.Spec.PlatformSpec.AWS; awsSpec != nil { + keys := sets.NewString() + desiredResourceTags = make([]configv1.AWSResourceTag, 0, len(awsSpec.ResourceTags)) + for _, tag := range awsSpec.ResourceTags { + if err := validateTag(tag); err != nil { + syncCtx.Recorder().Warningf("AWSResourceTagsController", "The resource tag with key=%q and value=%q is invalid: %v", tag.Key, tag.Value, err) + continue + } + if keys.Has(tag.Key) { + syncCtx.Recorder().Warningf("AWSResourceTagsController", "The resource tag with key=%q and value=%q is a duplicate", tag.Key, tag.Value) + continue + } + keys.Insert(tag.Key) + desiredResourceTags = append(desiredResourceTags, tag) + } + } + + var currentResourceTags []configv1.AWSResourceTag + if currentInfra.Status.PlatformStatus != nil && + currentInfra.Status.PlatformStatus.AWS != nil { + currentResourceTags = currentInfra.Status.PlatformStatus.AWS.ResourceTags + } + + if len(desiredResourceTags) == 0 && len(currentResourceTags) == 0 { + return nil + } + + if equality.Semantic.DeepEqual(desiredResourceTags, currentResourceTags) { + return nil + } + + if currentInfra.Status.PlatformStatus == nil { + currentInfra.Status.PlatformStatus = &configv1.PlatformStatus{} + } + if currentInfra.Status.PlatformStatus.AWS == nil { + currentInfra.Status.PlatformStatus.AWS = &configv1.AWSPlatformStatus{} + } + currentInfra.Status.PlatformStatus.AWS.ResourceTags = desiredResourceTags + + _, err = c.infraClient.UpdateStatus(ctx, currentInfra, metav1.UpdateOptions{}) + if err != nil { + syncCtx.Recorder().Warningf("AWSResourceTagsController", "Unable to update the infrastructure status") + return err + } + return nil +} + +// 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(tag configv1.AWSResourceTag) error { + if !tagRegex.MatchString(tag.Key) { + return fmt.Errorf("key contains invalid characters") + } + if !tagRegex.MatchString(tag.Value) { + return fmt.Errorf("value contains invalid characters") + } + if len(tag.Key) == 0 { + return fmt.Errorf("key is empty") + } + if len(tag.Key) > 128 { + return fmt.Errorf("key is too long") + } + if len(tag.Value) == 0 { + return fmt.Errorf("value is empty") + } + if len(tag.Value) > 256 { + return fmt.Errorf("value is too long") + } + if kubernetesNamespaceRegex.MatchString(tag.Key) { + return fmt.Errorf("key is in the kubernetes.io namespace") + } + if openshiftNamespaceRegex.MatchString(tag.Key) { + return fmt.Errorf("key is in the openshift.io namespace") + } + return nil +} diff --git a/pkg/operator/aws_resource_tags/controller_test.go b/pkg/operator/aws_resource_tags/controller_test.go new file mode 100644 index 000000000..0624ccd0e --- /dev/null +++ b/pkg/operator/aws_resource_tags/controller_test.go @@ -0,0 +1,287 @@ +package aws_resource_tags + +import ( + "context" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ktesting "k8s.io/client-go/testing" + "k8s.io/client-go/tools/cache" + + configv1 "github.com/openshift/api/config/v1" + configfakeclient "github.com/openshift/client-go/config/clientset/versioned/fake" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" +) + +func Test_sync(t *testing.T) { + cases := []struct { + name string + obj *configv1.Infrastructure + expectedActions int + expectedTags []configv1.AWSResourceTag + expectedErr string + }{{ + name: "empty infrastructure", + obj: buildInfra(), + }, { + name: "other platform", + obj: buildInfra(withGCPSpec(), withGCPStatus()), + }, { + name: "no spec tags, no platform status", + obj: buildInfra(withAWSSpec()), + }, { + name: "no spec tags, no aws status", + obj: buildInfra(withAWSSpec(), withPlatformStatus()), + }, { + name: "no spec tags, no status tags", + obj: buildInfra(withAWSSpec(), withAWSStatus()), + }, { + name: "no spec tags, non-empty status tags", + obj: buildInfra(withAWSSpec(), withStatusTag("test-key", "test-value")), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{}, + }, { + name: "in-sync resource tag", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withStatusTag("test-key", "test-value"), + ), + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "changed resource tag", + obj: buildInfra( + withSpecTag("test-key", "new-value"), + withStatusTag("test-key", "orig-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "new-value"}}, + }, { + name: "added resource tag", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("other-key", "other-value"), + withStatusTag("test-key", "test-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{ + {Key: "test-key", Value: "test-value"}, + {Key: "other-key", Value: "other-value"}, + }, + }, { + name: "removed resource tag", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withStatusTag("test-key", "test-value"), + withStatusTag("other-key", "other-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with invalid characters in key rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("bad-key***", "other-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with invalid characters in value rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("other-key", "bad-value***"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with missing key rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("", "other-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with missing value rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("other-key", ""), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with too-long key rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag(strings.Repeat("k", 129), "other-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with too-long value rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("other-key", strings.Repeat("v", 257)), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with key in kubernetes.io namespace rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("kubernetes.io/cluster/some-cluster", "owned"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with key in openshift.io namespace rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("openshift.io/some-key", "some-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with key in openshift.io subdomain namespace rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("other.openshift.io/some-key", "some-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }, { + name: "tag with key in namespace similar to openshift.io accepted", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("otheropenshift.io/some-key", "some-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{ + {Key: "test-key", Value: "test-value"}, + {Key: "otheropenshift.io/some-key", Value: "some-value"}, + }, + }, { + name: "tag with duplicate key rejected", + obj: buildInfra( + withSpecTag("test-key", "test-value"), + withSpecTag("test-key", "other-value"), + ), + expectedActions: 1, + expectedTags: []configv1.AWSResourceTag{{Key: "test-key", Value: "test-value"}}, + }} + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if err := indexer.Add(tc.obj); err != nil { + t.Fatal(err.Error()) + } + fake := configfakeclient.NewSimpleClientset(tc.obj) + ctrl := AWSResourceTagsController{ + infraClient: fake.ConfigV1().Infrastructures(), + infraLister: configv1listers.NewInfrastructureLister(indexer), + } + + err := ctrl.sync(context.TODO(), + factory.NewSyncContext("AWSResourceTagsController", events.NewInMemoryRecorder("AWSResourceTagsController"))) + if tc.expectedErr == "" { + assert.NoError(t, err) + } else { + if assert.Error(t, err) { + assert.Regexp(t, tc.expectedErr, err.Error()) + } + } + assert.Equal(t, tc.expectedActions, len(fake.Actions())) + + var tags []configv1.AWSResourceTag + if tc.obj.Status.PlatformStatus != nil && tc.obj.Status.PlatformStatus.AWS != nil { + tags = tc.obj.Status.PlatformStatus.AWS.ResourceTags + } + for _, a := range fake.Actions() { + obj := a.(ktesting.UpdateAction).GetObject().(*configv1.Infrastructure) + if obj.Status.PlatformStatus != nil && obj.Status.PlatformStatus.AWS != nil { + tags = obj.Status.PlatformStatus.AWS.ResourceTags + } + } + assert.EqualValues(t, tc.expectedTags, tags) + }) + } +} + +type infraOption func(*configv1.Infrastructure) + +func buildInfra(opts ...infraOption) *configv1.Infrastructure { + infra := &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + } + for _, o := range opts { + o(infra) + } + return infra +} + +func withSpecTag(key, value string) infraOption { + return func(infra *configv1.Infrastructure) { + withAWSSpec()(infra) + infra.Spec.PlatformSpec.AWS.ResourceTags = append( + infra.Spec.PlatformSpec.AWS.ResourceTags, + configv1.AWSResourceTag{Key: key, Value: value}, + ) + } +} + +func withStatusTag(key, value string) infraOption { + return func(infra *configv1.Infrastructure) { + withAWSStatus()(infra) + infra.Status.PlatformStatus.AWS.ResourceTags = append( + infra.Status.PlatformStatus.AWS.ResourceTags, + configv1.AWSResourceTag{Key: key, Value: value}, + ) + } +} + +func withAWSSpec() infraOption { + return func(infra *configv1.Infrastructure) { + if infra.Spec.PlatformSpec.AWS == nil { + infra.Spec.PlatformSpec.AWS = &configv1.AWSPlatformSpec{} + } + } +} + +func withGCPSpec() infraOption { + return func(infra *configv1.Infrastructure) { + if infra.Spec.PlatformSpec.GCP == nil { + infra.Spec.PlatformSpec.GCP = &configv1.GCPPlatformSpec{} + } + } +} + +func withPlatformStatus() infraOption { + return func(infra *configv1.Infrastructure) { + if infra.Status.PlatformStatus == nil { + infra.Status.PlatformStatus = &configv1.PlatformStatus{} + } + } +} + +func withAWSStatus() infraOption { + return func(infra *configv1.Infrastructure) { + withPlatformStatus()(infra) + if infra.Status.PlatformStatus.AWS == nil { + infra.Status.PlatformStatus.AWS = &configv1.AWSPlatformStatus{} + } + } +} + +func withGCPStatus() infraOption { + return func(infra *configv1.Infrastructure) { + withPlatformStatus()(infra) + if infra.Status.PlatformStatus.GCP == nil { + infra.Status.PlatformStatus.GCP = &configv1.GCPPlatformStatus{} + } + } +} \ No newline at end of file diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go index 2b43e8c92..c17537852 100644 --- a/pkg/operator/starter.go +++ b/pkg/operator/starter.go @@ -24,6 +24,7 @@ import ( "github.com/openshift/library-go/pkg/operator/v1helpers" "github.com/openshift/cluster-config-operator/pkg/operator/aws_platform_service_location" + "github.com/openshift/cluster-config-operator/pkg/operator/aws_resource_tags" "github.com/openshift/cluster-config-operator/pkg/operator/kube_cloud_config" "github.com/openshift/cluster-config-operator/pkg/operator/migration_platform_status" "github.com/openshift/cluster-config-operator/pkg/operator/operatorclient" @@ -88,6 +89,14 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle controllerContext.EventRecorder, ) + awsResourceTagsController := aws_resource_tags.NewController( + operatorClient, + configClient.ConfigV1(), + configInformers.Config().V1().Infrastructures().Lister(), + configInformers.Config().V1().Infrastructures().Informer(), + controllerContext.EventRecorder, + ) + // don't change any versions until we sync versionRecorder := status.NewVersionGetter() clusterOperator, err := configClient.ConfigV1().ClusterOperators().Get(ctx, "config-operator", metav1.GetOptions{}) @@ -160,6 +169,7 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle go statusController.Run(ctx, 1) go operatorController.Run(ctx, 1) go migrationPlatformStatusController.Run(ctx, 1) + go awsResourceTagsController.Run(ctx, 1) go logLevelNormalizer.Run(ctx, 1) go staleConditionsController.Run(ctx, 1)