From 0d81c42fc70140f500f41fbe503cc696dea17b47 Mon Sep 17 00:00:00 2001 From: Raffael Sahli Date: Mon, 8 Jan 2024 12:23:30 +0100 Subject: [PATCH] fest: add bucket policy --- README.md | 2 +- apis/minio/v1/bucket_types.go | 4 +++ apis/minio/v1/zz_generated.deepcopy.go | 7 ++++- operator/bucket/create.go | 7 +++++ operator/bucket/observe.go | 22 ++++++++++++++- operator/bucket/observe_test.go | 28 +++++++++++++++++++ operator/bucket/update.go | 15 +++++++++- package/crds/minio.crossplane.io_buckets.yaml | 5 ++++ 8 files changed, 86 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index fddd1f7..3ccd8d4 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Crossplane provider for managing resources on min.io. -Documentation: https://vshn.github.io/provider-minio/ +Documentation: https://vshn.github.io/provider-minio/provider-minio/ ## Local Development diff --git a/apis/minio/v1/bucket_types.go b/apis/minio/v1/bucket_types.go index 3f80c60..bca8598 100644 --- a/apis/minio/v1/bucket_types.go +++ b/apis/minio/v1/bucket_types.go @@ -79,6 +79,10 @@ type BucketParameters struct { // `DeleteAll` recursively deletes all objects in the bucket and then removes it. // To skip deletion of the bucket (orphan it) set `spec.deletionPolicy=Orphan`. BucketDeletionPolicy BucketDeletionPolicy `json:"bucketDeletionPolicy,omitempty"` + + // Policy is a raw S3 bucket policy. + // Please consult https://min.io/docs/minio/linux/administration/identity-access-management/policy-based-access-control.html for more details about the policy. + Policy *string `json:"policy,omitempty"` } type BucketProviderStatus struct { diff --git a/apis/minio/v1/zz_generated.deepcopy.go b/apis/minio/v1/zz_generated.deepcopy.go index 8795759..35ad521 100644 --- a/apis/minio/v1/zz_generated.deepcopy.go +++ b/apis/minio/v1/zz_generated.deepcopy.go @@ -71,6 +71,11 @@ func (in *BucketList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BucketParameters) DeepCopyInto(out *BucketParameters) { *out = *in + if in.Policy != nil { + in, out := &in.Policy, &out.Policy + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BucketParameters. @@ -102,7 +107,7 @@ func (in *BucketProviderStatus) DeepCopy() *BucketProviderStatus { func (in *BucketSpec) DeepCopyInto(out *BucketSpec) { *out = *in in.ResourceSpec.DeepCopyInto(&out.ResourceSpec) - out.ForProvider = in.ForProvider + in.ForProvider.DeepCopyInto(&out.ForProvider) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BucketSpec. diff --git a/operator/bucket/create.go b/operator/bucket/create.go index 2cb288e..485ac92 100644 --- a/operator/bucket/create.go +++ b/operator/bucket/create.go @@ -25,6 +25,13 @@ func (b *bucketClient) Create(ctx context.Context, mg resource.Managed) (managed return managed.ExternalCreation{}, err } + if bucket.Spec.ForProvider.Policy != nil { + err = b.mc.SetBucketPolicy(ctx, bucket.GetBucketName(), *bucket.Spec.ForProvider.Policy) + if err != nil { + return managed.ExternalCreation{}, err + } + } + b.setLock(bucket) return managed.ExternalCreation{}, b.emitCreationEvent(bucket) diff --git a/operator/bucket/observe.go b/operator/bucket/observe.go index f286e5f..e0340f6 100644 --- a/operator/bucket/observe.go +++ b/operator/bucket/observe.go @@ -18,6 +18,15 @@ var bucketExistsFn = func(ctx context.Context, mc *minio.Client, bucketName stri return mc.BucketExists(ctx, bucketName) } +var bucketPolicyLatestFn = func(ctx context.Context, mc *minio.Client, bucketName string, policy string) (bool, error) { + current, err := mc.GetBucketPolicy(ctx, bucketName) + if err != nil { + return false, err + } + + return current == policy, nil +} + func (d *bucketClient) Observe(ctx context.Context, mg resource.Managed) (managed.ExternalObservation, error) { log := controllerruntime.LoggerFrom(ctx) log.V(1).Info("observing resource") @@ -45,7 +54,18 @@ func (d *bucketClient) Observe(ctx context.Context, mg resource.Managed) (manage if _, hasAnnotation := bucket.GetAnnotations()[lockAnnotation]; hasAnnotation && exists { bucket.Status.AtProvider.BucketName = bucketName bucket.SetConditions(xpv1.Available()) - return managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil + + isLatest := true + if bucket.Spec.ForProvider.Policy != nil { + u, err := bucketPolicyLatestFn(ctx, d.mc, bucketName, *bucket.Spec.ForProvider.Policy) + if err != nil { + return managed.ExternalObservation{}, errors.Wrap(err, "cannot determine whether a bucket policy exists") + } + + isLatest = u + } + + return managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: isLatest}, nil } else if exists { return managed.ExternalObservation{}, fmt.Errorf("bucket already exists, try changing bucket name: %s", bucketName) } diff --git a/operator/bucket/observe_test.go b/operator/bucket/observe_test.go index 8ec77c5..f3517c7 100644 --- a/operator/bucket/observe_test.go +++ b/operator/bucket/observe_test.go @@ -16,10 +16,12 @@ import ( ) func TestProvisioningPipeline_Observe(t *testing.T) { + policy := "policy-struct" tests := map[string]struct { givenBucket *miniov1.Bucket bucketExists bool returnError error + policyLatest bool expectedError string expectedResult managed.ExternalObservation @@ -31,6 +33,13 @@ func TestProvisioningPipeline_Observe(t *testing.T) { }, expectedResult: managed.ExternalObservation{}, }, + "NewBucketWithPolicyDoesntYetExistOnMinio": { + givenBucket: &miniov1.Bucket{Spec: miniov1.BucketSpec{ForProvider: miniov1.BucketParameters{ + BucketName: "my-bucket-with-policy", + Policy: &policy}}, + }, + expectedResult: managed.ExternalObservation{}, + }, "BucketExistsAndAccessibleWithOurCredentials": { givenBucket: &miniov1.Bucket{ ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ @@ -80,6 +89,20 @@ func TestProvisioningPipeline_Observe(t *testing.T) { expectedResult: managed.ExternalObservation{}, expectedError: "mismatching endpointURL and zone, or bucket exists already in a different region, try changing bucket name: 301 Moved Permanently", }, + "BucketPolicyNoChangeRequired": { + givenBucket: &miniov1.Bucket{ + ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ + lockAnnotation: "claimed", + }}, + Spec: miniov1.BucketSpec{ForProvider: miniov1.BucketParameters{ + BucketName: "my-bucket", + Policy: &policy}}, + }, + policyLatest: true, + bucketExists: true, + expectedResult: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, + expectedBucketObservation: miniov1.BucketProviderStatus{BucketName: "my-bucket"}, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -87,6 +110,11 @@ func TestProvisioningPipeline_Observe(t *testing.T) { defer func() { bucketExistsFn = currFn }() + + bucketPolicyLatestFn = func(ctx context.Context, mc *minio.Client, bucketName string, policy string) (bool, error) { + return tc.policyLatest, tc.returnError + } + bucketExistsFn = func(ctx context.Context, mc *minio.Client, bucketName string) (bool, error) { return tc.bucketExists, tc.returnError } diff --git a/operator/bucket/update.go b/operator/bucket/update.go index 44a8fde..8d00367 100644 --- a/operator/bucket/update.go +++ b/operator/bucket/update.go @@ -5,12 +5,25 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" "github.com/crossplane/crossplane-runtime/pkg/resource" + miniov1 "github.com/vshn/provider-minio/apis/minio/v1" controllerruntime "sigs.k8s.io/controller-runtime" ) -func (d *bucketClient) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) { +func (b *bucketClient) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) { log := controllerruntime.LoggerFrom(ctx) log.V(1).Info("updating resource") + bucket, ok := mg.(*miniov1.Bucket) + if !ok { + return managed.ExternalUpdate{}, errNotBucket + } + + if bucket.Spec.ForProvider.Policy != nil { + err := b.mc.SetBucketPolicy(ctx, bucket.GetBucketName(), *bucket.Spec.ForProvider.Policy) + if err != nil { + return managed.ExternalUpdate{}, err + } + } + return managed.ExternalUpdate{}, nil } diff --git a/package/crds/minio.crossplane.io_buckets.yaml b/package/crds/minio.crossplane.io_buckets.yaml index dd193a2..f20b94b 100644 --- a/package/crds/minio.crossplane.io_buckets.yaml +++ b/package/crds/minio.crossplane.io_buckets.yaml @@ -86,6 +86,11 @@ spec: follows RFC 1123. Be aware that S3 providers may require a unique name across the platform or zone. type: string + policy: + description: Policy is a raw S3 bucket policy. Please consult + https://min.io/docs/minio/linux/administration/identity-access-management/policy-based-access-control.html + for more details about the policy. + type: string region: default: us-east-1 description: Region is the name of the region where the bucket