public/terraform-provider-google
0
0
Fork 0
mirror of https://github.com/letic/terraform-provider-google.git synced 2024-10-01 16:21:06 +00:00
Code Issues Projects Releases Wiki Activity

Deal with undeleatable bucket ACLs in storage.

Browse Source 
When GCS buckets are created, they're created with a set of default
ACLs:

* `OWNER:project-owners-{project_number}`
* `OWNER:project-editors-{project_number}`
* `READER:project-viewers-{project_number}`

Normally, this would be fine, or a minor inconvenience. Terraform could
either delete them itself, or the first apply of a user would overwrite
them.

However, trying to remove the `OWNER:project-owners-{project_number}`
ACL yields an API error that the bucket owner must maintain OWNER access
to the bucket. This breaks things like `terraform destroy`, but also
means any config without that line in it will fail to apply, not just
overwrite the value.

To make matters worse, trying to *add* the
`OWNER:project-owners-{project_number}` ACL to any bucket that already
has it _also_ yields the same error about not being able to remove it.

To get around this, the storage_bucket_acl resource has been updated to
largely ignore _just this_ ACL. It will not try to add it if it already
exists, will not try to remove it at all. This does mean that Terraform
is incapable of removing this ACL from a bucket, but I'm not sure it's
possible to do that with the API, anyways.

Tests were also updated to keep the default ACLs as part of the config,
and to change the email addresses to addresses we actually own. I tried
changing to non-existant hashicorp.com email addresses, but was
rejected; only email addresses that are backed by actual Google accounts
can be used, sadly.
This commit is contained in:
Paddy 2017-09-15 12:03:03 -07:00
parent b694d5a325
commit 72114636da
2 changed files with 105 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
google/resource_storage_bucket_acl.go
View File

@ -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()

77
google/resource_storage_bucket_acl_test.go
View File

@ -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 (
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 roleEntityBasic2 = "READER:user-anotheremail@gmail.com"
var roleEntityBasic3_owner = "OWNER:user-yetanotheremail@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 {