Skip to content

Commit b60d767

Browse files
Added Review nits
1 parent 0e627e9 commit b60d767

File tree

2 files changed

+15
-17
lines changed

2 files changed

+15
-17
lines changed

pkg/operator/awstagcontroller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ func validateUserTag(key, value string) error {
299299
return fmt.Errorf("key has invalid characters or length")
300300
}
301301
if strings.EqualFold(key, "Name") {
302-
return fmt.Errorf("key cannot be customized by user")
302+
return fmt.Errorf("name key is not allowed for user defined tags")
303303
}
304304
if !tagValRegex.MatchString(value) {
305305
return fmt.Errorf("value has invalid characters or length")

pkg/storage/s3/s3.go

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -773,12 +773,10 @@ func (d *driver) CreateStorage(cr *imageregistryv1.Config) error {
773773

774774
// at this stage we are not keeping user tags in sync. as per enhancement proposal
775775
// we only set user provided tags when we created the bucket.
776-
tagsInSpec := make(map[string]*struct{})
777776
if infra.Status.PlatformStatus.AWS != nil && len(infra.Status.PlatformStatus.AWS.ResourceTags) != 0 {
778-
klog.V(5).Infof("infra.Spec has %d user provided tags", len(infra.Status.PlatformStatus.AWS.ResourceTags))
777+
klog.V(5).Infof("infra.Status has %d user provided tags", len(infra.Status.PlatformStatus.AWS.ResourceTags))
779778
for _, tag := range infra.Status.PlatformStatus.AWS.ResourceTags {
780-
klog.Infof("user provided bucket tag in infra.Spec: %s: %s", tag.Key, tag.Value)
781-
tagsInSpec[tag.Key] = nil
779+
klog.Infof("user provided bucket tag in infra.Status: %s: %s", tag.Key, tag.Value)
782780
tagset = append(tagset, &s3.Tag{
783781
Key: aws.String(tag.Key),
784782
Value: aws.String(tag.Value),
@@ -1001,10 +999,10 @@ func sharedCredentialsDataFromStaticCreds(accessKey, accessSecret string) []byte
1001999
}
10021000

10031001
// PutStorageTags is for adding/overwriting tags of the S3 bucket
1004-
// which name is obtained using the ID() method.
1005-
func (d *driver) PutStorageTags(tagList map[string]string) error {
1006-
if len(tagList) == 0 {
1007-
klog.Info("TagSet is empty, no action taken")
1002+
// which name is obtained using this driver's ID() method.
1003+
func (d *driver) PutStorageTags(tagMap map[string]string) error {
1004+
if len(tagMap) == 0 {
1005+
klog.Info("Tags is empty, no action taken")
10081006
return nil
10091007
}
10101008

@@ -1013,9 +1011,9 @@ func (d *driver) PutStorageTags(tagList map[string]string) error {
10131011
return err
10141012
}
10151013

1016-
tagset := make([]*s3.Tag, 0, len(tagList))
1017-
for key, value := range tagList {
1018-
tagset = append(tagset, &s3.Tag{
1014+
tags := make([]*s3.Tag, 0, len(tagMap))
1015+
for key, value := range tagMap {
1016+
tags = append(tags, &s3.Tag{
10191017
Key: aws.String(key),
10201018
Value: aws.String(value),
10211019
})
@@ -1024,7 +1022,7 @@ func (d *driver) PutStorageTags(tagList map[string]string) error {
10241022
_, err = svc.PutBucketTaggingWithContext(d.Context, &s3.PutBucketTaggingInput{
10251023
Bucket: aws.String(d.ID()),
10261024
Tagging: &s3.Tagging{
1027-
TagSet: tagset,
1025+
TagSet: tags,
10281026
},
10291027
})
10301028
if err != nil {
@@ -1065,10 +1063,10 @@ func (d *driver) GetStorageTags() (map[string]string, error) {
10651063
return nil, fmt.Errorf("failed to fetch s3 bucket tags: %s", errMsg)
10661064
}
10671065

1068-
tagList := make(map[string]string)
1069-
for _, tags := range output.TagSet {
1070-
tagList[aws.StringValue(tags.Key)] = aws.StringValue(tags.Value)
1066+
tags := make(map[string]string)
1067+
for _, tag := range output.TagSet {
1068+
tags[aws.StringValue(tag.Key)] = aws.StringValue(tag.Value)
10711069
}
10721070

1073-
return tagList, nil
1071+
return tags, nil
10741072
}

0 commit comments

Comments
 (0)