From 41c929cb6cc10e0d9eac2625ef58e1751cf9e4d5 Mon Sep 17 00:00:00 2001 From: Susana Garcia Date: Tue, 31 Jan 2023 15:26:58 +0100 Subject: [PATCH 1/4] update pull request template: add reminder to run e2e tests --- .github/PULL_REQUEST_TEMPLATE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index e2119f0c..877f56f5 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -8,6 +8,7 @@ The PR has a meaningful description that sums up the change. It will be linked in the changelog. - [ ] PR contains a single logical change (to build a better changelog). +- [ ] I have run successfully `make test-e2e` locally. - [ ] Update the documentation. - [ ] Categorize the PR by adding one of the labels: `bug`, `enhancement`, `documentation`, `change`, `breaking`, `dependency` From 08d6b563bb79c1b897cf0c7b229986f176a4fdf2 Mon Sep 17 00:00:00 2001 From: Susana Garcia Date: Tue, 31 Jan 2023 15:42:49 +0100 Subject: [PATCH 2/4] s3 seems to use the default retention governance mode, which causes bucket deletion to fail unless bypassing governance retention --- operator/bucketcontroller/delete.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/operator/bucketcontroller/delete.go b/operator/bucketcontroller/delete.go index cf3ab5f8..64368525 100644 --- a/operator/bucketcontroller/delete.go +++ b/operator/bucketcontroller/delete.go @@ -3,6 +3,7 @@ package bucketcontroller import ( "context" "fmt" + pipeline "github.com/ccremer/go-command-pipeline" "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/event" @@ -55,7 +56,9 @@ func (p *ProvisioningPipeline) deleteAllObjects(ctx *pipelineContext) error { } }() - for obj := range p.minioClient.RemoveObjects(ctx, bucketName, objectsCh, minio.RemoveObjectsOptions{GovernanceBypass: true}) { + // s3 seems to use the default retention governance mode, which causes bucket deletion to fail, unless bypassing governance retention + // https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-lock-overview.html + for obj := range p.minioClient.RemoveObjects(ctx, bucketName, objectsCh, minio.RemoveObjectsOptions{GovernanceBypass: false}) { return fmt.Errorf("object %q cannot be removed: %w", obj.ObjectName, obj.Err) } return nil From b750a1017e466e56ba8fa602d30d1ea65fb52992 Mon Sep 17 00:00:00 2001 From: Simon Beck Date: Mon, 6 Feb 2023 12:43:24 +0100 Subject: [PATCH 3/4] Dynamically determine need to bypassGovernance --- operator/bucketcontroller/delete.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/operator/bucketcontroller/delete.go b/operator/bucketcontroller/delete.go index 64368525..5e41565d 100644 --- a/operator/bucketcontroller/delete.go +++ b/operator/bucketcontroller/delete.go @@ -56,14 +56,28 @@ func (p *ProvisioningPipeline) deleteAllObjects(ctx *pipelineContext) error { } }() - // s3 seems to use the default retention governance mode, which causes bucket deletion to fail, unless bypassing governance retention - // https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-lock-overview.html - for obj := range p.minioClient.RemoveObjects(ctx, bucketName, objectsCh, minio.RemoveObjectsOptions{GovernanceBypass: false}) { + bypassGovernance, err := p.isBucketLockEnabled(ctx, bucketName) + if err != nil { + log.Error(err, "not able to determine ObjectLock status for bucket", "bucket", bucketName) + } + + for obj := range p.minioClient.RemoveObjects(ctx, bucketName, objectsCh, minio.RemoveObjectsOptions{GovernanceBypass: bypassGovernance}) { return fmt.Errorf("object %q cannot be removed: %w", obj.ObjectName, obj.Err) } return nil } +func (p *ProvisioningPipeline) isBucketLockEnabled(ctx context.Context, bucketName string) (bool, error) { + _, mode, _, _, err := p.minioClient.GetObjectLockConfig(ctx, bucketName) + if err != nil && err.Error() == "Object Lock configuration does not exist for this bucket" { + return false, nil + } else if err != nil { + return false, err + } + // If there's an objectLockConfig it could still be disabled... + return mode != nil, nil +} + // deleteS3Bucket deletes the bucket. // NOTE: The removal fails if there are still objects in the bucket. // This func does not recursively delete all objects beforehand. From 379a936343cf2e79382419f151e99248d52d2997 Mon Sep 17 00:00:00 2001 From: Simon Beck Date: Mon, 6 Feb 2023 12:56:01 +0100 Subject: [PATCH 4/4] Add target to run specific tests --- test/local.mk | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/local.mk b/test/local.mk index c1cb593e..4e858ffe 100644 --- a/test/local.mk +++ b/test/local.mk @@ -128,6 +128,11 @@ test-e2e: $(kuttl_bin) $(mc_bin) local-install provider-config ## E2E tests @rm -f kubeconfig # kuttl leaves kubeconfig garbage: https://github.com/kudobuilder/kuttl/issues/297 +run-single-e2e: export KUBECONFIG = $(KIND_KUBECONFIG) +run-single-e2e: $(kuttl_bin) $(mc_bin) local-install provider-config ## Run specific e2e test with `run-single-e2e test=$name` + GOBIN=$(go_bin) $(kuttl_bin) test ./test/e2e --config ./test/e2e/kuttl-test.yaml --suppress-log=Events --test $(test) + @rm -f kubeconfig + .PHONY: .e2e-test-clean .e2e-test-clean: export KUBECONFIG = $(KIND_KUBECONFIG) .e2e-test-clean: