From 9da83681c116997dce9eecc1294d5f5f1173d554 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Mon, 20 May 2024 11:05:00 +0200 Subject: [PATCH 01/13] Add bucketPolicy in type --- apis/provider-ceph/v1alpha1/bucket_types.go | 6 ++++++ package/crds/provider-ceph.ceph.crossplane.io_buckets.yaml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/apis/provider-ceph/v1alpha1/bucket_types.go b/apis/provider-ceph/v1alpha1/bucket_types.go index 7bf84e8e..44821345 100644 --- a/apis/provider-ceph/v1alpha1/bucket_types.go +++ b/apis/provider-ceph/v1alpha1/bucket_types.go @@ -85,6 +85,12 @@ type BucketParameters struct { // AssumeRoleTags may be used to add custom values to an AssumeRole request. // +optional AssumeRoleTags []Tag `json:"assumeRoleTags,omitempty"` + + // BucketPolicy is a JSON string of BucketPolicy. + // If it is set, Provider-Ceph calls PutBucketPolicy API after creating the bucket. + // Before adding it, you should validate the JSON string. + // +optional + BucketPolicy string `json:"bucketPolicy,omitempty"` } // BackendInfo contains relevant information about an S3 backend for diff --git a/package/crds/provider-ceph.ceph.crossplane.io_buckets.yaml b/package/crds/provider-ceph.ceph.crossplane.io_buckets.yaml index 032838ed..32113c9a 100644 --- a/package/crds/provider-ceph.ceph.crossplane.io_buckets.yaml +++ b/package/crds/provider-ceph.ceph.crossplane.io_buckets.yaml @@ -179,6 +179,12 @@ spec: - value type: object type: array + bucketPolicy: + description: |- + BucketPolicy is a JSON string of BucketPolicy. + If it is set, Provider-Ceph calls PutBucketPolicy API after creating the bucket. + Before adding it, you should validate the JSON string. + type: string grantFullControl: description: |- Allows grantee the read, write, read ACP, and write ACP permissions on the From 7592392c299000e80aa90d3f7783e0c624d40977 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Mon, 20 May 2024 11:11:00 +0200 Subject: [PATCH 02/13] mod --- go.mod | 1 + go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/go.mod b/go.mod index 5d9b71db..f032919c 100644 --- a/go.mod +++ b/go.mod @@ -84,6 +84,7 @@ require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.20 // indirect github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect + github.com/maxbrunsfeld/counterfeiter/v6 v6.8.1 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect diff --git a/go.sum b/go.sum index aeacd325..831f9e19 100644 --- a/go.sum +++ b/go.sum @@ -143,6 +143,8 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg= github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k= +github.com/maxbrunsfeld/counterfeiter/v6 v6.8.1 h1:NicmruxkeqHjDv03SfSxqmaLuisddudfP3h5wdXFbhM= +github.com/maxbrunsfeld/counterfeiter/v6 v6.8.1/go.mod h1:eyp4DdUJAKkr9tvxR3jWhw2mDK7CWABMG5r9uyaKC7I= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= From e0915836d25f3ff4588335d42d6aae339f2c3605 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Mon, 20 May 2024 11:11:14 +0200 Subject: [PATCH 03/13] add new APIs in s3 backend --- internal/backendstore/backend.go | 3 + .../backendstorefakes/fake_s3client.go | 249 ++++++++++++++++++ 2 files changed, 252 insertions(+) diff --git a/internal/backendstore/backend.go b/internal/backendstore/backend.go index b871aa7c..e152210b 100644 --- a/internal/backendstore/backend.go +++ b/internal/backendstore/backend.go @@ -41,6 +41,9 @@ type S3Client interface { DeleteBucketLifecycle(context.Context, *s3.DeleteBucketLifecycleInput, ...func(*s3.Options)) (*s3.DeleteBucketLifecycleOutput, error) GetBucketAcl(context.Context, *s3.GetBucketAclInput, ...func(*s3.Options)) (*s3.GetBucketAclOutput, error) PutBucketAcl(context.Context, *s3.PutBucketAclInput, ...func(*s3.Options)) (*s3.PutBucketAclOutput, error) + PutBucketPolicy(context.Context, *s3.PutBucketPolicyInput, ...func(*s3.Options)) (*s3.PutBucketPolicyOutput, error) + GetBucketPolicy(context.Context, *s3.GetBucketPolicyInput, ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) + DeleteBucketPolicy(context.Context, *s3.DeleteBucketPolicyInput, ...func(*s3.Options)) (*s3.DeleteBucketPolicyOutput, error) } //counterfeiter:generate . STSClient diff --git a/internal/backendstore/backendstorefakes/fake_s3client.go b/internal/backendstore/backendstorefakes/fake_s3client.go index 24ba2922..90c41468 100644 --- a/internal/backendstore/backendstorefakes/fake_s3client.go +++ b/internal/backendstore/backendstorefakes/fake_s3client.go @@ -55,6 +55,21 @@ type FakeS3Client struct { result1 *s3.DeleteBucketLifecycleOutput result2 error } + DeleteBucketPolicyStub func(context.Context, *s3.DeleteBucketPolicyInput, ...func(*s3.Options)) (*s3.DeleteBucketPolicyOutput, error) + deleteBucketPolicyMutex sync.RWMutex + deleteBucketPolicyArgsForCall []struct { + arg1 context.Context + arg2 *s3.DeleteBucketPolicyInput + arg3 []func(*s3.Options) + } + deleteBucketPolicyReturns struct { + result1 *s3.DeleteBucketPolicyOutput + result2 error + } + deleteBucketPolicyReturnsOnCall map[int]struct { + result1 *s3.DeleteBucketPolicyOutput + result2 error + } DeleteObjectStub func(context.Context, *s3.DeleteObjectInput, ...func(*s3.Options)) (*s3.DeleteObjectOutput, error) deleteObjectMutex sync.RWMutex deleteObjectArgsForCall []struct { @@ -100,6 +115,21 @@ type FakeS3Client struct { result1 *s3.GetBucketLifecycleConfigurationOutput result2 error } + GetBucketPolicyStub func(context.Context, *s3.GetBucketPolicyInput, ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) + getBucketPolicyMutex sync.RWMutex + getBucketPolicyArgsForCall []struct { + arg1 context.Context + arg2 *s3.GetBucketPolicyInput + arg3 []func(*s3.Options) + } + getBucketPolicyReturns struct { + result1 *s3.GetBucketPolicyOutput + result2 error + } + getBucketPolicyReturnsOnCall map[int]struct { + result1 *s3.GetBucketPolicyOutput + result2 error + } GetObjectStub func(context.Context, *s3.GetObjectInput, ...func(*s3.Options)) (*s3.GetObjectOutput, error) getObjectMutex sync.RWMutex getObjectArgsForCall []struct { @@ -190,6 +220,21 @@ type FakeS3Client struct { result1 *s3.PutBucketLifecycleConfigurationOutput result2 error } + PutBucketPolicyStub func(context.Context, *s3.PutBucketPolicyInput, ...func(*s3.Options)) (*s3.PutBucketPolicyOutput, error) + putBucketPolicyMutex sync.RWMutex + putBucketPolicyArgsForCall []struct { + arg1 context.Context + arg2 *s3.PutBucketPolicyInput + arg3 []func(*s3.Options) + } + putBucketPolicyReturns struct { + result1 *s3.PutBucketPolicyOutput + result2 error + } + putBucketPolicyReturnsOnCall map[int]struct { + result1 *s3.PutBucketPolicyOutput + result2 error + } PutObjectStub func(context.Context, *s3.PutObjectInput, ...func(*s3.Options)) (*s3.PutObjectOutput, error) putObjectMutex sync.RWMutex putObjectArgsForCall []struct { @@ -407,6 +452,72 @@ func (fake *FakeS3Client) DeleteBucketLifecycleReturnsOnCall(i int, result1 *s3. }{result1, result2} } +func (fake *FakeS3Client) DeleteBucketPolicy(arg1 context.Context, arg2 *s3.DeleteBucketPolicyInput, arg3 ...func(*s3.Options)) (*s3.DeleteBucketPolicyOutput, error) { + fake.deleteBucketPolicyMutex.Lock() + ret, specificReturn := fake.deleteBucketPolicyReturnsOnCall[len(fake.deleteBucketPolicyArgsForCall)] + fake.deleteBucketPolicyArgsForCall = append(fake.deleteBucketPolicyArgsForCall, struct { + arg1 context.Context + arg2 *s3.DeleteBucketPolicyInput + arg3 []func(*s3.Options) + }{arg1, arg2, arg3}) + stub := fake.DeleteBucketPolicyStub + fakeReturns := fake.deleteBucketPolicyReturns + fake.recordInvocation("DeleteBucketPolicy", []interface{}{arg1, arg2, arg3}) + fake.deleteBucketPolicyMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3...) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeS3Client) DeleteBucketPolicyCallCount() int { + fake.deleteBucketPolicyMutex.RLock() + defer fake.deleteBucketPolicyMutex.RUnlock() + return len(fake.deleteBucketPolicyArgsForCall) +} + +func (fake *FakeS3Client) DeleteBucketPolicyCalls(stub func(context.Context, *s3.DeleteBucketPolicyInput, ...func(*s3.Options)) (*s3.DeleteBucketPolicyOutput, error)) { + fake.deleteBucketPolicyMutex.Lock() + defer fake.deleteBucketPolicyMutex.Unlock() + fake.DeleteBucketPolicyStub = stub +} + +func (fake *FakeS3Client) DeleteBucketPolicyArgsForCall(i int) (context.Context, *s3.DeleteBucketPolicyInput, []func(*s3.Options)) { + fake.deleteBucketPolicyMutex.RLock() + defer fake.deleteBucketPolicyMutex.RUnlock() + argsForCall := fake.deleteBucketPolicyArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeS3Client) DeleteBucketPolicyReturns(result1 *s3.DeleteBucketPolicyOutput, result2 error) { + fake.deleteBucketPolicyMutex.Lock() + defer fake.deleteBucketPolicyMutex.Unlock() + fake.DeleteBucketPolicyStub = nil + fake.deleteBucketPolicyReturns = struct { + result1 *s3.DeleteBucketPolicyOutput + result2 error + }{result1, result2} +} + +func (fake *FakeS3Client) DeleteBucketPolicyReturnsOnCall(i int, result1 *s3.DeleteBucketPolicyOutput, result2 error) { + fake.deleteBucketPolicyMutex.Lock() + defer fake.deleteBucketPolicyMutex.Unlock() + fake.DeleteBucketPolicyStub = nil + if fake.deleteBucketPolicyReturnsOnCall == nil { + fake.deleteBucketPolicyReturnsOnCall = make(map[int]struct { + result1 *s3.DeleteBucketPolicyOutput + result2 error + }) + } + fake.deleteBucketPolicyReturnsOnCall[i] = struct { + result1 *s3.DeleteBucketPolicyOutput + result2 error + }{result1, result2} +} + func (fake *FakeS3Client) DeleteObject(arg1 context.Context, arg2 *s3.DeleteObjectInput, arg3 ...func(*s3.Options)) (*s3.DeleteObjectOutput, error) { fake.deleteObjectMutex.Lock() ret, specificReturn := fake.deleteObjectReturnsOnCall[len(fake.deleteObjectArgsForCall)] @@ -605,6 +716,72 @@ func (fake *FakeS3Client) GetBucketLifecycleConfigurationReturnsOnCall(i int, re }{result1, result2} } +func (fake *FakeS3Client) GetBucketPolicy(arg1 context.Context, arg2 *s3.GetBucketPolicyInput, arg3 ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) { + fake.getBucketPolicyMutex.Lock() + ret, specificReturn := fake.getBucketPolicyReturnsOnCall[len(fake.getBucketPolicyArgsForCall)] + fake.getBucketPolicyArgsForCall = append(fake.getBucketPolicyArgsForCall, struct { + arg1 context.Context + arg2 *s3.GetBucketPolicyInput + arg3 []func(*s3.Options) + }{arg1, arg2, arg3}) + stub := fake.GetBucketPolicyStub + fakeReturns := fake.getBucketPolicyReturns + fake.recordInvocation("GetBucketPolicy", []interface{}{arg1, arg2, arg3}) + fake.getBucketPolicyMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3...) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeS3Client) GetBucketPolicyCallCount() int { + fake.getBucketPolicyMutex.RLock() + defer fake.getBucketPolicyMutex.RUnlock() + return len(fake.getBucketPolicyArgsForCall) +} + +func (fake *FakeS3Client) GetBucketPolicyCalls(stub func(context.Context, *s3.GetBucketPolicyInput, ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error)) { + fake.getBucketPolicyMutex.Lock() + defer fake.getBucketPolicyMutex.Unlock() + fake.GetBucketPolicyStub = stub +} + +func (fake *FakeS3Client) GetBucketPolicyArgsForCall(i int) (context.Context, *s3.GetBucketPolicyInput, []func(*s3.Options)) { + fake.getBucketPolicyMutex.RLock() + defer fake.getBucketPolicyMutex.RUnlock() + argsForCall := fake.getBucketPolicyArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeS3Client) GetBucketPolicyReturns(result1 *s3.GetBucketPolicyOutput, result2 error) { + fake.getBucketPolicyMutex.Lock() + defer fake.getBucketPolicyMutex.Unlock() + fake.GetBucketPolicyStub = nil + fake.getBucketPolicyReturns = struct { + result1 *s3.GetBucketPolicyOutput + result2 error + }{result1, result2} +} + +func (fake *FakeS3Client) GetBucketPolicyReturnsOnCall(i int, result1 *s3.GetBucketPolicyOutput, result2 error) { + fake.getBucketPolicyMutex.Lock() + defer fake.getBucketPolicyMutex.Unlock() + fake.GetBucketPolicyStub = nil + if fake.getBucketPolicyReturnsOnCall == nil { + fake.getBucketPolicyReturnsOnCall = make(map[int]struct { + result1 *s3.GetBucketPolicyOutput + result2 error + }) + } + fake.getBucketPolicyReturnsOnCall[i] = struct { + result1 *s3.GetBucketPolicyOutput + result2 error + }{result1, result2} +} + func (fake *FakeS3Client) GetObject(arg1 context.Context, arg2 *s3.GetObjectInput, arg3 ...func(*s3.Options)) (*s3.GetObjectOutput, error) { fake.getObjectMutex.Lock() ret, specificReturn := fake.getObjectReturnsOnCall[len(fake.getObjectArgsForCall)] @@ -1001,6 +1178,72 @@ func (fake *FakeS3Client) PutBucketLifecycleConfigurationReturnsOnCall(i int, re }{result1, result2} } +func (fake *FakeS3Client) PutBucketPolicy(arg1 context.Context, arg2 *s3.PutBucketPolicyInput, arg3 ...func(*s3.Options)) (*s3.PutBucketPolicyOutput, error) { + fake.putBucketPolicyMutex.Lock() + ret, specificReturn := fake.putBucketPolicyReturnsOnCall[len(fake.putBucketPolicyArgsForCall)] + fake.putBucketPolicyArgsForCall = append(fake.putBucketPolicyArgsForCall, struct { + arg1 context.Context + arg2 *s3.PutBucketPolicyInput + arg3 []func(*s3.Options) + }{arg1, arg2, arg3}) + stub := fake.PutBucketPolicyStub + fakeReturns := fake.putBucketPolicyReturns + fake.recordInvocation("PutBucketPolicy", []interface{}{arg1, arg2, arg3}) + fake.putBucketPolicyMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3...) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeS3Client) PutBucketPolicyCallCount() int { + fake.putBucketPolicyMutex.RLock() + defer fake.putBucketPolicyMutex.RUnlock() + return len(fake.putBucketPolicyArgsForCall) +} + +func (fake *FakeS3Client) PutBucketPolicyCalls(stub func(context.Context, *s3.PutBucketPolicyInput, ...func(*s3.Options)) (*s3.PutBucketPolicyOutput, error)) { + fake.putBucketPolicyMutex.Lock() + defer fake.putBucketPolicyMutex.Unlock() + fake.PutBucketPolicyStub = stub +} + +func (fake *FakeS3Client) PutBucketPolicyArgsForCall(i int) (context.Context, *s3.PutBucketPolicyInput, []func(*s3.Options)) { + fake.putBucketPolicyMutex.RLock() + defer fake.putBucketPolicyMutex.RUnlock() + argsForCall := fake.putBucketPolicyArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeS3Client) PutBucketPolicyReturns(result1 *s3.PutBucketPolicyOutput, result2 error) { + fake.putBucketPolicyMutex.Lock() + defer fake.putBucketPolicyMutex.Unlock() + fake.PutBucketPolicyStub = nil + fake.putBucketPolicyReturns = struct { + result1 *s3.PutBucketPolicyOutput + result2 error + }{result1, result2} +} + +func (fake *FakeS3Client) PutBucketPolicyReturnsOnCall(i int, result1 *s3.PutBucketPolicyOutput, result2 error) { + fake.putBucketPolicyMutex.Lock() + defer fake.putBucketPolicyMutex.Unlock() + fake.PutBucketPolicyStub = nil + if fake.putBucketPolicyReturnsOnCall == nil { + fake.putBucketPolicyReturnsOnCall = make(map[int]struct { + result1 *s3.PutBucketPolicyOutput + result2 error + }) + } + fake.putBucketPolicyReturnsOnCall[i] = struct { + result1 *s3.PutBucketPolicyOutput + result2 error + }{result1, result2} +} + func (fake *FakeS3Client) PutObject(arg1 context.Context, arg2 *s3.PutObjectInput, arg3 ...func(*s3.Options)) (*s3.PutObjectOutput, error) { fake.putObjectMutex.Lock() ret, specificReturn := fake.putObjectReturnsOnCall[len(fake.putObjectArgsForCall)] @@ -1076,12 +1319,16 @@ func (fake *FakeS3Client) Invocations() map[string][][]interface{} { defer fake.deleteBucketMutex.RUnlock() fake.deleteBucketLifecycleMutex.RLock() defer fake.deleteBucketLifecycleMutex.RUnlock() + fake.deleteBucketPolicyMutex.RLock() + defer fake.deleteBucketPolicyMutex.RUnlock() fake.deleteObjectMutex.RLock() defer fake.deleteObjectMutex.RUnlock() fake.getBucketAclMutex.RLock() defer fake.getBucketAclMutex.RUnlock() fake.getBucketLifecycleConfigurationMutex.RLock() defer fake.getBucketLifecycleConfigurationMutex.RUnlock() + fake.getBucketPolicyMutex.RLock() + defer fake.getBucketPolicyMutex.RUnlock() fake.getObjectMutex.RLock() defer fake.getObjectMutex.RUnlock() fake.headBucketMutex.RLock() @@ -1094,6 +1341,8 @@ func (fake *FakeS3Client) Invocations() map[string][][]interface{} { defer fake.putBucketAclMutex.RUnlock() fake.putBucketLifecycleConfigurationMutex.RLock() defer fake.putBucketLifecycleConfigurationMutex.RUnlock() + fake.putBucketPolicyMutex.RLock() + defer fake.putBucketPolicyMutex.RUnlock() fake.putObjectMutex.RLock() defer fake.putObjectMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} From 538f1bf71f4a74b76e385f19b116a3c38ba4fbba Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Mon, 20 May 2024 11:49:01 +0200 Subject: [PATCH 04/13] add policies in RGW --- internal/rgw/policy.go | 60 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 internal/rgw/policy.go diff --git a/internal/rgw/policy.go b/internal/rgw/policy.go new file mode 100644 index 00000000..2f7eb98e --- /dev/null +++ b/internal/rgw/policy.go @@ -0,0 +1,60 @@ +package rgw + +import ( + "context" + + awss3 "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/crossplane/crossplane-runtime/pkg/errors" + "github.com/linode/provider-ceph/apis/provider-ceph/v1alpha1" + "github.com/linode/provider-ceph/internal/backendstore" + "github.com/linode/provider-ceph/internal/otel/traces" + "go.opentelemetry.io/otel" +) + +const ( + errGetBucketPolicy = "" + errPutBucketPolicy = "" + errDeleteBucketPolicy = "" +) + +func GetBucketPolicy(ctx context.Context, s3Backend backendstore.S3Client, bucketName *string) (*awss3.GetBucketPolicyOutput, error) { + ctx, span := otel.Tracer("").Start(ctx, "GetBucketPolicy") + defer span.End() + + resp, err := s3Backend.GetBucketPolicy(ctx, &awss3.GetBucketPolicyInput{Bucket: bucketName}) + if err != nil { + traces.SetAndRecordError(span, err) + + return resp, errors.Wrap(err, errGetBucketPolicy) + } + + return resp, nil +} + +func PutBucketPolicy(ctx context.Context, s3Backend backendstore.S3Client, b *v1alpha1.Bucket) (*awss3.PutBucketPolicyOutput, error) { + ctx, span := otel.Tracer("").Start(ctx, "PutBucketPolicy") + defer span.End() + + resp, err := s3Backend.PutBucketPolicy(ctx, &awss3.PutBucketPolicyInput{Bucket: &b.Name, Policy: &b.Spec.ForProvider.BucketPolicy}) + if err != nil { + traces.SetAndRecordError(span, err) + + return resp, errors.Wrap(err, errPutBucketPolicy) + } + + return resp, nil +} + +func DeleteBucketPolicy(ctx context.Context, s3Backend backendstore.S3Client, bucketName *string) (*awss3.DeleteBucketPolicyOutput, error) { + ctx, span := otel.Tracer("").Start(ctx, "DeleteBucketPolicy") + defer span.End() + + resp, err := s3Backend.DeleteBucketPolicy(ctx, &awss3.DeleteBucketPolicyInput{Bucket: bucketName}) + if err != nil { + traces.SetAndRecordError(span, err) + + return resp, errors.Wrap(err, errDeleteBucketPolicy) + } + + return resp, nil +} From 4fb60de3e9e2aff68a60cf4cdd3ee1596ea1d5ea Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Mon, 20 May 2024 15:18:11 +0200 Subject: [PATCH 05/13] add policy --- internal/controller/bucket/consts.go | 4 + internal/controller/bucket/policy.go | 192 +++++++++++++++++++++++++++ internal/rgw/policy.go | 8 +- 3 files changed, 200 insertions(+), 4 deletions(-) create mode 100644 internal/controller/bucket/policy.go diff --git a/internal/controller/bucket/consts.go b/internal/controller/bucket/consts.go index 478872ca..4325b871 100644 --- a/internal/controller/bucket/consts.go +++ b/internal/controller/bucket/consts.go @@ -24,5 +24,9 @@ const ( errObserveAcl = "failed to observe bucket acl" errHandleAcl = "failed to handle bucket acl" + // BucketPolicy error messages. + errObservePolicy = "failed to observe bucket policy" + errHandlePolicy = "failed to handle bucket policy" + True = "true" ) diff --git a/internal/controller/bucket/policy.go b/internal/controller/bucket/policy.go new file mode 100644 index 00000000..4fce5de9 --- /dev/null +++ b/internal/controller/bucket/policy.go @@ -0,0 +1,192 @@ +package bucket + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/aws" + + "github.com/crossplane/crossplane-runtime/pkg/errors" + "github.com/crossplane/crossplane-runtime/pkg/logging" + + "github.com/linode/provider-ceph/apis/provider-ceph/v1alpha1" + apisv1alpha1 "github.com/linode/provider-ceph/apis/v1alpha1" + "github.com/linode/provider-ceph/internal/backendstore" + "github.com/linode/provider-ceph/internal/consts" + "github.com/linode/provider-ceph/internal/controller/s3clienthandler" + "github.com/linode/provider-ceph/internal/otel/traces" + "github.com/linode/provider-ceph/internal/rgw" + "go.opentelemetry.io/otel" +) + +// PolicyClient is the client for API methods and reconciling the BucketPolicy +type BucketPolicyClient struct { + backendStore *backendstore.BackendStore + s3ClientHandler *s3clienthandler.Handler + log logging.Logger +} + +// NewBucketPolicyClient creates the client for Accelerate Configuration +func NewBucketPolicyClient(b *backendstore.BackendStore, h *s3clienthandler.Handler, l logging.Logger) *BucketPolicyClient { + return &BucketPolicyClient{backendStore: b, s3ClientHandler: h, log: l} +} + +func (p *BucketPolicyClient) Observe(ctx context.Context, bucket *v1alpha1.Bucket, backendNames []string) (ResourceStatus, error) { + ctx, span := otel.Tracer("").Start(ctx, "bucket.BucketPolicyClient.Observe") + defer span.End() + + observationChan := make(chan ResourceStatus) + errChan := make(chan error) + + for _, backendName := range backendNames { + beName := backendName + go func() { + observation, err := p.observeBackend(ctx, bucket, beName) + if err != nil { + errChan <- err + + return + } + observationChan <- observation + }() + } + + for i := 0; i < len(backendNames); i++ { + select { + case <-ctx.Done(): + p.log.Info("Context timeout during bucket policy observation", consts.KeyBucketName, bucket.Name) + err := errors.Wrap(ctx.Err(), errObservePolicy) + traces.SetAndRecordError(span, err) + + return NeedsUpdate, err + case observation := <-observationChan: + if observation != Updated { + return observation, nil + } + case err := <-errChan: + err = errors.Wrap(err, errObservePolicy) + traces.SetAndRecordError(span, err) + + return NeedsUpdate, err + } + } + + return Updated, nil +} + +func (p *BucketPolicyClient) observeBackend(ctx context.Context, bucket *v1alpha1.Bucket, backendName string) (ResourceStatus, error) { + p.log.Info("Observing subresource policy on backend", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName) + + if p.backendStore.GetBackendHealthStatus(backendName) == apisv1alpha1.HealthStatusUnhealthy { + // If a backend is marked as unhealthy, we can ignore it for now by returning Updated. + // The backend may be down for some time and we do not want to block Create/Update/Delete + // calls on other backends. By returning NeedsUpdate here, we would never pass the Observe + // phase until the backend becomes Healthy or Disabled. + return Updated, nil + } + + s3Client, err := p.s3ClientHandler.GetS3Client(ctx, bucket, backendName) + if err != nil { + return NeedsUpdate, err + } + + response, err := rgw.GetBucketPolicy(ctx, s3Client, aws.String(bucket.Name)) + if err != nil { + return NeedsUpdate, err + } + + if bucket.Spec.ForProvider.BucketPolicy == "" { + // No policy config is specified. + // In that case, it should not exist on any backend. + if *response.Policy == "" { + p.log.Info("No bucket policy found on backend - no action required", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName) + + return Updated, nil + } else { + p.log.Info("Bucket policy found on backend - requires deletion", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName) + + return NeedsDeletion, nil + } + } + + // TODO: Ensure how to compare + local := bucket.Spec.ForProvider.BucketPolicy + + external := *response.Policy + + if external != "" && local == "" { + return NeedsUpdate, nil + } + + if local != external { + p.log.Info("Bucket policy requires update on backend", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName) + + return NeedsUpdate, nil + } + + return Updated, nil +} + +func (p *BucketPolicyClient) Handle(ctx context.Context, b *v1alpha1.Bucket, backendName string, bb *bucketBackends) error { + ctx, span := otel.Tracer("").Start(ctx, "bucket.BucketPolicyClient.Handle") + defer span.End() + + observation, err := p.observeBackend(ctx, b, backendName) + if err != nil { + err = errors.Wrap(err, errHandlePolicy) + traces.SetAndRecordError(span, err) + + return err + } + + switch observation { + case Updated: + return nil + case NeedsDeletion: + if err := p.delete(ctx, b, backendName); err != nil { + err = errors.Wrap(err, errHandlePolicy) + + traces.SetAndRecordError(span, err) + + return err + } + case NeedsUpdate: + if err := p.createOrUpdate(ctx, b, backendName); err != nil { + err = errors.Wrap(err, errHandlePolicy) + + traces.SetAndRecordError(span, err) + + return err + } + } + + return nil +} + +func (p *BucketPolicyClient) createOrUpdate(ctx context.Context, b *v1alpha1.Bucket, backendName string) error { + p.log.Info("Updating bucket policy", consts.KeyBucketName, b.Name, consts.KeyBackendName, backendName) + s3Client, err := p.s3ClientHandler.GetS3Client(ctx, b, backendName) + if err != nil { + return err + } + + _, err = rgw.PutBucketPolicy(ctx, s3Client, b) + if err != nil { + return err + } + + return nil +} + +func (p *BucketPolicyClient) delete(ctx context.Context, b *v1alpha1.Bucket, backendName string) error { + p.log.Info("Deleting bucket policy", consts.KeyBucketName, b.Name, consts.KeyBackendName, backendName) + s3Client, err := p.s3ClientHandler.GetS3Client(ctx, b, backendName) + if err != nil { + return err + } + + if err := rgw.DeleteBucketPolicy(ctx, s3Client, aws.String(b.Name)); err != nil { + return err + } + + return nil +} diff --git a/internal/rgw/policy.go b/internal/rgw/policy.go index 2f7eb98e..2dff5202 100644 --- a/internal/rgw/policy.go +++ b/internal/rgw/policy.go @@ -45,16 +45,16 @@ func PutBucketPolicy(ctx context.Context, s3Backend backendstore.S3Client, b *v1 return resp, nil } -func DeleteBucketPolicy(ctx context.Context, s3Backend backendstore.S3Client, bucketName *string) (*awss3.DeleteBucketPolicyOutput, error) { +func DeleteBucketPolicy(ctx context.Context, s3Backend backendstore.S3Client, bucketName *string) error { ctx, span := otel.Tracer("").Start(ctx, "DeleteBucketPolicy") defer span.End() - resp, err := s3Backend.DeleteBucketPolicy(ctx, &awss3.DeleteBucketPolicyInput{Bucket: bucketName}) + _, err := s3Backend.DeleteBucketPolicy(ctx, &awss3.DeleteBucketPolicyInput{Bucket: bucketName}) if err != nil { traces.SetAndRecordError(span, err) - return resp, errors.Wrap(err, errDeleteBucketPolicy) + return errors.Wrap(err, errDeleteBucketPolicy) } - return resp, nil + return nil } From 160b8dfbfeb0cbb88dea6f2e2194569d6a8d68d1 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Mon, 20 May 2024 23:33:39 +0200 Subject: [PATCH 06/13] policy --- internal/controller/bucket/policy.go | 30 ++- internal/controller/bucket/policy_test.go | 307 ++++++++++++++++++++++ 2 files changed, 327 insertions(+), 10 deletions(-) create mode 100644 internal/controller/bucket/policy_test.go diff --git a/internal/controller/bucket/policy.go b/internal/controller/bucket/policy.go index 4fce5de9..d452e432 100644 --- a/internal/controller/bucket/policy.go +++ b/internal/controller/bucket/policy.go @@ -4,6 +4,7 @@ import ( "context" "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/smithy-go" "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/logging" @@ -89,15 +90,23 @@ func (p *BucketPolicyClient) observeBackend(ctx context.Context, bucket *v1alpha return NeedsUpdate, err } + // external keeps the bucket policy in backend. + var external string + response, err := rgw.GetBucketPolicy(ctx, s3Client, aws.String(bucket.Name)) - if err != nil { + // If error is not NoSuchBucketPolicy error, return with the error. + if err != nil && !isNoSuchBucketPolicy(err) { return NeedsUpdate, err } + if response != nil && response.Policy != nil { + external = *response.Policy + } + if bucket.Spec.ForProvider.BucketPolicy == "" { // No policy config is specified. // In that case, it should not exist on any backend. - if *response.Policy == "" { + if external == "" { p.log.Info("No bucket policy found on backend - no action required", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName) return Updated, nil @@ -108,15 +117,7 @@ func (p *BucketPolicyClient) observeBackend(ctx context.Context, bucket *v1alpha } } - // TODO: Ensure how to compare local := bucket.Spec.ForProvider.BucketPolicy - - external := *response.Policy - - if external != "" && local == "" { - return NeedsUpdate, nil - } - if local != external { p.log.Info("Bucket policy requires update on backend", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName) @@ -190,3 +191,12 @@ func (p *BucketPolicyClient) delete(ctx context.Context, b *v1alpha1.Bucket, bac return nil } + +func isNoSuchBucketPolicy(err error) bool { + var ae smithy.APIError + if !errors.As(err, &ae) { + return false + } + + return ae != nil && ae.ErrorCode() == "NoSuchBucketPolicy" +} diff --git a/internal/controller/bucket/policy_test.go b/internal/controller/bucket/policy_test.go new file mode 100644 index 00000000..8b50071f --- /dev/null +++ b/internal/controller/bucket/policy_test.go @@ -0,0 +1,307 @@ +package bucket + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/smithy-go" + "github.com/crossplane/crossplane-runtime/pkg/errors" + "github.com/crossplane/crossplane-runtime/pkg/logging" + "github.com/linode/provider-ceph/apis/provider-ceph/v1alpha1" + apisv1alpha1 "github.com/linode/provider-ceph/apis/v1alpha1" + "github.com/linode/provider-ceph/internal/backendstore" + "github.com/linode/provider-ceph/internal/backendstore/backendstorefakes" + "github.com/linode/provider-ceph/internal/controller/s3clienthandler" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var ( + s3Err = errors.New("some error") + samplePolicy = `{"Policy": "{\"Version\": \"2012-10-17\", \"Statement\": [{\"Action\": [\"*\"], \"Principal\": \"*\", \"Sid\": \"\", \"Effect\": \"Allow\", \"Resource\": \"arn:aws:s3:::shunsuke-rgw-acl-test/*\"},]}"}` +) + +func TestPolicyObserveBackend(t *testing.T) { + t.Parallel() + + type fields struct { + backendStore *backendstore.BackendStore + } + + type args struct { + bucket *v1alpha1.Bucket + backendName string + } + + type want struct { + status ResourceStatus + err error + } + + cases := map[string]struct { + reason string + fields fields + args args + want want + }{ + "Attempt to observe policy on unhealthy backend": { + fields: fields{ + backendStore: func() *backendstore.BackendStore { + fake := backendstorefakes.FakeS3Client{} + + bs := backendstore.NewBackendStore() + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusUnhealthy) + + return bs + }(), + }, + args: args{ + bucket: &v1alpha1.Bucket{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bucket", + }, + }, + backendName: "s3-backend-1", + }, + want: want{ + status: Updated, + }, + }, + + "s3 error": { + fields: fields{ + backendStore: func() *backendstore.BackendStore { + fake := backendstorefakes.FakeS3Client{ + GetBucketPolicyStub: func(ctx context.Context, in *s3.GetBucketPolicyInput, f ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) { + return nil, s3Err + }, + } + + bs := backendstore.NewBackendStore() + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + + return bs + }(), + }, + args: args{ + bucket: &v1alpha1.Bucket{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bucket", + }, + }, + backendName: "s3-backend-1", + }, + want: want{ + status: NeedsUpdate, + err: s3Err, + }, + }, + "ok - policy is updated": { + fields: fields{ + backendStore: func() *backendstore.BackendStore { + fake := backendstorefakes.FakeS3Client{ + GetBucketPolicyStub: func(ctx context.Context, in *s3.GetBucketPolicyInput, f ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) { + return &s3.GetBucketPolicyOutput{Policy: aws.String(samplePolicy)}, nil + }, + } + + bs := backendstore.NewBackendStore() + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + + return bs + }(), + }, + args: args{ + bucket: &v1alpha1.Bucket{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bucket", + }, + Spec: v1alpha1.BucketSpec{ + ForProvider: v1alpha1.BucketParameters{ + BucketPolicy: samplePolicy, + }, + }, + }, + backendName: "s3-backend-1", + }, + want: want{ + status: Updated, + }, + }, + "ok - no bucket policy in backend, but bucket CR has": { + fields: fields{ + backendStore: func() *backendstore.BackendStore { + fake := backendstorefakes.FakeS3Client{ + GetBucketPolicyStub: func(ctx context.Context, in *s3.GetBucketPolicyInput, f ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) { + return nil, &smithy.GenericAPIError{Code: "NoSuchBucketPolicy", Message: "no policy"} + }, + } + + bs := backendstore.NewBackendStore() + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + + return bs + }(), + }, + args: args{ + bucket: &v1alpha1.Bucket{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bucket", + }, + Spec: v1alpha1.BucketSpec{ + ForProvider: v1alpha1.BucketParameters{ + BucketPolicy: samplePolicy, + }, + }, + }, + backendName: "s3-backend-1", + }, + want: want{ + status: NeedsUpdate, + }, + }, + "ok - no bucket policy both in backend and bucket cr": { + fields: fields{ + backendStore: func() *backendstore.BackendStore { + fake := backendstorefakes.FakeS3Client{ + GetBucketPolicyStub: func(ctx context.Context, in *s3.GetBucketPolicyInput, f ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) { + return nil, &smithy.GenericAPIError{Code: "NoSuchBucketPolicy", Message: "no policy"} + }, + } + + bs := backendstore.NewBackendStore() + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + + return bs + }(), + }, + args: args{ + bucket: &v1alpha1.Bucket{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bucket", + }, + }, + backendName: "s3-backend-1", + }, + want: want{ + status: Updated, + }, + }, + "ok - policy is returned, but empty": { + fields: fields{ + backendStore: func() *backendstore.BackendStore { + emptyPolicy := "" + fake := backendstorefakes.FakeS3Client{ + GetBucketPolicyStub: func(ctx context.Context, in *s3.GetBucketPolicyInput, f ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) { + return &s3.GetBucketPolicyOutput{Policy: aws.String(emptyPolicy)}, nil + }, + } + + bs := backendstore.NewBackendStore() + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + + return bs + }(), + }, + args: args{ + bucket: &v1alpha1.Bucket{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bucket", + }, + Spec: v1alpha1.BucketSpec{ + ForProvider: v1alpha1.BucketParameters{ + BucketPolicy: samplePolicy, + }, + }, + }, + backendName: "s3-backend-1", + }, + want: want{ + status: NeedsUpdate, + }, + }, + "ok - backend has policy, but bucket cr doesn't": { + fields: fields{ + backendStore: func() *backendstore.BackendStore { + fake := backendstorefakes.FakeS3Client{ + GetBucketPolicyStub: func(ctx context.Context, in *s3.GetBucketPolicyInput, f ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) { + return &s3.GetBucketPolicyOutput{Policy: aws.String(samplePolicy)}, nil + }, + } + + bs := backendstore.NewBackendStore() + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + + return bs + }(), + }, + args: args{ + bucket: &v1alpha1.Bucket{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bucket", + }, + }, + backendName: "s3-backend-1", + }, + want: want{ + status: NeedsDeletion, + }, + }, + "ok - backend has different policy": { + fields: fields{ + backendStore: func() *backendstore.BackendStore { + differentPolicy := "{}" + fake := backendstorefakes.FakeS3Client{ + GetBucketPolicyStub: func(ctx context.Context, in *s3.GetBucketPolicyInput, f ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) { + return &s3.GetBucketPolicyOutput{Policy: aws.String(differentPolicy)}, nil + }, + } + + bs := backendstore.NewBackendStore() + bs.AddOrUpdateBackend("s3-backend-1", &fake, nil, true, apisv1alpha1.HealthStatusHealthy) + + return bs + }(), + }, + args: args{ + bucket: &v1alpha1.Bucket{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bucket", + }, + Spec: v1alpha1.BucketSpec{ + ForProvider: v1alpha1.BucketParameters{ + BucketPolicy: samplePolicy, + }, + }, + }, + backendName: "s3-backend-1", + }, + want: want{ + status: NeedsUpdate, + }, + }, + } + + for name, tc := range cases { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + c := NewBucketPolicyClient( + tc.fields.backendStore, + s3clienthandler.NewHandler( + s3clienthandler.WithAssumeRoleArn(nil), + s3clienthandler.WithBackendStore(tc.fields.backendStore), + ), + logging.NewNopLogger(), + ) + + got, err := c.observeBackend(context.Background(), tc.args.bucket, tc.args.backendName) + require.ErrorIs(t, err, tc.want.err, "unexpected error") + assert.Equal(t, tc.want.status, got, "unexpected status") + }) + } +} From abdc1e5a1705cfc1597fa1b87a011aafb87f1996 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Mon, 20 May 2024 23:41:37 +0200 Subject: [PATCH 07/13] lint --- internal/controller/bucket/lifecycleconfiguration.go | 1 + internal/controller/bucket/policy.go | 1 + internal/controller/bucket/policy_test.go | 7 ++++--- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/controller/bucket/lifecycleconfiguration.go b/internal/controller/bucket/lifecycleconfiguration.go index ba86c869..2a4355a0 100644 --- a/internal/controller/bucket/lifecycleconfiguration.go +++ b/internal/controller/bucket/lifecycleconfiguration.go @@ -37,6 +37,7 @@ func NewLifecycleConfigurationClient(b *backendstore.BackendStore, h *s3clientha return &LifecycleConfigurationClient{backendStore: b, s3ClientHandler: h, log: l} } +//nolint:dupl // LifecycleConfiguration and BucketPolicy are different feature. func (l *LifecycleConfigurationClient) Observe(ctx context.Context, bucket *v1alpha1.Bucket, backendNames []string) (ResourceStatus, error) { ctx, span := otel.Tracer("").Start(ctx, "bucket.LifecycleConfigurationClient.Observe") defer span.End() diff --git a/internal/controller/bucket/policy.go b/internal/controller/bucket/policy.go index d452e432..ba83ebb0 100644 --- a/internal/controller/bucket/policy.go +++ b/internal/controller/bucket/policy.go @@ -31,6 +31,7 @@ func NewBucketPolicyClient(b *backendstore.BackendStore, h *s3clienthandler.Hand return &BucketPolicyClient{backendStore: b, s3ClientHandler: h, log: l} } +//nolint:dupl // LifecycleConfiguration and BucketPolicy are different feature. func (p *BucketPolicyClient) Observe(ctx context.Context, bucket *v1alpha1.Bucket, backendNames []string) (ResourceStatus, error) { ctx, span := otel.Tracer("").Start(ctx, "bucket.BucketPolicyClient.Observe") defer span.End() diff --git a/internal/controller/bucket/policy_test.go b/internal/controller/bucket/policy_test.go index 8b50071f..a97464ab 100644 --- a/internal/controller/bucket/policy_test.go +++ b/internal/controller/bucket/policy_test.go @@ -20,10 +20,11 @@ import ( ) var ( - s3Err = errors.New("some error") + errS3 = errors.New("some error") samplePolicy = `{"Policy": "{\"Version\": \"2012-10-17\", \"Statement\": [{\"Action\": [\"*\"], \"Principal\": \"*\", \"Sid\": \"\", \"Effect\": \"Allow\", \"Resource\": \"arn:aws:s3:::shunsuke-rgw-acl-test/*\"},]}"}` ) +//nolint:maintidx // for testing func TestPolicyObserveBackend(t *testing.T) { t.Parallel() @@ -76,7 +77,7 @@ func TestPolicyObserveBackend(t *testing.T) { backendStore: func() *backendstore.BackendStore { fake := backendstorefakes.FakeS3Client{ GetBucketPolicyStub: func(ctx context.Context, in *s3.GetBucketPolicyInput, f ...func(*s3.Options)) (*s3.GetBucketPolicyOutput, error) { - return nil, s3Err + return nil, errS3 }, } @@ -96,7 +97,7 @@ func TestPolicyObserveBackend(t *testing.T) { }, want: want{ status: NeedsUpdate, - err: s3Err, + err: errS3, }, }, "ok - policy is updated": { From 9557b41a9a17d064e231a8295bdb27ca93c41d8e Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Mon, 20 May 2024 23:44:21 +0200 Subject: [PATCH 08/13] tidy --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index f032919c..5d9b71db 100644 --- a/go.mod +++ b/go.mod @@ -84,7 +84,6 @@ require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.20 // indirect github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect - github.com/maxbrunsfeld/counterfeiter/v6 v6.8.1 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect diff --git a/go.sum b/go.sum index 831f9e19..aeacd325 100644 --- a/go.sum +++ b/go.sum @@ -143,8 +143,6 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg= github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k= -github.com/maxbrunsfeld/counterfeiter/v6 v6.8.1 h1:NicmruxkeqHjDv03SfSxqmaLuisddudfP3h5wdXFbhM= -github.com/maxbrunsfeld/counterfeiter/v6 v6.8.1/go.mod h1:eyp4DdUJAKkr9tvxR3jWhw2mDK7CWABMG5r9uyaKC7I= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= From a0f1bddb3e36d0fd5ae192d78ad5085ed109be85 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Tue, 21 May 2024 12:39:45 +0200 Subject: [PATCH 09/13] remove unnecessary prefix 'Bucket' --- apis/provider-ceph/v1alpha1/bucket_types.go | 4 +-- internal/controller/bucket/consts.go | 2 +- .../bucket/lifecycleconfiguration.go | 2 +- internal/controller/bucket/policy.go | 30 +++++++++---------- internal/controller/bucket/policy_test.go | 10 +++---- internal/rgw/policy.go | 2 +- ...vider-ceph.ceph.crossplane.io_buckets.yaml | 12 ++++---- 7 files changed, 31 insertions(+), 31 deletions(-) diff --git a/apis/provider-ceph/v1alpha1/bucket_types.go b/apis/provider-ceph/v1alpha1/bucket_types.go index 44821345..0f7f760c 100644 --- a/apis/provider-ceph/v1alpha1/bucket_types.go +++ b/apis/provider-ceph/v1alpha1/bucket_types.go @@ -86,11 +86,11 @@ type BucketParameters struct { // +optional AssumeRoleTags []Tag `json:"assumeRoleTags,omitempty"` - // BucketPolicy is a JSON string of BucketPolicy. + // Policy is a JSON string of BucketPolicy. // If it is set, Provider-Ceph calls PutBucketPolicy API after creating the bucket. // Before adding it, you should validate the JSON string. // +optional - BucketPolicy string `json:"bucketPolicy,omitempty"` + Policy string `json:"policy,omitempty"` } // BackendInfo contains relevant information about an S3 backend for diff --git a/internal/controller/bucket/consts.go b/internal/controller/bucket/consts.go index 4325b871..8ab0bfef 100644 --- a/internal/controller/bucket/consts.go +++ b/internal/controller/bucket/consts.go @@ -24,7 +24,7 @@ const ( errObserveAcl = "failed to observe bucket acl" errHandleAcl = "failed to handle bucket acl" - // BucketPolicy error messages. + // Policy error messages. errObservePolicy = "failed to observe bucket policy" errHandlePolicy = "failed to handle bucket policy" diff --git a/internal/controller/bucket/lifecycleconfiguration.go b/internal/controller/bucket/lifecycleconfiguration.go index 2a4355a0..d8948505 100644 --- a/internal/controller/bucket/lifecycleconfiguration.go +++ b/internal/controller/bucket/lifecycleconfiguration.go @@ -37,7 +37,7 @@ func NewLifecycleConfigurationClient(b *backendstore.BackendStore, h *s3clientha return &LifecycleConfigurationClient{backendStore: b, s3ClientHandler: h, log: l} } -//nolint:dupl // LifecycleConfiguration and BucketPolicy are different feature. +//nolint:dupl // LifecycleConfiguration and Policy are different feature. func (l *LifecycleConfigurationClient) Observe(ctx context.Context, bucket *v1alpha1.Bucket, backendNames []string) (ResourceStatus, error) { ctx, span := otel.Tracer("").Start(ctx, "bucket.LifecycleConfigurationClient.Observe") defer span.End() diff --git a/internal/controller/bucket/policy.go b/internal/controller/bucket/policy.go index ba83ebb0..b13dd808 100644 --- a/internal/controller/bucket/policy.go +++ b/internal/controller/bucket/policy.go @@ -19,21 +19,21 @@ import ( "go.opentelemetry.io/otel" ) -// PolicyClient is the client for API methods and reconciling the BucketPolicy -type BucketPolicyClient struct { +// PolicyClient is the client for API methods and reconciling a BucketPolicy +type PolicyClient struct { backendStore *backendstore.BackendStore s3ClientHandler *s3clienthandler.Handler log logging.Logger } -// NewBucketPolicyClient creates the client for Accelerate Configuration -func NewBucketPolicyClient(b *backendstore.BackendStore, h *s3clienthandler.Handler, l logging.Logger) *BucketPolicyClient { - return &BucketPolicyClient{backendStore: b, s3ClientHandler: h, log: l} +// NewPolicyClient creates the client for Accelerate Configuration +func NewPolicyClient(b *backendstore.BackendStore, h *s3clienthandler.Handler, l logging.Logger) *PolicyClient { + return &PolicyClient{backendStore: b, s3ClientHandler: h, log: l} } -//nolint:dupl // LifecycleConfiguration and BucketPolicy are different feature. -func (p *BucketPolicyClient) Observe(ctx context.Context, bucket *v1alpha1.Bucket, backendNames []string) (ResourceStatus, error) { - ctx, span := otel.Tracer("").Start(ctx, "bucket.BucketPolicyClient.Observe") +//nolint:dupl // LifecycleConfiguration and Policy are different feature. +func (p *PolicyClient) Observe(ctx context.Context, bucket *v1alpha1.Bucket, backendNames []string) (ResourceStatus, error) { + ctx, span := otel.Tracer("").Start(ctx, "bucket.PolicyClient.Observe") defer span.End() observationChan := make(chan ResourceStatus) @@ -75,7 +75,7 @@ func (p *BucketPolicyClient) Observe(ctx context.Context, bucket *v1alpha1.Bucke return Updated, nil } -func (p *BucketPolicyClient) observeBackend(ctx context.Context, bucket *v1alpha1.Bucket, backendName string) (ResourceStatus, error) { +func (p *PolicyClient) observeBackend(ctx context.Context, bucket *v1alpha1.Bucket, backendName string) (ResourceStatus, error) { p.log.Info("Observing subresource policy on backend", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName) if p.backendStore.GetBackendHealthStatus(backendName) == apisv1alpha1.HealthStatusUnhealthy { @@ -104,7 +104,7 @@ func (p *BucketPolicyClient) observeBackend(ctx context.Context, bucket *v1alpha external = *response.Policy } - if bucket.Spec.ForProvider.BucketPolicy == "" { + if bucket.Spec.ForProvider.Policy == "" { // No policy config is specified. // In that case, it should not exist on any backend. if external == "" { @@ -118,7 +118,7 @@ func (p *BucketPolicyClient) observeBackend(ctx context.Context, bucket *v1alpha } } - local := bucket.Spec.ForProvider.BucketPolicy + local := bucket.Spec.ForProvider.Policy if local != external { p.log.Info("Bucket policy requires update on backend", consts.KeyBucketName, bucket.Name, consts.KeyBackendName, backendName) @@ -128,8 +128,8 @@ func (p *BucketPolicyClient) observeBackend(ctx context.Context, bucket *v1alpha return Updated, nil } -func (p *BucketPolicyClient) Handle(ctx context.Context, b *v1alpha1.Bucket, backendName string, bb *bucketBackends) error { - ctx, span := otel.Tracer("").Start(ctx, "bucket.BucketPolicyClient.Handle") +func (p *PolicyClient) Handle(ctx context.Context, b *v1alpha1.Bucket, backendName string, bb *bucketBackends) error { + ctx, span := otel.Tracer("").Start(ctx, "bucket.PolicyClient.Handle") defer span.End() observation, err := p.observeBackend(ctx, b, backendName) @@ -164,7 +164,7 @@ func (p *BucketPolicyClient) Handle(ctx context.Context, b *v1alpha1.Bucket, bac return nil } -func (p *BucketPolicyClient) createOrUpdate(ctx context.Context, b *v1alpha1.Bucket, backendName string) error { +func (p *PolicyClient) createOrUpdate(ctx context.Context, b *v1alpha1.Bucket, backendName string) error { p.log.Info("Updating bucket policy", consts.KeyBucketName, b.Name, consts.KeyBackendName, backendName) s3Client, err := p.s3ClientHandler.GetS3Client(ctx, b, backendName) if err != nil { @@ -179,7 +179,7 @@ func (p *BucketPolicyClient) createOrUpdate(ctx context.Context, b *v1alpha1.Buc return nil } -func (p *BucketPolicyClient) delete(ctx context.Context, b *v1alpha1.Bucket, backendName string) error { +func (p *PolicyClient) delete(ctx context.Context, b *v1alpha1.Bucket, backendName string) error { p.log.Info("Deleting bucket policy", consts.KeyBucketName, b.Name, consts.KeyBackendName, backendName) s3Client, err := p.s3ClientHandler.GetS3Client(ctx, b, backendName) if err != nil { diff --git a/internal/controller/bucket/policy_test.go b/internal/controller/bucket/policy_test.go index a97464ab..558162bc 100644 --- a/internal/controller/bucket/policy_test.go +++ b/internal/controller/bucket/policy_test.go @@ -122,7 +122,7 @@ func TestPolicyObserveBackend(t *testing.T) { }, Spec: v1alpha1.BucketSpec{ ForProvider: v1alpha1.BucketParameters{ - BucketPolicy: samplePolicy, + Policy: samplePolicy, }, }, }, @@ -154,7 +154,7 @@ func TestPolicyObserveBackend(t *testing.T) { }, Spec: v1alpha1.BucketSpec{ ForProvider: v1alpha1.BucketParameters{ - BucketPolicy: samplePolicy, + Policy: samplePolicy, }, }, }, @@ -214,7 +214,7 @@ func TestPolicyObserveBackend(t *testing.T) { }, Spec: v1alpha1.BucketSpec{ ForProvider: v1alpha1.BucketParameters{ - BucketPolicy: samplePolicy, + Policy: samplePolicy, }, }, }, @@ -274,7 +274,7 @@ func TestPolicyObserveBackend(t *testing.T) { }, Spec: v1alpha1.BucketSpec{ ForProvider: v1alpha1.BucketParameters{ - BucketPolicy: samplePolicy, + Policy: samplePolicy, }, }, }, @@ -291,7 +291,7 @@ func TestPolicyObserveBackend(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - c := NewBucketPolicyClient( + c := NewPolicyClient( tc.fields.backendStore, s3clienthandler.NewHandler( s3clienthandler.WithAssumeRoleArn(nil), diff --git a/internal/rgw/policy.go b/internal/rgw/policy.go index 2dff5202..c166d133 100644 --- a/internal/rgw/policy.go +++ b/internal/rgw/policy.go @@ -35,7 +35,7 @@ func PutBucketPolicy(ctx context.Context, s3Backend backendstore.S3Client, b *v1 ctx, span := otel.Tracer("").Start(ctx, "PutBucketPolicy") defer span.End() - resp, err := s3Backend.PutBucketPolicy(ctx, &awss3.PutBucketPolicyInput{Bucket: &b.Name, Policy: &b.Spec.ForProvider.BucketPolicy}) + resp, err := s3Backend.PutBucketPolicy(ctx, &awss3.PutBucketPolicyInput{Bucket: &b.Name, Policy: &b.Spec.ForProvider.Policy}) if err != nil { traces.SetAndRecordError(span, err) diff --git a/package/crds/provider-ceph.ceph.crossplane.io_buckets.yaml b/package/crds/provider-ceph.ceph.crossplane.io_buckets.yaml index 32113c9a..dadcef1b 100644 --- a/package/crds/provider-ceph.ceph.crossplane.io_buckets.yaml +++ b/package/crds/provider-ceph.ceph.crossplane.io_buckets.yaml @@ -179,12 +179,6 @@ spec: - value type: object type: array - bucketPolicy: - description: |- - BucketPolicy is a JSON string of BucketPolicy. - If it is set, Provider-Ceph calls PutBucketPolicy API after creating the bucket. - Before adding it, you should validate the JSON string. - type: string grantFullControl: description: |- Allows grantee the read, write, read ACP, and write ACP permissions on the @@ -485,6 +479,12 @@ spec: don't specify an ACL or bucket owner full control ACLs, such as the bucket-owner-full-control canned ACL or an equivalent form of this ACL expressed in the XML format. type: string + policy: + description: |- + Policy is a JSON string of BucketPolicy. + If it is set, Provider-Ceph calls PutBucketPolicy API after creating the bucket. + Before adding it, you should validate the JSON string. + type: string type: object lifecycleConfigurationDisabled: description: |- From a065918b56708cd334c0ffba29ccf6e4381aff98 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Tue, 21 May 2024 12:40:17 +0200 Subject: [PATCH 10/13] remove meaningless comments --- internal/controller/bucket/lifecycleconfiguration.go | 1 - internal/controller/bucket/policy.go | 1 - 2 files changed, 2 deletions(-) diff --git a/internal/controller/bucket/lifecycleconfiguration.go b/internal/controller/bucket/lifecycleconfiguration.go index d8948505..88ae3d8e 100644 --- a/internal/controller/bucket/lifecycleconfiguration.go +++ b/internal/controller/bucket/lifecycleconfiguration.go @@ -32,7 +32,6 @@ type LifecycleConfigurationClient struct { log logging.Logger } -// NewLifecycleConfigurationClient creates the client for Accelerate Configuration func NewLifecycleConfigurationClient(b *backendstore.BackendStore, h *s3clienthandler.Handler, l logging.Logger) *LifecycleConfigurationClient { return &LifecycleConfigurationClient{backendStore: b, s3ClientHandler: h, log: l} } diff --git a/internal/controller/bucket/policy.go b/internal/controller/bucket/policy.go index b13dd808..ff4a14e7 100644 --- a/internal/controller/bucket/policy.go +++ b/internal/controller/bucket/policy.go @@ -26,7 +26,6 @@ type PolicyClient struct { log logging.Logger } -// NewPolicyClient creates the client for Accelerate Configuration func NewPolicyClient(b *backendstore.BackendStore, h *s3clienthandler.Handler, l logging.Logger) *PolicyClient { return &PolicyClient{backendStore: b, s3ClientHandler: h, log: l} } From 48d5d4bdbd158135087c43cafc7f78d2c7d0f4d0 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Tue, 21 May 2024 12:42:37 +0200 Subject: [PATCH 11/13] add error messages --- internal/rgw/policy.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/rgw/policy.go b/internal/rgw/policy.go index c166d133..494629ac 100644 --- a/internal/rgw/policy.go +++ b/internal/rgw/policy.go @@ -12,9 +12,9 @@ import ( ) const ( - errGetBucketPolicy = "" - errPutBucketPolicy = "" - errDeleteBucketPolicy = "" + errGetBucketPolicy = "failed to get bucket policy" + errPutBucketPolicy = "failed to put bucket policy" + errDeleteBucketPolicy = "failed to delete bucket policy" ) func GetBucketPolicy(ctx context.Context, s3Backend backendstore.S3Client, bucketName *string) (*awss3.GetBucketPolicyOutput, error) { From 0ee09739f15e5e98d871624aacb736841bf222b2 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Tue, 21 May 2024 15:06:24 +0200 Subject: [PATCH 12/13] add PolicyClient to subresourceClient --- internal/controller/bucket/subresources.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/controller/bucket/subresources.go b/internal/controller/bucket/subresources.go index cc18078b..987950b2 100644 --- a/internal/controller/bucket/subresources.go +++ b/internal/controller/bucket/subresources.go @@ -37,6 +37,7 @@ func NewSubresourceClients(b *backendstore.BackendStore, h *s3clienthandler.Hand return []SubresourceClient{ NewLifecycleConfigurationClient(b, h, l.WithValues("lifecycle-configuration-client", managed.ControllerName(v1alpha1.BucketGroupKind))), NewACLClient(b, h, l.WithValues("acl-client", managed.ControllerName(v1alpha1.BucketGroupKind))), + NewPolicyClient(b, h, l.WithValues("policy-client", managed.ControllerName(v1alpha1.BucketGroupKind))), } } From 393697919db336c82b0dd73086e0485c101d35b0 Mon Sep 17 00:00:00 2001 From: Shunpoco Date: Tue, 21 May 2024 15:07:27 +0200 Subject: [PATCH 13/13] fix code block in DEVELOPMENT --- docs/DEVELOPMENT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 2a6daa33..a8976c78 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -20,7 +20,7 @@ Spin up the test environment, but without Localstack and use your own external C ``` AWS_ACCESS_KEY_ID= AWS_SECRET_ACCESS_KEY= CEPH_ADDRESS= make dev-ceph -` +``` In either case, after you've made some changes, kill (Ctrl+C) the existing `provider-ceph` and re-run it: