fix(object): acl: revert bucket acl to private on resource deletion

go.mod

@@ -2,6 +2,7 @@ module github.com/scaleway/terraform-provider-scaleway/v2
 
 require (
 	github.com/aws/aws-sdk-go v1.51.19
+	github.com/aws/aws-sdk-go-v2/service/s3 v1.53.1
 	github.com/docker/docker v26.0.0+incompatible
 	github.com/dustin/go-humanize v1.0.1
 	github.com/google/go-cmp v0.6.0
@@ -28,6 +29,7 @@ require (
 	github.com/ProtonMail/go-crypto v1.1.0-alpha.0 // indirect
 	github.com/agext/levenshtein v1.2.3 // indirect
 	github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect
+	github.com/aws/smithy-go v1.20.2 // indirect
 	github.com/cloudflare/circl v1.3.7 // indirect
 	github.com/containerd/log v0.1.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect

go.sum

@@ -14,6 +14,10 @@ github.com/apparentlymart/go-textseg/v15 v15.0.0/go.mod h1:K8XmNZdhEBkdlyDdvbmms
 github.com/aws/aws-sdk-go v1.31.9/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0=
 github.com/aws/aws-sdk-go v1.51.19 h1:jp/Vx/mUpXttthvvo/4/Nn/3+zumirIlAFkp1Irf1kM=
 github.com/aws/aws-sdk-go v1.51.19/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk=
+github.com/aws/aws-sdk-go-v2/service/s3 v1.53.1 h1:6cnno47Me9bRykw9AEv9zkXE+5or7jz8TsskTTccbgc=
+github.com/aws/aws-sdk-go-v2/service/s3 v1.53.1/go.mod h1:qmdkIIAC+GCLASF7R2whgNrJADz0QZPX+Seiw/i4S3o=
+github.com/aws/smithy-go v1.20.2 h1:tbp628ireGtzcHDDmLT/6ADHidqnwgF57XOXZe6tp4Q=
+github.com/aws/smithy-go v1.20.2/go.mod h1:krry+ya/rV9RDcV/Q16kpu6ypI4K2czasz0NC3qS14E=
 github.com/bufbuild/protocompile v0.4.0 h1:LbFKd2XowZvQ/kajzguUp2DC9UEIQhIq77fZZlaQsNA=
 github.com/bufbuild/protocompile v0.4.0/go.mod h1:3v93+mbWn/v3xzN+31nwkJfrEpAUwp+BagBSZWx+TP8=
 github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM=

internal/services/object/bucket.go

@@ -48,7 +48,7 @@ func ResourceBucket() *schema.Resource {
 				Type:        schema.TypeString,
 				Optional:    true,
 				Default:     "private",
-				Description: "ACL of the bucket: either 'public-read' or 'private'.",
+				Description: "ACL of the bucket: either 'private', 'public-read', 'public-read-write' or 'authenticated-read'.",
 				ValidateFunc: validation.StringInSlice([]string{
 					s3.ObjectCannedACLPrivate,
 					s3.ObjectCannedACLPublicRead,
@@ -470,7 +470,7 @@ func resourceObjectBucketRead(ctx context.Context, d *schema.ResourceData, m int
 			return diags
 		}
 	} else if acl != nil && acl.Owner != nil {
-		_ = d.Set("project_id", normalizeOwnerID(acl.Owner.ID))
+		_ = d.Set("project_id", NormalizeOwnerID(acl.Owner.ID))
 	}
 
 	// Get object_lock_enabled

internal/services/object/bucket_acl.go

@@ -112,7 +112,7 @@ func ResourceBucketACL() *schema.Resource {
 			"acl": {
 				Type:        schema.TypeString,
 				Optional:    true,
-				Description: "ACL of the bucket: either 'public-read' or 'private'.",
+				Description: "ACL of the bucket: either 'private', 'public-read', 'public-read-write' or 'authenticated-read'.",
 				ValidateFunc: validation.StringInSlice([]string{
 					s3.ObjectCannedACLPrivate,
 					s3.ObjectCannedACLPublicRead,
@@ -333,11 +333,11 @@ func flattenBucketACLAccessControlPolicyGrantsGrantee(grantee *s3.Grantee) []int
 	m := make(map[string]interface{})
 
 	if grantee.DisplayName != nil {
-		m["display_name"] = aws.StringValue(normalizeOwnerID(grantee.DisplayName))
+		m["display_name"] = aws.StringValue(NormalizeOwnerID(grantee.DisplayName))
 	}
 
 	if grantee.ID != nil {
-		m["id"] = aws.StringValue(normalizeOwnerID(grantee.ID))
+		m["id"] = aws.StringValue(NormalizeOwnerID(grantee.ID))
 	}
 
 	if grantee.Type != nil {
@@ -355,11 +355,11 @@ func flattenBucketACLAccessControlPolicyOwner(owner *s3.Owner) []interface{} {
 	m := make(map[string]interface{})
 
 	if owner.DisplayName != nil {
-		m["display_name"] = aws.StringValue(normalizeOwnerID(owner.DisplayName))
+		m["display_name"] = aws.StringValue(NormalizeOwnerID(owner.DisplayName))
 	}
 
 	if owner.ID != nil {
-		m["id"] = aws.StringValue(normalizeOwnerID(owner.ID))
+		m["id"] = aws.StringValue(NormalizeOwnerID(owner.ID))
 	}
 
 	return []interface{}{m}
@@ -402,7 +402,7 @@ func resourceBucketACLRead(ctx context.Context, d *schema.ResourceData, m interf
 		return diag.FromErr(fmt.Errorf("error setting access_control_policy: %w", err))
 	}
 	_ = d.Set("region", region)
-	_ = d.Set("project_id", normalizeOwnerID(output.Owner.ID))
+	_ = d.Set("project_id", NormalizeOwnerID(output.Owner.ID))
 	_ = d.Set("bucket", locality.ExpandID(bucket))
 
 	return nil
@@ -453,7 +453,25 @@ func resourceBucketACLUpdate(ctx context.Context, d *schema.ResourceData, m inte
 	return resourceBucketACLRead(ctx, d, m)
 }
 
-func resourceBucketACLDelete(ctx context.Context, _ *schema.ResourceData, _ interface{}) diag.Diagnostics {
-	tflog.Warn(ctx, "[WARN] Cannot destroy Object Bucket ACL. Terraform will remove this resource from the state file, however resources may remain.")
-	return nil
+func resourceBucketACLDelete(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
+	conn, _, bucket, _, err := s3ClientWithRegionWithNameACL(d, m, d.Id())
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	_, err = conn.PutBucketAclWithContext(ctx, &s3.PutBucketAclInput{
+		Bucket: &bucket,
+		ACL:    scw.StringPtr(s3.ObjectCannedACLPrivate),
+	})
+	if err != nil {
+		return diag.FromErr(fmt.Errorf("error putting bucket ACL: %w", err))
+	}
+
+	return diag.Diagnostics{
+		diag.Diagnostic{
+			Severity: diag.Warning,
+			Summary:  "Deleting Object Bucket ACL resource resets ACL to private",
+			Detail:   "Deleting Object Bucket ACL resource resets the bucket's ACL to its default value: private.\nIf you wish to set it to something else, you should recreate a Bucket ACL resource with the `acl` field filled accordingly.",
+		},
+	}
 }

internal/services/object/bucket_acl_test.go

@@ -1,14 +1,25 @@
 package object_test
 
 import (
+	"errors"
 	"fmt"
 	"regexp"
+	"strings"
 	"testing"
 
+	"github.com/aws/aws-sdk-go/service/s3"
 	sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 	"github.com/scaleway/terraform-provider-scaleway/v2/internal/acctest"
+	"github.com/scaleway/terraform-provider-scaleway/v2/internal/services/object"
 	objectchecks "github.com/scaleway/terraform-provider-scaleway/v2/internal/services/object/testfuncs"
+	"github.com/scaleway/terraform-provider-scaleway/v2/internal/types"
+)
+
+const (
+	s3ACLGranteeAllUsers           = "AllUsers"
+	s3ACLGranteeAuthenticatedUsers = "AuthenticatedUsers"
 )
 
 func TestAccObjectBucketACL_Basic(t *testing.T) {
@@ -263,3 +274,178 @@ func TestAccObjectBucketACL_WithBucketName(t *testing.T) {
 		},
 	})
 }
+
+func TestAccObjectBucketACL_Remove(t *testing.T) {
+	tt := acctest.NewTestTools(t)
+	defer tt.Cleanup()
+	testBucketName := sdkacctest.RandomWithPrefix("test-acc-scw-object-acl-remove")
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() { acctest.PreCheck(t) },
+		ProviderFactories: tt.ProviderFactories,
+		CheckDestroy:      objectchecks.IsBucketDestroyed(tt),
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+					resource "scaleway_object_bucket" "main" {
+						name = "%s"
+						region = "%s"
+						acl = "authenticated-read"
+					}
+					`, testBucketName, objectTestsMainRegion),
+				Check: resource.ComposeTestCheckFunc(
+					objectchecks.CheckBucketExists(tt, "scaleway_object_bucket.main", true),
+					resource.TestCheckResourceAttr("scaleway_object_bucket.main", "acl", "authenticated-read"),
+					testAccObjectBucketACLCheck(tt, "scaleway_object_bucket.main", "authenticated-read"),
+				),
+			},
+			{
+				Config: fmt.Sprintf(`
+					resource "scaleway_object_bucket" "main" {
+						name = "%s"
+						region = "%s"
+						acl = "authenticated-read"
+					}
+
+					resource "scaleway_object_bucket_acl" "main" {
+						bucket = scaleway_object_bucket.main.id
+						acl = "public-read"
+					}
+					`, testBucketName, objectTestsMainRegion),
+				Check: resource.ComposeTestCheckFunc(
+					objectchecks.CheckBucketExists(tt, "scaleway_object_bucket.main", true),
+					resource.TestCheckResourceAttr("scaleway_object_bucket_acl.main", "bucket", testBucketName),
+					resource.TestCheckResourceAttr("scaleway_object_bucket.main", "acl", "authenticated-read"),
+					resource.TestCheckResourceAttr("scaleway_object_bucket_acl.main", "acl", "public-read"),
+					testAccObjectBucketACLCheck(tt, "scaleway_object_bucket.main", "public-read"),
+				),
+			},
+			{
+				Config: fmt.Sprintf(`
+					resource "scaleway_object_bucket" "main" {
+						name = "%s"
+						region = "%s"
+						acl = "authenticated-read"
+					}
+					`, testBucketName, objectTestsMainRegion),
+				Check: resource.ComposeTestCheckFunc(
+					objectchecks.CheckBucketExists(tt, "scaleway_object_bucket.main", true),
+					resource.TestCheckResourceAttr("scaleway_object_bucket.main", "acl", "authenticated-read"),
+					testAccObjectBucketACLCheck(tt, "scaleway_object_bucket.main", "private"),
+				),
+			},
+			{
+				Config: fmt.Sprintf(`
+					resource "scaleway_object_bucket" "main" {
+						name = "%s"
+						region = "%s"
+					}
+					`, testBucketName, objectTestsMainRegion),
+				Check: resource.ComposeTestCheckFunc(
+					objectchecks.CheckBucketExists(tt, "scaleway_object_bucket.main", true),
+					resource.TestCheckResourceAttr("scaleway_object_bucket.main", "acl", "private"),
+					testAccObjectBucketACLCheck(tt, "scaleway_object_bucket.main", "private"),
+				),
+			},
+		},
+	})
+}
+
+func testAccObjectBucketACLCheck(tt *acctest.TestTools, name string, expectedACL string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[name]
+		if !ok {
+			return fmt.Errorf("resource not found: %s", name)
+		}
+
+		bucketRegion := rs.Primary.Attributes["region"]
+		s3Client, err := object.NewS3ClientFromMeta(tt.Meta, bucketRegion)
+		if err != nil {
+			return err
+		}
+		if rs.Primary.ID == "" {
+			return errors.New("no ID is set")
+		}
+
+		bucketName := rs.Primary.Attributes["name"]
+		actualACL, err := s3Client.GetBucketAcl(&s3.GetBucketAclInput{
+			Bucket: types.ExpandStringPtr(bucketName),
+		})
+		if err != nil {
+			return fmt.Errorf("could not get ACL for bucket %s: %v", bucketName, err)
+		}
+
+		errs := s3ACLAreEqual(expectedACL, actualACL)
+		if len(errs) > 0 {
+			return fmt.Errorf("unexpected result: %w", errors.Join(errs...))
+		}
+		return nil
+	}
+}
+
+func s3ACLAreEqual(expected string, actual *s3.GetBucketAclOutput) (errs []error) {
+	ownerID := *object.NormalizeOwnerID(actual.Owner.ID)
+	grantsMap := make(map[string]string)
+	for _, actualACL := range actual.Grants {
+		if actualACL.Permission == nil {
+			return append(errs, errors.New("grant has no permission"))
+		}
+		if actualACL.Grantee.ID != nil {
+			grantsMap[*actualACL.Permission] = *object.NormalizeOwnerID(actualACL.Grantee.ID)
+		} else {
+			groupURI := strings.Split(*actualACL.Grantee.URI, "/")
+			grantsMap[*actualACL.Permission] = groupURI[len(groupURI)-1]
+		}
+	}
+
+	switch expected {
+	case "private":
+		if len(grantsMap) != 1 {
+			errs = append(errs, fmt.Errorf("expected 1 grant, but got %d", len(grantsMap)))
+			return errs
+		}
+		if grantsMap["FULL_CONTROL"] != ownerID {
+			errs = append(errs, fmt.Errorf("expected FULL_CONTROL to be granted to owner (%s), instead got %q", ownerID, grantsMap["FULL_CONTROL"]))
+		}
+
+	case "public-read":
+		if len(grantsMap) != 2 {
+			errs = append(errs, fmt.Errorf("expected 2 grants, but got %d", len(grantsMap)))
+			return errs
+		}
+		if grantsMap["FULL_CONTROL"] != ownerID {
+			errs = append(errs, fmt.Errorf("expected FULL_CONTROL to be granted to owner (%s), instead got %q", ownerID, grantsMap["FULL_CONTROL"]))
+		}
+		if grantsMap["READ"] != s3ACLGranteeAllUsers {
+			errs = append(errs, fmt.Errorf("expected READ to be granted to %q, instead got %q", s3ACLGranteeAllUsers, grantsMap["READ"]))
+		}
+
+	case "public-read-write":
+		if len(grantsMap) != 3 {
+			errs = append(errs, fmt.Errorf("expected 3 grants, but got %d", len(grantsMap)))
+			return errs
+		}
+		if grantsMap["FULL_CONTROL"] != ownerID {
+			errs = append(errs, fmt.Errorf("expected FULL_CONTROL to be granted to owner (%s), instead got %q", ownerID, grantsMap["FULL_CONTROL"]))
+		}
+		if grantsMap["READ"] != s3ACLGranteeAllUsers {
+			errs = append(errs, fmt.Errorf("expected READ to be granted to %q, instead got %q", s3ACLGranteeAllUsers, grantsMap["READ"]))
+		}
+		if grantsMap["WRITE"] != s3ACLGranteeAllUsers {
+			errs = append(errs, fmt.Errorf("expected WRITE to be granted to %q, instead got %q", s3ACLGranteeAllUsers, grantsMap["WRITE"]))
+		}
+
+	case "authenticated-read":
+		if len(grantsMap) != 2 {
+			errs = append(errs, fmt.Errorf("expected 2 grants, but got %d", len(grantsMap)))
+			return errs
+		}
+		if grantsMap["FULL_CONTROL"] != ownerID {
+			errs = append(errs, fmt.Errorf("expected FULL_CONTROL to be granted to owner (%s), instead got %q", ownerID, grantsMap["FULL_CONTROL"]))
+		}
+		if grantsMap["READ"] != s3ACLGranteeAuthenticatedUsers {
+			errs = append(errs, fmt.Errorf("expected READ to be granted to %q, instead got %q", s3ACLGranteeAuthenticatedUsers, grantsMap["READ"]))
+		}
+	}
+	return errs
+}

internal/services/object/bucket_lock_configuration.go

@@ -148,7 +148,7 @@ func resourceObjectLockConfigurationRead(ctx context.Context, d *schema.Resource
 	if err != nil {
 		return diag.FromErr(fmt.Errorf("couldn't read bucket acl: %s", err))
 	}
-	_ = d.Set("project_id", normalizeOwnerID(acl.Owner.ID))
+	_ = d.Set("project_id", NormalizeOwnerID(acl.Owner.ID))
 
 	_ = d.Set("bucket", bucket)
 	if output.ObjectLockConfiguration != nil {

internal/services/object/bucket_policy.go

@@ -159,7 +159,7 @@ func resourceObjectBucketPolicyRead(ctx context.Context, d *schema.ResourceData,
 			return diags
 		}
 	} else if acl != nil && acl.Owner != nil {
-		_ = d.Set("project_id", normalizeOwnerID(acl.Owner.ID))
+		_ = d.Set("project_id", NormalizeOwnerID(acl.Owner.ID))
 	}
 
 	return diags

internal/services/object/bucket_website_configuration.go

@@ -189,7 +189,7 @@ func resourceBucketWebsiteConfigurationRead(ctx context.Context, d *schema.Resou
 	if err != nil {
 		return diag.FromErr(fmt.Errorf("couldn't read bucket acl: %s", err))
 	}
-	_ = d.Set("project_id", normalizeOwnerID(acl.Owner.ID))
+	_ = d.Set("project_id", NormalizeOwnerID(acl.Owner.ID))
 
 	return nil
 }

internal/services/object/data_source_object_bucket.go

@@ -60,7 +60,7 @@ func DataSourceObjectStorageRead(ctx context.Context, d *schema.ResourceData, m
 	if err != nil {
 		return diag.FromErr(fmt.Errorf("couldn't read bucket acl: %s", err))
 	}
-	_ = d.Set("project_id", normalizeOwnerID(acl.Owner.ID))
+	_ = d.Set("project_id", NormalizeOwnerID(acl.Owner.ID))
 
 	bucketRegionalID := regional.NewIDString(region, bucket)
 	d.SetId(bucketRegionalID)

internal/services/object/data_source_object_bucket_policy.go

@@ -78,7 +78,7 @@ func DataSourceObjectBucketPolicyRead(ctx context.Context, d *schema.ResourceDat
 	if err != nil {
 		return diag.FromErr(fmt.Errorf("couldn't read bucket acl: %s", err))
 	}
-	_ = d.Set("project_id", normalizeOwnerID(acl.Owner.ID))
+	_ = d.Set("project_id", NormalizeOwnerID(acl.Owner.ID))
 
 	d.SetId(regional.NewIDString(region, bucket))
 	return nil

internal/services/object/helpers_object.go

@@ -561,7 +561,7 @@ func buildBucketOwnerID(id *string) *string {
 	return &s
 }
 
-func normalizeOwnerID(id *string) *string {
+func NormalizeOwnerID(id *string) *string {
 	tab := strings.Split(*id, ":")
 	if len(tab) != 2 {
 		return id