Skip to content

Commit

Permalink
policy
Browse files Browse the repository at this point in the history
  • Loading branch information
@Shunpoco
Shunpoco committed May 20, 2024
1 parent 4fb60de commit 160b8df
Show file tree
Hide file tree
Showing 2 changed files with 327 additions and 10 deletions.
30 changes: 20 additions & 10 deletions internal/controller/bucket/policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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"
}
307 changes: 307 additions & 0 deletions internal/controller/bucket/policy_test.go
View file
Original file line number Diff line number Diff line change
@@ -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")
})
}
}

0 comments on commit 160b8df

Please sign in to comment.