Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(s3 bucket): Ignore AWS system tags #2018

Merged
merged 1 commit into from
Mar 25, 2024
Merged

fix(s3 bucket): Ignore AWS system tags #2018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 53 additions & 6 deletions pkg/controller/s3/bucket/taggingConfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package bucket

import (
"context"
"strings"

awss3 "github.com/aws/aws-sdk-go-v2/service/s3"
"github.com/aws/aws-sdk-go-v2/service/s3/types"
Expand All @@ -26,6 +27,7 @@ import (
"github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"k8s.io/utils/ptr"

"github.com/crossplane-contrib/provider-aws/apis/s3/v1beta1"
"github.com/crossplane-contrib/provider-aws/pkg/clients/s3"
Expand All @@ -39,27 +41,46 @@ const (
taggingDeleteFailed = "cannot delete Bucket tagging set"
)

type cache struct {
getBucketTaggingOutput *awss3.GetBucketTaggingOutput
}

// TaggingConfigurationClient is the client for API methods and reconciling the CORSConfiguration
type TaggingConfigurationClient struct {
client s3.BucketClient
cache cache
}

// NewTaggingConfigurationClient creates the client for CORS Configuration
func NewTaggingConfigurationClient(client s3.BucketClient) *TaggingConfigurationClient {
return &TaggingConfigurationClient{client: client}
}

// CacheBucketTaggingOutput returns cached *awss3.GetBucketTaggingOutput` if it exists, otherwise adds
// `TaggingConfigurationClient.GetBucketTagging` output to cache and then returns it
func (in *TaggingConfigurationClient) CacheBucketTaggingOutput(ctx context.Context, bucketName *string) (*awss3.GetBucketTaggingOutput, error) {
if in.cache.getBucketTaggingOutput == nil {
external, err := in.client.GetBucketTagging(ctx, &awss3.GetBucketTaggingInput{Bucket: bucketName})
if err != nil {
return external, err
}
in.cache.getBucketTaggingOutput = external
return external, nil
}
return in.cache.getBucketTaggingOutput, nil
}

// Observe checks if the resource exists and if it matches the local configuration
func (in *TaggingConfigurationClient) Observe(ctx context.Context, bucket *v1beta1.Bucket) (ResourceStatus, error) {
external, err := in.client.GetBucketTagging(ctx, &awss3.GetBucketTaggingInput{Bucket: pointer.ToOrNilIfZeroValue(meta.GetExternalName(bucket))})
config := bucket.Spec.ForProvider.BucketTagging
config := bucket.Spec.ForProvider.BucketTagging.DeepCopy()
external, err := in.CacheBucketTaggingOutput(ctx, pointer.ToOrNilIfZeroValue(meta.GetExternalName(bucket)))
if err != nil {
if s3.TaggingNotFound(err) && config == nil {
return Updated, nil
}
return NeedsUpdate, errorutils.Wrap(resource.Ignore(s3.TaggingNotFound, err), taggingGetFailed)
}

config = addExistingSystemTags(config, external)
switch {
case config == nil && len(external.TagSet) == 0:
return Updated, nil
Expand All @@ -77,8 +98,12 @@ func (in *TaggingConfigurationClient) CreateOrUpdate(ctx context.Context, bucket
if bucket.Spec.ForProvider.BucketTagging == nil {
return nil
}
input := GeneratePutBucketTagging(meta.GetExternalName(bucket), bucket.Spec.ForProvider.BucketTagging)
_, err := in.client.PutBucketTagging(ctx, input)
external, err := in.CacheBucketTaggingOutput(ctx, pointer.ToOrNilIfZeroValue(meta.GetExternalName(bucket)))
if err != nil {
return err
}
input := GeneratePutBucketTagging(meta.GetExternalName(bucket), addExistingSystemTags(bucket.Spec.ForProvider.BucketTagging, external))
_, err = in.client.PutBucketTagging(ctx, input)
return errorutils.Wrap(err, taggingPutFailed)
}

Expand All @@ -95,7 +120,7 @@ func (in *TaggingConfigurationClient) Delete(ctx context.Context, bucket *v1beta
// LateInitialize does nothing because the resource might have been deleted by
// the user.
func (in *TaggingConfigurationClient) LateInitialize(ctx context.Context, bucket *v1beta1.Bucket) error {
external, err := in.client.GetBucketTagging(ctx, &awss3.GetBucketTaggingInput{Bucket: pointer.ToOrNilIfZeroValue(meta.GetExternalName(bucket))})
external, err := in.CacheBucketTaggingOutput(ctx, pointer.ToOrNilIfZeroValue(meta.GetExternalName(bucket)))
if err != nil {
return errorutils.Wrap(resource.Ignore(s3.TaggingNotFound, err), taggingGetFailed)
}
Expand Down Expand Up @@ -145,3 +170,25 @@ func GeneratePutBucketTagging(name string, config *v1beta1.Tagging) *awss3.PutBu
Tagging: GenerateTagging(config),
}
}

// addExistingSystemTags returns `*v1beta1.Tagging` which contains tags from desired state and system tags from observed resource if these tags exist
// AWS API provides only put/delete/get operations for tags, so there is only one way to change - override the whole tag set,
// It is impossible in case if observed bucket already has systemd tags(they are not settable), in this case combining tags from desired tagSet
// with system tags from observed bucket is equal to ignoring of them
func addExistingSystemTags(desiredTags *v1beta1.Tagging, observedTags *awss3.GetBucketTaggingOutput) *v1beta1.Tagging {
var systemTags []v1beta1.Tag
tagSet := desiredTags.DeepCopy()
for _, t := range observedTags.TagSet {
key := pointer.StringValue(t.Key)
if strings.HasPrefix(key, "aws:") {
systemTags = append(systemTags, v1beta1.Tag{Key: ptr.Deref(t.Key, ""), Value: ptr.Deref(t.Value, "")})
}
}
if systemTags != nil {
if tagSet == nil {
tagSet = &v1beta1.Tagging{TagSet: make([]v1beta1.Tag, 0)}
}
tagSet.TagSet = append(tagSet.TagSet, systemTags...)
}
return tagSet
}
41 changes: 41 additions & 0 deletions pkg/controller/s3/bucket/taggingConfig_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ var (
Key: aws.String("abc"),
Value: aws.String("abc"),
}
awsSystemTag1 = types.Tag{
Key: aws.String("aws:tag1"),
Value: aws.String("1"),
}
awsTags = []types.Tag{awsTag, awsTag1, awsTag2}
_ SubresourceClient = &TaggingConfigurationClient{}
)
Expand Down Expand Up @@ -133,6 +137,34 @@ func TestTaggingObserve(t *testing.T) {
err: nil,
},
},
"DeleteNoNeededIgnoreSystemTags": {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(nil)),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: []types.Tag{awsSystemTag1}}, nil
},
}),
},
want: want{
status: Updated,
err: nil,
},
},
"NoUpdateIgnoreSystemTags": {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(generateTaggingConfig())),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: []types.Tag{awsTag2, awsTag, awsTag1, awsSystemTag1}}, nil
},
}),
},
want: want{
status: Updated,
err: nil,
},
},
"NoUpdateNotExists": {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(nil)),
Expand Down Expand Up @@ -225,6 +257,9 @@ func TestTaggingCreateOrUpdate(t *testing.T) {
MockPutBucketTagging: func(ctx context.Context, input *s3.PutBucketTaggingInput, opts []func(*s3.Options)) (*s3.PutBucketTaggingOutput, error) {
return nil, errBoom
},
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: nil}, nil
},
}),
},
want: want{
Expand All @@ -238,6 +273,9 @@ func TestTaggingCreateOrUpdate(t *testing.T) {
MockPutBucketTagging: func(ctx context.Context, input *s3.PutBucketTaggingInput, opts []func(*s3.Options)) (*s3.PutBucketTaggingOutput, error) {
return &s3.PutBucketTaggingOutput{}, nil
},
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: nil}, nil
},
}),
},
want: want{
Expand All @@ -251,6 +289,9 @@ func TestTaggingCreateOrUpdate(t *testing.T) {
MockPutBucketTagging: func(ctx context.Context, input *s3.PutBucketTaggingInput, opts []func(*s3.Options)) (*s3.PutBucketTaggingOutput, error) {
return &s3.PutBucketTaggingOutput{}, nil
},
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: nil}, nil
},
}),
},
want: want{
Expand Down
Loading