Skip to content

Commit

Permalink
WIP, s3 bucket ingores system tags #2
Browse files Browse the repository at this point in the history
  • Loading branch information
Kirill Sushkov committed Mar 21, 2024
1 parent d0d3836 commit 53f59c2
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 35 deletions.
7 changes: 0 additions & 7 deletions pkg/clients/s3/fake/bucket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ type MockBucketClient struct {
MockPutBucketOwnershipControls func(ctx context.Context, input *s3.PutBucketOwnershipControlsInput, opts []func(*s3.Options)) (*s3.PutBucketOwnershipControlsOutput, error)
MockDeleteBucketOwnershipControls func(ctx context.Context, input *s3.DeleteBucketOwnershipControlsInput, opts []func(*s3.Options)) (*s3.DeleteBucketOwnershipControlsOutput, error)

MockCacheTaggingOutput func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error)

MockBucketPolicyClient
}

Expand Down Expand Up @@ -295,8 +293,3 @@ func (m MockBucketClient) PutBucketOwnershipControls(ctx context.Context, input
func (m MockBucketClient) DeleteBucketOwnershipControls(ctx context.Context, input *s3.DeleteBucketOwnershipControlsInput, opts ...func(*s3.Options)) (*s3.DeleteBucketOwnershipControlsOutput, error) {
return m.MockDeleteBucketOwnershipControls(ctx, input, opts)
}

// CacheTaggingOutput is the fake method call to invoke internal mock method
func (m MockBucketClient) CacheTaggingOutput(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
return m.MockCacheTaggingOutput(ctx, bucketName)
}
25 changes: 10 additions & 15 deletions pkg/controller/s3/bucket/taggingConfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package bucket

import (
"context"
"fmt"
"k8s.io/utils/ptr"
"strings"

Expand Down Expand Up @@ -57,8 +56,8 @@ func NewTaggingConfigurationClient(client s3.BucketClient) *TaggingConfiguration
return &TaggingConfigurationClient{client: client}
}

// CacheTaggingOutput caches `TaggingConfigurationClient.GetBucketTagging` output or retrieve already cached object
func (in *TaggingConfigurationClient) CacheTaggingOutput(ctx context.Context, bucketName *string) (*awss3.GetBucketTaggingOutput, error) {
// CacheBucketTaggingOutput caches `TaggingConfigurationClient.GetBucketTagging` output or retrieve already cached object
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 {
Expand All @@ -73,16 +72,14 @@ func (in *TaggingConfigurationClient) CacheTaggingOutput(ctx context.Context, bu
// 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) {
config := bucket.Spec.ForProvider.BucketTagging.DeepCopy()
external, err := in.CacheTaggingOutput(ctx, pointer.ToOrNilIfZeroValue(meta.GetExternalName(bucket)))
fmt.Printf("real CacheTaggingOutput: %+v\n", external)
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)
}
AddExistingSystemTags(config, external)
fmt.Printf("real current config: %+v\n", config)
AddExistingSystemTags(&config, external)
switch {
case len(config.TagSet) == 0 && len(external.TagSet) == 0:
return Updated, nil
Expand All @@ -98,7 +95,7 @@ func (in *TaggingConfigurationClient) Observe(ctx context.Context, bucket *v1bet
// CreateOrUpdate sends a request to have resource created on AWS
func (in *TaggingConfigurationClient) CreateOrUpdate(ctx context.Context, bucket *v1beta1.Bucket) error {
if in.cache.getBucketTaggingOutput != nil {
AddExistingSystemTags(bucket.Spec.ForProvider.BucketTagging, in.cache.getBucketTaggingOutput)
AddExistingSystemTags(&bucket.Spec.ForProvider.BucketTagging, in.cache.getBucketTaggingOutput)
}
if len(bucket.Spec.ForProvider.BucketTagging.TagSet) == 0 {
return nil
Expand All @@ -121,7 +118,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.CacheTaggingOutput(ctx, 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 @@ -173,10 +170,9 @@ func GeneratePutBucketTagging(name string, config *v1beta1.Tagging) *awss3.PutBu
}

// AddExistingSystemTags enrich `currentTags` by system tags from observed object if these tags exist
func AddExistingSystemTags(currentTags *v1beta1.Tagging, observedTags *awss3.GetBucketTaggingOutput) {
if currentTags == nil {
currentTags = &v1beta1.Tagging{TagSet: make([]v1beta1.Tag, 0)}
fmt.Printf("It was nil, but know: %+v\n", currentTags)
func AddExistingSystemTags(currentTags **v1beta1.Tagging, observedTags *awss3.GetBucketTaggingOutput) {
if *currentTags == nil {
*currentTags = &v1beta1.Tagging{TagSet: make([]v1beta1.Tag, 0)}
}
var systemTagSet []v1beta1.Tag
for _, t := range observedTags.TagSet {
Expand All @@ -185,6 +181,5 @@ func AddExistingSystemTags(currentTags *v1beta1.Tagging, observedTags *awss3.Get
systemTagSet = append(systemTagSet, v1beta1.Tag{Key: ptr.Deref(t.Key, ""), Value: ptr.Deref(t.Value, "")})
}
}
currentTags.TagSet = append(currentTags.TagSet, systemTagSet...)
fmt.Printf("AddExistingSystemTags result: %+v\n", currentTags)
(*currentTags).TagSet = append((*currentTags).TagSet, systemTagSet...)
}
26 changes: 13 additions & 13 deletions pkg/controller/s3/bucket/taggingConfig_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestTaggingObserve(t *testing.T) {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(generateTaggingConfig())),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return nil, errBoom
},
}),
Expand All @@ -109,7 +109,7 @@ func TestTaggingObserve(t *testing.T) {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(generateTaggingConfig())),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: nil}, nil
},
}),
Expand All @@ -123,7 +123,7 @@ func TestTaggingObserve(t *testing.T) {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(nil)),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: generateAWSTagging().TagSet}, nil
},
}),
Expand All @@ -137,7 +137,7 @@ func TestTaggingObserve(t *testing.T) {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(nil)),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return nil, &smithy.GenericAPIError{Code: clientss3.TaggingNotFoundErrCode}
},
}),
Expand All @@ -151,7 +151,7 @@ func TestTaggingObserve(t *testing.T) {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(nil)),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: nil}, nil
},
}),
Expand All @@ -165,7 +165,7 @@ func TestTaggingObserve(t *testing.T) {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(generateTaggingConfig())),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: generateAWSTagging().TagSet}, nil
},
}),
Expand All @@ -179,7 +179,7 @@ func TestTaggingObserve(t *testing.T) {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(generateTaggingConfig())),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: []types.Tag{awsTag2, awsTag, awsTag1}}, nil
},
}),
Expand Down Expand Up @@ -340,7 +340,7 @@ func TestTaggingLateInit(t *testing.T) {
args: args{
b: s3testing.Bucket(),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return nil, errBoom
},
}),
Expand All @@ -354,7 +354,7 @@ func TestTaggingLateInit(t *testing.T) {
args: args{
b: s3testing.Bucket(),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return nil, &smithy.GenericAPIError{Code: clientss3.TaggingNotFoundErrCode}
},
}),
Expand All @@ -368,7 +368,7 @@ func TestTaggingLateInit(t *testing.T) {
args: args{
b: s3testing.Bucket(),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: nil}, nil
},
}),
Expand All @@ -382,7 +382,7 @@ func TestTaggingLateInit(t *testing.T) {
args: args{
b: s3testing.Bucket(),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: []types.Tag{}}, nil
},
}),
Expand All @@ -396,7 +396,7 @@ func TestTaggingLateInit(t *testing.T) {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(nil)),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: generateAWSTagging().TagSet}, nil
},
}),
Expand All @@ -410,7 +410,7 @@ func TestTaggingLateInit(t *testing.T) {
args: args{
b: s3testing.Bucket(s3testing.WithTaggingConfig(generateTaggingConfig())),
cl: NewTaggingConfigurationClient(fake.MockBucketClient{
MockCacheTaggingOutput: func(ctx context.Context, bucketName *string) (*s3.GetBucketTaggingOutput, error) {
MockGetBucketTagging: func(ctx context.Context, input *s3.GetBucketTaggingInput, opts []func(*s3.Options)) (*s3.GetBucketTaggingOutput, error) {
return &s3.GetBucketTaggingOutput{TagSet: []types.Tag{}}, nil
},
}),
Expand Down

0 comments on commit 53f59c2

Please sign in to comment.