diff --git a/google/resource_storage_bucket_acl.go b/google/resource_storage_bucket_acl.go index 78a2ec38498..3a7882bdde3 100644 --- a/google/resource_storage_bucket_acl.go +++ b/google/resource_storage_bucket_acl.go @@ -3,6 +3,7 @@ package google import ( "fmt" "log" + "strconv" "strings" "github.com/hashicorp/terraform/helper/schema" @@ -101,9 +102,26 @@ func resourceStorageBucketAclCreate(d *schema.ResourceData, meta interface{}) er } if len(role_entity) > 0 { + current, err := config.clientStorage.BucketAccessControls.List(bucket).Do() + if err != nil { + return fmt.Errorf("Error retrieving current ACLs: %s", err) + } for _, v := range role_entity { pair, err := getRoleEntityPair(v.(string)) - + if err != nil { + return err + } + var alreadyInserted bool + for _, cur := range current.Items { + if cur.Entity == pair.Entity && cur.Role == pair.Role { + alreadyInserted = true + break + } + } + if alreadyInserted { + log.Printf("[DEBUG]: pair %s-%s already exists, not trying to insert again\n", pair.Role, pair.Entity) + continue + } bucketAccessControl := &storage.BucketAccessControl{ Role: pair.Role, Entity: pair.Entity, @@ -172,8 +190,16 @@ func resourceStorageBucketAclUpdate(d *schema.ResourceData, meta interface{}) er config := meta.(*Config) bucket := d.Get("bucket").(string) + project, err := getProject(d, config) + if err != nil { + return err + } if d.HasChange("role_entity") { + p, err := config.clientResourceManager.Projects.Get(project).Do() + if err != nil { + return fmt.Errorf("Error retrieving project %q: %s", project, err) + } o, n := d.GetChange("role_entity") old_re, new_re := o.([]interface{}), n.([]interface{}) @@ -197,12 +223,8 @@ func resourceStorageBucketAclUpdate(d *schema.ResourceData, meta interface{}) er Entity: pair.Entity, } - // If the old state is missing this entity, it needs to - // be created. Otherwise it is updated - if _, ok := old_re_map[pair.Entity]; ok { - _, err = config.clientStorage.BucketAccessControls.Update( - bucket, pair.Entity, bucketAccessControl).Do() - } else { + // If the old state is missing this entity, it needs to be inserted + if _, ok := old_re_map[pair.Entity]; !ok { _, err = config.clientStorage.BucketAccessControls.Insert( bucket, bucketAccessControl).Do() } @@ -215,7 +237,11 @@ func resourceStorageBucketAclUpdate(d *schema.ResourceData, meta interface{}) er } } - for entity, _ := range old_re_map { + for entity, role := range old_re_map { + if entity == "project-owners-"+strconv.FormatInt(p.ProjectNumber, 10) && role == "OWNER" { + log.Printf("Skipping %s-%s; not deleting owner ACL.", role, entity) + continue + } log.Printf("[DEBUG]: removing entity %s", entity) err := config.clientStorage.BucketAccessControls.Delete(bucket, entity).Do() @@ -252,8 +278,18 @@ func resourceStorageBucketAclUpdate(d *schema.ResourceData, meta interface{}) er func resourceStorageBucketAclDelete(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) + project, err := getProject(d, config) + if err != nil { + return err + } + bucket := d.Get("bucket").(string) + p, err := config.clientResourceManager.Projects.Get(project).Do() + if err != nil { + return fmt.Errorf("Error retrieving project %q: %s", project, err) + } + re_local := d.Get("role_entity").([]interface{}) for _, v := range re_local { res, err := getRoleEntityPair(v.(string)) @@ -261,6 +297,11 @@ func resourceStorageBucketAclDelete(d *schema.ResourceData, meta interface{}) er return err } + if res.Entity == "project-owners-"+strconv.FormatInt(p.ProjectNumber, 10) && res.Role == "OWNER" { + log.Printf("Skipping %s-%s; not deleting owner ACL.", res.Role, res.Entity) + continue + } + log.Printf("[DEBUG]: removing entity %s", res.Entity) err = config.clientStorage.BucketAccessControls.Delete(bucket, res.Entity).Do() diff --git a/google/resource_storage_bucket_acl_test.go b/google/resource_storage_bucket_acl_test.go index 05de2d5eddc..61fc761d2bf 100644 --- a/google/resource_storage_bucket_acl_test.go +++ b/google/resource_storage_bucket_acl_test.go @@ -2,21 +2,44 @@ package google import ( "fmt" + "strconv" "testing" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" - //"google.golang.org/api/storage/v1" ) -var roleEntityBasic1 = "OWNER:user-omeemail@gmail.com" - -var roleEntityBasic2 = "READER:user-anotheremail@gmail.com" - -var roleEntityBasic3_owner = "OWNER:user-yetanotheremail@gmail.com" +var ( + roleEntityBasic1 = "OWNER:user-paddy@hashicorp.com" + roleEntityBasic2 = "READER:user-paddy@carvers.co" + roleEntityBasic3_owner = "OWNER:user-paddy@paddy.io" + roleEntityBasic3_reader = "READER:user-foran.paddy@gmail.com" +) -var roleEntityBasic3_reader = "READER:user-yetanotheremail@gmail.com" +func defaultRoleEntities() ([]string, error) { + creds := multiEnvSearch(credsEnvVars) + project := multiEnvSearch(projectEnvVars) + region := multiEnvSearch(regionEnvVars) + config := Config{ + Credentials: creds, + Project: project, + Region: region, + } + if err := config.loadAndValidate(); err != nil { + return nil, fmt.Errorf("Error setting up client: %s", err) + } + p, err := config.clientResourceManager.Projects.Get(project).Do() + if err != nil { + return nil, fmt.Errorf("Error retrieving test project %q: %s", project, err) + } + projectNumber := p.ProjectNumber + return []string{ + "OWNER:project-owners-" + strconv.FormatInt(projectNumber, 10), + "OWNER:project-editors-" + strconv.FormatInt(projectNumber, 10), + "READER:project-viewers-" + strconv.FormatInt(projectNumber, 10), + }, nil +} func testBucketName() string { return fmt.Sprintf("%s-%d", "tf-test-acl-bucket", acctest.RandInt()) @@ -24,13 +47,17 @@ func testBucketName() string { func TestAccGoogleStorageBucketAcl_basic(t *testing.T) { bucketName := testBucketName() + entities, err := defaultRoleEntities() + if err != nil { + t.Fatal(err) + } resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccGoogleStorageBucketAclDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testGoogleStorageBucketsAclBasic1(bucketName), + Config: testGoogleStorageBucketsAclBasic1(bucketName, entities), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic1), testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic2), @@ -42,13 +69,17 @@ func TestAccGoogleStorageBucketAcl_basic(t *testing.T) { func TestAccGoogleStorageBucketAcl_upgrade(t *testing.T) { bucketName := testBucketName() + entities, err := defaultRoleEntities() + if err != nil { + t.Fatal(err) + } resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccGoogleStorageBucketAclDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testGoogleStorageBucketsAclBasic1(bucketName), + Config: testGoogleStorageBucketsAclBasic1(bucketName, entities), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic1), testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic2), @@ -56,7 +87,7 @@ func TestAccGoogleStorageBucketAcl_upgrade(t *testing.T) { }, resource.TestStep{ - Config: testGoogleStorageBucketsAclBasic2(bucketName), + Config: testGoogleStorageBucketsAclBasic2(bucketName, entities), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic2), testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic3_owner), @@ -77,13 +108,17 @@ func TestAccGoogleStorageBucketAcl_upgrade(t *testing.T) { func TestAccGoogleStorageBucketAcl_downgrade(t *testing.T) { bucketName := testBucketName() + entities, err := defaultRoleEntities() + if err != nil { + t.Fatal(err) + } resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccGoogleStorageBucketAclDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testGoogleStorageBucketsAclBasic2(bucketName), + Config: testGoogleStorageBucketsAclBasic2(bucketName, entities), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic2), testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic3_owner), @@ -91,7 +126,7 @@ func TestAccGoogleStorageBucketAcl_downgrade(t *testing.T) { }, resource.TestStep{ - Config: testGoogleStorageBucketsAclBasic3(bucketName), + Config: testGoogleStorageBucketsAclBasic3(bucketName, entities), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic2), testAccCheckGoogleStorageBucketAcl(bucketName, roleEntityBasic3_reader), @@ -178,7 +213,7 @@ func testAccGoogleStorageBucketAclDestroy(s *terraform.State) error { return nil } -func testGoogleStorageBucketsAclBasic1(bucketName string) string { +func testGoogleStorageBucketsAclBasic1(bucketName string, entities []string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" { name = "%s" @@ -186,12 +221,12 @@ resource "google_storage_bucket" "bucket" { resource "google_storage_bucket_acl" "acl" { bucket = "${google_storage_bucket.bucket.name}" - role_entity = ["%s", "%s"] + role_entity = ["%s", "%s", "%s", "%s", "%s"] } -`, bucketName, roleEntityBasic1, roleEntityBasic2) +`, bucketName, entities[0], entities[1], entities[2], roleEntityBasic1, roleEntityBasic2) } -func testGoogleStorageBucketsAclBasic2(bucketName string) string { +func testGoogleStorageBucketsAclBasic2(bucketName string, entities []string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" { name = "%s" @@ -199,9 +234,9 @@ resource "google_storage_bucket" "bucket" { resource "google_storage_bucket_acl" "acl" { bucket = "${google_storage_bucket.bucket.name}" - role_entity = ["%s", "%s"] + role_entity = ["%s", "%s", "%s", "%s", "%s"] } -`, bucketName, roleEntityBasic2, roleEntityBasic3_owner) +`, bucketName, entities[0], entities[1], entities[2], roleEntityBasic2, roleEntityBasic3_owner) } func testGoogleStorageBucketsAclBasicDelete(bucketName string) string { @@ -217,7 +252,7 @@ resource "google_storage_bucket_acl" "acl" { `, bucketName) } -func testGoogleStorageBucketsAclBasic3(bucketName string) string { +func testGoogleStorageBucketsAclBasic3(bucketName string, entities []string) string { return fmt.Sprintf(` resource "google_storage_bucket" "bucket" { name = "%s" @@ -225,9 +260,9 @@ resource "google_storage_bucket" "bucket" { resource "google_storage_bucket_acl" "acl" { bucket = "${google_storage_bucket.bucket.name}" - role_entity = ["%s", "%s"] + role_entity = ["%s", "%s", "%s", "%s", "%s"] } -`, bucketName, roleEntityBasic2, roleEntityBasic3_reader) +`, bucketName, entities[0], entities[1], entities[2], roleEntityBasic2, roleEntityBasic3_reader) } func testGoogleStorageBucketsAclPredefined(bucketName string) string {