-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add google_storage_default_object_acl resource #992
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
ba3c60c
Storage Default Object ACL resource
ishashchuk 19912ec
Fixed the doc
ishashchuk 6810e53
Renamed the resource id. Log change
ishashchuk 2c80aee
Merged latest upstream changes into the branch
ishashchuk 58c7e0e
Merge pull request #1 from wayfair/storage_default_acl
amoiseiev e7cebf7
Complying with go vet
ishashchuk 5e35381
Changes for review
ishashchuk c702a73
Merge remote-tracking branch 'upstream/master'
ishashchuk bca2b1b
Merge pull request #2 from wayfair/ishashchuk_storage_defaul_acl
amoiseiev a89bc5d
link to default object acl docs in sidebar
danawillow File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,185 @@ | ||
package google | ||
|
||
import ( | ||
"fmt" | ||
"log" | ||
|
||
"github.com/hashicorp/terraform/helper/schema" | ||
"google.golang.org/api/storage/v1" | ||
) | ||
|
||
func resourceStorageDefaultObjectAcl() *schema.Resource { | ||
return &schema.Resource{ | ||
Create: resourceStorageDefaultObjectAclCreate, | ||
Read: resourceStorageDefaultObjectAclRead, | ||
Update: resourceStorageDefaultObjectAclUpdate, | ||
Delete: resourceStorageDefaultObjectAclDelete, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"bucket": &schema.Schema{ | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"role_entity": &schema.Schema{ | ||
Type: schema.TypeList, | ||
Required: true, | ||
Elem: &schema.Schema{Type: schema.TypeString}, | ||
MinItems: 1, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func resourceStorageDefaultObjectAclCreate(d *schema.ResourceData, meta interface{}) error { | ||
config := meta.(*Config) | ||
|
||
bucket := d.Get("bucket").(string) | ||
roleEntity := d.Get("role_entity").([]interface{}) | ||
|
||
for _, v := range roleEntity { | ||
pair, err := getRoleEntityPair(v.(string)) | ||
|
||
ObjectAccessControl := &storage.ObjectAccessControl{ | ||
Role: pair.Role, | ||
Entity: pair.Entity, | ||
} | ||
|
||
log.Printf("[DEBUG]: setting role = %s, entity = %s on bucket %s", pair.Role, pair.Entity, bucket) | ||
|
||
_, err = config.clientStorage.DefaultObjectAccessControls.Insert(bucket, ObjectAccessControl).Do() | ||
|
||
if err != nil { | ||
return fmt.Errorf("Error setting Default Object ACL for %s on bucket %s: %v", pair.Entity, bucket, err) | ||
} | ||
} | ||
d.SetId(bucket) | ||
return resourceStorageDefaultObjectAclRead(d, meta) | ||
} | ||
|
||
func resourceStorageDefaultObjectAclRead(d *schema.ResourceData, meta interface{}) error { | ||
config := meta.(*Config) | ||
|
||
bucket := d.Get("bucket").(string) | ||
|
||
roleEntities := make([]interface{}, 0) | ||
reLocal := d.Get("role_entity").([]interface{}) | ||
reLocalMap := make(map[string]string) | ||
for _, v := range reLocal { | ||
res, err := getRoleEntityPair(v.(string)) | ||
|
||
if err != nil { | ||
return fmt.Errorf( | ||
"Old state has malformed Role/Entity pair: %v", err) | ||
} | ||
|
||
reLocalMap[res.Entity] = res.Role | ||
} | ||
|
||
res, err := config.clientStorage.DefaultObjectAccessControls.List(bucket).Do() | ||
|
||
if err != nil { | ||
return handleNotFoundError(err, d, fmt.Sprintf("Storage Default Object ACL for bucket %q", d.Get("bucket").(string))) | ||
} | ||
|
||
for _, v := range res.Items { | ||
role := v.Role | ||
entity := v.Entity | ||
// We only store updates to the locally defined access controls | ||
if _, in := reLocalMap[entity]; in { | ||
roleEntities = append(roleEntities, fmt.Sprintf("%s:%s", role, entity)) | ||
log.Printf("[DEBUG]: saving re %s-%s", v.Role, v.Entity) | ||
} | ||
} | ||
|
||
d.Set("role_entity", roleEntities) | ||
|
||
return nil | ||
} | ||
|
||
func resourceStorageDefaultObjectAclUpdate(d *schema.ResourceData, meta interface{}) error { | ||
config := meta.(*Config) | ||
|
||
bucket := d.Get("bucket").(string) | ||
|
||
if !d.HasChange("role_entity") { | ||
return nil | ||
} | ||
o, n := d.GetChange("role_entity") | ||
oldRe := o.([]interface{}) | ||
newRe := n.([]interface{}) | ||
|
||
oldReMap := make(map[string]string) | ||
for _, v := range oldRe { | ||
res, err := getRoleEntityPair(v.(string)) | ||
|
||
if err != nil { | ||
return fmt.Errorf( | ||
"Old state has malformed Role/Entity pair: %v", err) | ||
} | ||
|
||
oldReMap[res.Entity] = res.Role | ||
} | ||
|
||
for _, v := range newRe { | ||
pair, err := getRoleEntityPair(v.(string)) | ||
|
||
ObjectAccessControl := &storage.ObjectAccessControl{ | ||
Role: pair.Role, | ||
Entity: pair.Entity, | ||
} | ||
|
||
// If the old state is present for the entity, it is updated | ||
// If the old state is missing, it is inserted | ||
if _, ok := oldReMap[pair.Entity]; ok { | ||
_, err = config.clientStorage.DefaultObjectAccessControls.Update( | ||
bucket, pair.Entity, ObjectAccessControl).Do() | ||
} else { | ||
_, err = config.clientStorage.DefaultObjectAccessControls.Insert( | ||
bucket, ObjectAccessControl).Do() | ||
} | ||
|
||
// Now we only store the keys that have to be removed | ||
delete(oldReMap, pair.Entity) | ||
|
||
if err != nil { | ||
return fmt.Errorf("Error updating Storage Default Object ACL for bucket %s: %v", bucket, err) | ||
} | ||
} | ||
|
||
for entity := range oldReMap { | ||
log.Printf("[DEBUG]: removing entity %s", entity) | ||
err := config.clientStorage.DefaultObjectAccessControls.Delete(bucket, entity).Do() | ||
|
||
if err != nil { | ||
return fmt.Errorf("Error updating Storage Default Object ACL for bucket %s: %v", bucket, err) | ||
} | ||
} | ||
|
||
return resourceStorageDefaultObjectAclRead(d, meta) | ||
} | ||
|
||
func resourceStorageDefaultObjectAclDelete(d *schema.ResourceData, meta interface{}) error { | ||
config := meta.(*Config) | ||
|
||
bucket := d.Get("bucket").(string) | ||
|
||
reLocal := d.Get("role_entity").([]interface{}) | ||
for _, v := range reLocal { | ||
res, err := getRoleEntityPair(v.(string)) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
log.Printf("[DEBUG]: removing entity %s", res.Entity) | ||
|
||
err = config.clientStorage.DefaultObjectAccessControls.Delete(bucket, res.Entity).Do() | ||
|
||
if err != nil { | ||
return fmt.Errorf("Error deleting entity %s ACL: %s", res.Entity, err) | ||
} | ||
} | ||
|
||
return nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,187 @@ | ||
package google | ||
|
||
import ( | ||
"fmt" | ||
"testing" | ||
|
||
"github.com/hashicorp/terraform/helper/resource" | ||
"github.com/hashicorp/terraform/terraform" | ||
) | ||
|
||
func TestAccGoogleStorageDefaultObjectAcl_basic(t *testing.T) { | ||
t.Parallel() | ||
|
||
bucketName := testBucketName() | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testAccGoogleStorageDefaultObjectAclDestroy, | ||
Steps: []resource.TestStep{ | ||
resource.TestStep{ | ||
Config: testGoogleStorageDefaultObjectsAclBasic(bucketName, roleEntityBasic1, roleEntityBasic2), | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic1), | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic2), | ||
), | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func TestAccGoogleStorageDefaultObjectAcl_upgrade(t *testing.T) { | ||
t.Parallel() | ||
|
||
bucketName := testBucketName() | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testAccGoogleStorageDefaultObjectAclDestroy, | ||
Steps: []resource.TestStep{ | ||
resource.TestStep{ | ||
Config: testGoogleStorageDefaultObjectsAclBasic(bucketName, roleEntityBasic1, roleEntityBasic2), | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic1), | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic2), | ||
), | ||
}, | ||
|
||
resource.TestStep{ | ||
Config: testGoogleStorageDefaultObjectsAclBasic(bucketName, roleEntityBasic2, roleEntityBasic3_owner), | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic2), | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic3_owner), | ||
), | ||
}, | ||
|
||
resource.TestStep{ | ||
Config: testGoogleStorageDefaultObjectsAclBasicDelete(bucketName, roleEntityBasic1), | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic1), | ||
testAccCheckGoogleStorageDefaultObjectAclDelete(bucketName, roleEntityBasic2), | ||
testAccCheckGoogleStorageDefaultObjectAclDelete(bucketName, roleEntityBasic3_reader), | ||
), | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func TestAccGoogleStorageDefaultObjectAcl_downgrade(t *testing.T) { | ||
t.Parallel() | ||
|
||
bucketName := testBucketName() | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testAccGoogleStorageDefaultObjectAclDestroy, | ||
Steps: []resource.TestStep{ | ||
resource.TestStep{ | ||
Config: testGoogleStorageDefaultObjectsAclBasic(bucketName, roleEntityBasic2, roleEntityBasic3_owner), | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic2), | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic3_owner), | ||
), | ||
}, | ||
|
||
resource.TestStep{ | ||
Config: testGoogleStorageDefaultObjectsAclBasic(bucketName, roleEntityBasic2, roleEntityBasic3_reader), | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic2), | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic3_reader), | ||
), | ||
}, | ||
|
||
resource.TestStep{ | ||
Config: testGoogleStorageDefaultObjectsAclBasicDelete(bucketName, roleEntityBasic1), | ||
Check: resource.ComposeTestCheckFunc( | ||
testAccCheckGoogleStorageDefaultObjectAcl(bucketName, roleEntityBasic1), | ||
testAccCheckGoogleStorageDefaultObjectAclDelete(bucketName, roleEntityBasic2), | ||
testAccCheckGoogleStorageDefaultObjectAclDelete(bucketName, roleEntityBasic3_reader), | ||
), | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func testAccCheckGoogleStorageDefaultObjectAcl(bucket, roleEntityS string) resource.TestCheckFunc { | ||
return func(s *terraform.State) error { | ||
roleEntity, _ := getRoleEntityPair(roleEntityS) | ||
config := testAccProvider.Meta().(*Config) | ||
|
||
res, err := config.clientStorage.DefaultObjectAccessControls.Get(bucket, | ||
roleEntity.Entity).Do() | ||
|
||
if err != nil { | ||
return fmt.Errorf("Error retrieving contents of storage default Acl for bucket %s: %s", bucket, err) | ||
} | ||
|
||
if res.Role != roleEntity.Role { | ||
return fmt.Errorf("Error, Role mismatch %s != %s", res.Role, roleEntity.Role) | ||
} | ||
|
||
return nil | ||
} | ||
} | ||
|
||
func testAccGoogleStorageDefaultObjectAclDestroy(s *terraform.State) error { | ||
config := testAccProvider.Meta().(*Config) | ||
|
||
for _, rs := range s.RootModule().Resources { | ||
|
||
if rs.Type != "google_storage_default_object_acl" { | ||
continue | ||
} | ||
|
||
bucket := rs.Primary.Attributes["bucket"] | ||
|
||
_, err := config.clientStorage.DefaultObjectAccessControls.List(bucket).Do() | ||
if err == nil { | ||
return fmt.Errorf("Default Storage Object Acl for bucket %s still exists", bucket) | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func testAccCheckGoogleStorageDefaultObjectAclDelete(bucket, roleEntityS string) resource.TestCheckFunc { | ||
return func(s *terraform.State) error { | ||
roleEntity, _ := getRoleEntityPair(roleEntityS) | ||
config := testAccProvider.Meta().(*Config) | ||
|
||
_, err := config.clientStorage.DefaultObjectAccessControls.Get(bucket, roleEntity.Entity).Do() | ||
|
||
if err != nil { | ||
return nil | ||
} | ||
|
||
return fmt.Errorf("Error, Object Default Acl Entity still exists %s for bucket %s", | ||
roleEntity.Entity, bucket) | ||
} | ||
} | ||
|
||
func testGoogleStorageDefaultObjectsAclBasicDelete(bucketName, roleEntity string) string { | ||
return fmt.Sprintf(` | ||
resource "google_storage_bucket" "bucket" { | ||
name = "%s" | ||
} | ||
|
||
resource "google_storage_default_object_acl" "acl" { | ||
bucket = "${google_storage_bucket.bucket.name}" | ||
role_entity = ["%s"] | ||
} | ||
`, bucketName, roleEntity) | ||
} | ||
|
||
func testGoogleStorageDefaultObjectsAclBasic(bucketName, roleEntity1, roleEntity2 string) string { | ||
return fmt.Sprintf(` | ||
resource "google_storage_bucket" "bucket" { | ||
name = "%s" | ||
} | ||
|
||
resource "google_storage_default_object_acl" "acl" { | ||
bucket = "${google_storage_bucket.bucket.name}" | ||
role_entity = ["%s", "%s"] | ||
} | ||
`, bucketName, roleEntity1, roleEntity2) | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd highly recommend just having two separate fields here- one for the role, and the other for the entity. The id for the resource can then be
bucket/entity
, which would allow reading back the resource just from the id. If a Terraform user wants to specify multiple default acls, they can just have multiple resources. I'm going to hold off on reviewing the rest of this PR for now since it'll change a lot if you end up going with this approach.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't necessarily think it is a good idea, because it is very likely that users will have multiple default acls. Default acls get applied to the objects only at object creation time, which means that the explicit dependency will need to be set for "google_storage_bucket_object" resource on "google_storage_default_object_acl" resource to ensure that if both of those resources are created simultaneously, the objects always gets created after the default acl is in place. Having multiple resources will make it more complicated to control for those explicit dependencies.
Besides, it seems to me that this resource should be similar in use with google_storage_object_acl, since it essentially does the same thing(sets up object permissions), but on the bucket level. So it seems to me that both should have similar resource declaration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, I can get behind having them be consistent with each other. That's fine then, I'll continue reviewing as-is :)