feat(object): bucket policy takes on bucket's region rather than default region if not explicit (#2229)

Author: Mia-Cross

docs/resources/object_bucket_policy.md

@@ -21,7 +21,7 @@ resource "scaleway_iam_application" "main" {
 }
 
 resource "scaleway_object_bucket_policy" "policy" {
-  bucket = scaleway_object_bucket.bucket.name
+  bucket = scaleway_object_bucket.bucket.id
   policy = jsonencode(
     {
       Version = "2023-04-17",
@@ -53,7 +53,7 @@ resource "scaleway_object_bucket" "bucket" {
 }
 
 resource "scaleway_object_bucket_policy" "main" {
-  bucket = scaleway_object_bucket.bucket.name
+  bucket = scaleway_object_bucket.bucket.id
   policy = data.aws_iam_policy_document.policy.json
 }
 

docs/resources/object_bucket_website_configuration.md

@@ -33,7 +33,7 @@ resource "scaleway_object_bucket" "main" {
 }
 
 resource "scaleway_object_bucket_policy" "main" {
-    bucket = scaleway_object_bucket.main.name
+    bucket = scaleway_object_bucket.main.id
     policy = jsonencode(
     {
         "Version" = "2012-10-17",

scaleway/data_source_object_bucket_policy.go

@@ -32,9 +32,18 @@ func dataSourceScalewayObjectBucketPolicyRead(ctx context.Context, d *schema.Res
 		return diag.FromErr(err)
 	}
 
-	bucket := expandID(d.Get("bucket"))
+	regionalID := expandRegionalID(d.Get("bucket"))
+	bucket := regionalID.ID
+	bucketRegion := regionalID.Region
 	tflog.Debug(ctx, fmt.Sprintf("bucket name: %s", bucket))
 
+	if bucketRegion != "" && bucketRegion != region {
+		s3Client, err = s3ClientForceRegion(d, meta, bucketRegion.String())
+		if err != nil {
+			return diag.FromErr(err)
+		}
+		region = bucketRegion
+	}
 	_ = d.Set("region", region)
 
 	tflog.Debug(ctx, fmt.Sprintf("[DEBUG] SCW bucket policy, read for bucket: %s", d.Id()))

scaleway/data_source_object_bucket_policy_test.go

@@ -4,8 +4,10 @@ import (
 	"fmt"
 	"testing"
 
+	awspolicy "github.com/hashicorp/awspolicyequivalence"
 	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"
 )
 
 func TestAccScalewayDataSourceObjectBucketPolicy_Basic(t *testing.T) {
@@ -14,6 +16,24 @@ func TestAccScalewayDataSourceObjectBucketPolicy_Basic(t *testing.T) {
 
 	bucketName := sdkacctest.RandomWithPrefix("test-acc-scw-obp-data-basic")
 
+	expectedPolicyText := `{
+	"Version":"2012-10-17",
+	"Id":"MyPolicy",
+	"Statement": [
+		{
+			"Sid":"GrantToEveryone",
+			"Effect":"Allow",
+			"Principal":{
+				"SCW":"*"
+			},
+			"Action":[
+				"s3:ListBucket",
+				"s3:GetObject"
+			]
+		}
+   ]
+}`
+
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:          func() { testAccPreCheck(t) },
 		ProviderFactories: tt.ProviderFactories,
@@ -23,10 +43,12 @@ func TestAccScalewayDataSourceObjectBucketPolicy_Basic(t *testing.T) {
 				Config: fmt.Sprintf(`
 					resource "scaleway_object_bucket" "main" {
 						name = "%[1]s"
+						region = "%[2]s"
 					}
 
 					resource "scaleway_object_bucket_policy" "main" {
-						bucket = scaleway_object_bucket.main.name
+						bucket = scaleway_object_bucket.main.id
+						region = "%[2]s"
 						policy = jsonencode(
 							{
 								Id = "MyPolicy"
@@ -55,14 +77,41 @@ func TestAccScalewayDataSourceObjectBucketPolicy_Basic(t *testing.T) {
 					data "scaleway_object_bucket_policy" "selected" {
 						bucket = scaleway_object_bucket_policy.main.bucket
 					}
-				`, bucketName),
+				`, bucketName, objectTestsMainRegion),
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("data.scaleway_object_bucket_policy.selected", "bucket", bucketName),
+					testAccCheckScalewayObjectBucketExistsForceRegion(tt, "scaleway_object_bucket.main"),
+					resource.TestCheckResourceAttr("data.scaleway_object_bucket_policy.selected", "bucket", objectTestsMainRegion+"/"+bucketName),
 					resource.TestCheckResourceAttrSet("data.scaleway_object_bucket_policy.selected", "policy"),
-					resource.TestCheckResourceAttrPair("data.scaleway_object_bucket_policy.selected", "policy", "scaleway_object_bucket_policy.main", "policy"),
+					testAccCheckDataSourcePolicyIsEquivalent("data.scaleway_object_bucket_policy.selected", expectedPolicyText),
 				),
 				ExpectNonEmptyPlan: !*UpdateCassettes,
 			},
 		},
 	})
 }
+
+func testAccCheckDataSourcePolicyIsEquivalent(n, expectedPolicyText string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		ds, ok := s.RootModule().Resources[n]
+		if !ok {
+			return fmt.Errorf("not found: %s", n)
+		}
+		dataSourcePolicy := ds.Primary.Attributes["policy"]
+
+		dataSourcePolicyToCompare, err := removePolicyStatementResources(dataSourcePolicy)
+		if err != nil {
+			return err
+		}
+
+		equivalent, err := awspolicy.PoliciesAreEquivalent(expectedPolicyText, dataSourcePolicyToCompare)
+		if err != nil {
+			return fmt.Errorf("error testing policy equivalence: %s", err)
+		}
+		if !equivalent {
+			return fmt.Errorf("non equivalent policy error:\n\nexpected: %s\n\n     got: %s",
+				expectedPolicyText, dataSourcePolicyToCompare)
+		}
+
+		return nil
+	}
+}

scaleway/helpers_object.go

@@ -30,6 +30,9 @@ const (
 	defaultObjectBucketTimeout = 10 * time.Minute
 
 	maxObjectVersionDeletionWorkers = 8
+
+	objectTestsMainRegion      = "nl-ams"
+	objectTestsSecondaryRegion = "pl-waw"
 )
 
 func newS3Client(httpClient *http.Client, region, accessKey, secretKey string) (*s3.S3, error) {
@@ -66,6 +69,18 @@ func newS3ClientFromMeta(meta *Meta) (*s3.S3, error) {
 	return newS3Client(meta.httpClient, region.String(), accessKey, secretKey)
 }
 
+func newS3ClientFromMetaForceRegion(meta *Meta, region string) (*s3.S3, error) {
+	accessKey, _ := meta.scwClient.GetAccessKey()
+	secretKey, _ := meta.scwClient.GetSecretKey()
+
+	projectID, _ := meta.scwClient.GetDefaultProjectID()
+	if projectID != "" {
+		accessKey = accessKeyWithProjectID(accessKey, projectID)
+	}
+
+	return newS3Client(meta.httpClient, region, accessKey, secretKey)
+}
+
 func s3ClientWithRegion(d *schema.ResourceData, m interface{}) (*s3.S3, scw.Region, error) {
 	meta := m.(*Meta)
 	region, err := extractRegion(d, meta)
@@ -163,6 +178,23 @@ func s3ClientWithRegionWithNameACL(d *schema.ResourceData, m interface{}, name s
 	return s3Client, scw.Region(region), name, outerID, err
 }
 
+func s3ClientForceRegion(d *schema.ResourceData, m interface{}, region string) (*s3.S3, error) {
+	meta := m.(*Meta)
+
+	accessKey, _ := meta.scwClient.GetAccessKey()
+	if projectID, _, err := extractProjectID(d, meta); err == nil {
+		accessKey = accessKeyWithProjectID(accessKey, projectID)
+	}
+	secretKey, _ := meta.scwClient.GetSecretKey()
+
+	s3Client, err := newS3Client(meta.httpClient, region, accessKey, secretKey)
+	if err != nil {
+		return nil, err
+	}
+
+	return s3Client, err
+}
+
 func accessKeyWithProjectID(accessKey string, projectID string) string {
 	return accessKey + "@" + projectID
 }

scaleway/resource_object_bucket_policy.go

@@ -10,7 +10,7 @@ import (
 	"github.com/hashicorp/aws-sdk-go-base/tfawserr"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
 	"github.com/scaleway/scaleway-sdk-go/scw"
@@ -30,9 +30,10 @@ func resourceScalewayObjectBucketPolicy() *schema.Resource {
 		},
 		Schema: map[string]*schema.Schema{
 			"bucket": {
-				Type:        schema.TypeString,
-				Required:    true,
-				Description: "The bucket name.",
+				Type:             schema.TypeString,
+				Required:         true,
+				Description:      "The bucket's name or regional ID.",
+				DiffSuppressFunc: diffSuppressFuncLocality,
 			},
 			"policy": {
 				Type:             schema.TypeString,
@@ -52,9 +53,19 @@ func resourceScalewayObjectBucketPolicyCreate(ctx context.Context, d *schema.Res
 		return diag.FromErr(err)
 	}
 
-	bucket := expandID(d.Get("bucket"))
+	regionalID := expandRegionalID(d.Get("bucket"))
+	bucket := regionalID.ID
+	bucketRegion := regionalID.Region
 	tflog.Debug(ctx, fmt.Sprintf("bucket name: %s", bucket))
 
+	if bucketRegion != "" && bucketRegion != region {
+		s3Client, err = s3ClientForceRegion(d, meta, bucketRegion.String())
+		if err != nil {
+			return diag.FromErr(err)
+		}
+		region = bucketRegion
+	}
+
 	policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
 	if err != nil {
 		return diag.FromErr(fmt.Errorf("policy (%s) is an invalid JSON: %w", policy, err))
@@ -67,13 +78,13 @@ func resourceScalewayObjectBucketPolicyCreate(ctx context.Context, d *schema.Res
 		Policy: scw.StringPtr(policy),
 	}
 
-	err = resource.RetryContext(ctx, 1*time.Minute, func() *resource.RetryError {
+	err = retry.RetryContext(ctx, 1*time.Minute, func() *retry.RetryError {
 		_, err := s3Client.PutBucketPolicyWithContext(ctx, params)
 		if tfawserr.ErrCodeEquals(err, "MalformedPolicy") {
-			return resource.RetryableError(err)
+			return retry.RetryableError(err)
 		}
 		if err != nil {
-			return resource.NonRetryableError(err)
+			return retry.NonRetryableError(err)
 		}
 		return nil
 	})
@@ -97,7 +108,8 @@ func resourceScalewayObjectBucketPolicyRead(ctx context.Context, d *schema.Resou
 		return diag.FromErr(err)
 	}
 
-	bucket := expandID(d.Id())
+	regionalID := expandRegionalID(d.Id())
+	bucket := regionalID.ID
 
 	_ = d.Set("region", region)
 
@@ -132,7 +144,7 @@ func resourceScalewayObjectBucketPolicyRead(ctx context.Context, d *schema.Resou
 		return diag.FromErr(err)
 	}
 
-	if err := d.Set("bucket", bucket); err != nil {
+	if err := d.Set("bucket", regionalID.String()); err != nil {
 		return diag.FromErr(err)
 	}