Skip to content
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_project_iam_binding and google_project_iam_member resources. #171

Merged
merged 27 commits into from
Jul 28, 2017
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8e70487
Add google_project_iam_binding resource.
paddycarver Jul 4, 2017
eed8487
Update website docs for google_project_iam_binding.
paddycarver Jul 4, 2017
11abe24
Fix reads, make ForceNew.
paddycarver Jul 4, 2017
65a4a36
Remove ID when binding isn't found.
paddycarver Jul 4, 2017
a84b22d
Fix vet errors.
paddycarver Jul 4, 2017
f9236ab
Add the google_project_iam_member resource.
paddycarver Jul 4, 2017
8a880fd
Documentation updates.
paddycarver Jul 4, 2017
d3f901b
Refactor binding update loop for clarity.
paddycarver Jul 25, 2017
9c1c0bb
Just remove deleted bindings not present in the API.
paddycarver Jul 25, 2017
729e9fc
Create an iam policy read/modify/write helper.
paddycarver Jul 25, 2017
5282ad7
Use string slice conversion helper.
paddycarver Jul 25, 2017
f9eeb36
terraform fmt test configs.
paddycarver Jul 25, 2017
52d552d
id => project_id in test configs.
paddycarver Jul 25, 2017
b0e3790
Use the policy r/m/w helper and handle edge case.
paddycarver Jul 25, 2017
91d227c
id => project_id
paddycarver Jul 25, 2017
655435f
Terraform fmt on our test configs.
paddycarver Jul 25, 2017
fa2d54f
Terraform fmt website examples.
paddycarver Jul 25, 2017
199ff5d
Excise unnecessary type declarations.
paddycarver Jul 27, 2017
adc206a
Add test case for updating to remove member.
paddycarver Jul 27, 2017
f94c387
Switch to / as separator.
paddycarver Jul 27, 2017
7854535
Add logging statements, update : to / in IDs.
paddycarver Jul 27, 2017
f88e042
Test adding multiple bindings at once.
paddycarver Jul 27, 2017
ac5df40
Return a type that was needed, rename a test function.
paddycarver Jul 27, 2017
d3426d5
Don't set IDs in RMW loops.
paddycarver Jul 27, 2017
6f98217
Fix embarrassing typo in log message.
paddycarver Jul 27, 2017
4b9432d
Newlines on website source.
paddycarver Jul 27, 2017
c89429c
Newlines, but really this time.
paddycarver Jul 27, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions google/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"log"
"strings"

"github.com/hashicorp/errwrap"
"github.com/hashicorp/terraform/helper/mutexkv"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/terraform"
Expand Down Expand Up @@ -106,6 +107,8 @@ func Provider() terraform.ResourceProvider {
"google_sql_user": resourceSqlUser(),
"google_project": resourceGoogleProject(),
"google_project_iam_policy": resourceGoogleProjectIamPolicy(),
"google_project_iam_binding": resourceGoogleProjectIamBinding(),
"google_project_iam_member": resourceGoogleProjectIamMember(),
"google_project_services": resourceGoogleProjectServices(),
"google_pubsub_topic": resourcePubsubTopic(),
"google_pubsub_subscription": resourcePubsubSubscription(),
Expand Down Expand Up @@ -279,6 +282,18 @@ func handleNotFoundError(err error, d *schema.ResourceData, resource string) err
return fmt.Errorf("Error reading %s: %s", resource, err)
}

func isConflictError(err error) bool {
if e, ok := err.(*googleapi.Error); ok && e.Code == 409 {
return true
} else if !ok && errwrap.ContainsType(err, &googleapi.Error{}) {
e := errwrap.GetType(err, &googleapi.Error{}).(*googleapi.Error)
if e.Code == 409 {
return true
}
}
return false
}

func linkDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
parts := strings.Split(old, "/")
if parts[len(parts)-1] == new {
Expand Down
246 changes: 246 additions & 0 deletions google/resource_google_project_iam_binding.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,246 @@
package google

import (
"fmt"
"log"
"time"

"github.com/hashicorp/terraform/helper/schema"
"google.golang.org/api/cloudresourcemanager/v1"
)

func resourceGoogleProjectIamBinding() *schema.Resource {
return &schema.Resource{
Create: resourceGoogleProjectIamBindingCreate,
Read: resourceGoogleProjectIamBindingRead,
Update: resourceGoogleProjectIamBindingUpdate,
Delete: resourceGoogleProjectIamBindingDelete,

Schema: map[string]*schema.Schema{
"project": &schema.Schema{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super minor nit: can remove the duplicate type declarations here, and the rest of the schema struct:

"project": { 

Type: schema.TypeString,
Optional: true,
ForceNew: true,
},
"role": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"members": &schema.Schema{
Type: schema.TypeSet,
Required: true,
Elem: &schema.Schema{
Type: schema.TypeString,
},
},
"etag": &schema.Schema{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're doing a read-modify-write, do we actually need to store the etag?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code doesn't use it for anything, but I couldn't think of a good reason not to expose it.

Type: schema.TypeString,
Computed: true,
},
},
}
}

func resourceGoogleProjectIamBindingCreate(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)
pid, err := getProject(d, config)
if err != nil {
return err
}

// Get the binding in the template
log.Println("[DEBUG]: Reading google_project_iam_binding")
p := getResourceIamBinding(d)
mutexKV.Lock(projectIamBindingMutexKey(pid, p.Role))
defer mutexKV.Unlock(projectIamBindingMutexKey(pid, p.Role))

for {
backoff := time.Second
// Get the existing bindings
log.Println("[DEBUG]: Retrieving policy for project", pid)
ep, err := getProjectIamPolicy(pid, config)
if err != nil {
return err
}
log.Printf("[DEBUG]: Retrieved policy for project %q: %+v\n", pid, ep)

// Merge the bindings together
ep.Bindings = mergeBindings(append(ep.Bindings, p))
log.Printf("[DEBUG]: Setting policy for project %q to %+v\n", pid, ep)
err = setProjectIamPolicy(ep, config, pid)
if err != nil && isConflictError(err) {
log.Printf("[DEBUG]: Concurrent policy changes, restarting read-modify-write after %s\n", backoff)
time.Sleep(backoff)
backoff = backoff * 2
if backoff > 30*time.Second {
return fmt.Errorf("Error applying IAM policy to project %q: too many concurrent policy changes.\n", pid)
}
continue
} else if err != nil {
return fmt.Errorf("Error applying IAM policy to project: %v", err)
}
break
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally up to you, but I think this structure would be a bit more clear:

for {
  // stuff
  if err == nil {
    // yay, successful update
    break
  } else if isConflictError(err) {
    // stuff
    continue
  } else {
    return fmt.Errorf(stuff)
}

}
log.Printf("[DEBUG]: Set policy for project %q", pid)
d.SetId(pid + ":" + p.Role)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of the resources we've been adding import to lately have settled on "/" as a separator for ids. Maybe do that for consistency?

return resourceGoogleProjectIamBindingRead(d, meta)
}

func resourceGoogleProjectIamBindingRead(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)
pid, err := getProject(d, config)
if err != nil {
return err
}

eBinding := getResourceIamBinding(d)

log.Println("[DEBUG]: Retrieving policy for project", pid)
p, err := getProjectIamPolicy(pid, config)
if err != nil {
return err
}
log.Printf("[DEBUG]: Retrieved policy for project %q: %+v\n", pid, p)

var binding *cloudresourcemanager.Binding
for _, b := range p.Bindings {
if b.Role != eBinding.Role {
continue
}
binding = b
break
}
if binding == nil {
d.SetId("")
return nil
}
d.Set("etag", p.Etag)
d.Set("members", binding.Members)
d.Set("role", binding.Role)
return nil
}

func resourceGoogleProjectIamBindingUpdate(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)
pid, err := getProject(d, config)
if err != nil {
return err
}

binding := getResourceIamBinding(d)
mutexKV.Lock(projectIamBindingMutexKey(pid, binding.Role))
defer mutexKV.Unlock(projectIamBindingMutexKey(pid, binding.Role))

for {
backoff := time.Second
log.Println("[DEBUG]: Retrieving policy for project", pid)
p, err := getProjectIamPolicy(pid, config)
if err != nil {
return err
}
log.Printf("[DEBUG]: Retrieved policy for project %q: %+v\n", pid, p)

var found bool
for pos, b := range p.Bindings {
if b.Role != binding.Role {
continue
}
found = true
p.Bindings[pos] = binding
break
}
if !found {
p.Bindings = append(p.Bindings, binding)
}

log.Printf("[DEBUG]: Setting policy for project %q to %+v\n", pid, p)
err = setProjectIamPolicy(p, config, pid)
if err != nil && isConflictError(err) {
log.Printf("[DEBUG]: Concurrent policy changes, restarting read-modify-write after %s\n", backoff)
time.Sleep(backoff)
backoff = backoff * 2
if backoff > 30*time.Second {
return fmt.Errorf("Error applying IAM policy to project %q: too many concurrent policy changes.\n", pid)
}
continue
} else if err != nil {
return fmt.Errorf("Error applying IAM policy to project: %v", err)
}
break
}
log.Printf("[DEBUG]: Set policy for project %q\n", pid)

return resourceGoogleProjectIamBindingRead(d, meta)
}

func resourceGoogleProjectIamBindingDelete(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)
pid, err := getProject(d, config)
if err != nil {
return err
}

binding := getResourceIamBinding(d)
mutexKV.Lock(projectIamBindingMutexKey(pid, binding.Role))
defer mutexKV.Unlock(projectIamBindingMutexKey(pid, binding.Role))

for {
backoff := time.Second
log.Println("[DEBUG]: Retrieving policy for project", pid)
p, err := getProjectIamPolicy(pid, config)
if err != nil {
return err
}
log.Printf("[DEBUG]: Retrieved policy for project %q: %+v\n", pid, p)

toRemove := -1
for pos, b := range p.Bindings {
if b.Role != binding.Role {
continue
}
toRemove = pos
break
}
if toRemove < 0 {
return resourceGoogleProjectIamBindingRead(d, meta)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we need to do another read here, or can we just set the id to "" and be done?

}

p.Bindings = append(p.Bindings[:toRemove], p.Bindings[toRemove+1:]...)

log.Printf("[DEBUG]: Setting policy for project %q to %+v\n", pid, p)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the policy seems to be done exactly the same in create, update, and delete- any way we could have a separate function that gets called by all 3?

err = setProjectIamPolicy(p, config, pid)
if err != nil && isConflictError(err) {
log.Printf("[DEBUG]: Concurrent policy changes, restarting read-modify-write after %s\n", backoff)
time.Sleep(backoff)
backoff = backoff * 2
if backoff > 30*time.Second {
return fmt.Errorf("Error applying IAM policy to project %q: too many concurrent policy changes.\n", pid)
}
continue
} else if err != nil {
return fmt.Errorf("Error applying IAM policy to project: %v", err)
}
break
}
log.Printf("[DEBUG]: Set policy for project %q\n", pid)

return resourceGoogleProjectIamBindingRead(d, meta)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of the final read?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updates the state file with the state according to the API. In this case, confirms the API no longer has the binding in it, and removes it. If for whatever reason the delete function silently failed and didn't actually delete the binding, Terraform wouldn't lose it.

}

// Get a cloudresourcemanager.Binding from a schema.ResourceData
func getResourceIamBinding(d *schema.ResourceData) *cloudresourcemanager.Binding {
members := d.Get("members").(*schema.Set).List()
m := make([]string, 0, len(members))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want you can do m := convertStringArr(members) (the fn is in a weird place right now but it's getting moved to provider.go to make it easier to find in #152

for _, member := range members {
m = append(m, member.(string))
}
return &cloudresourcemanager.Binding{
Members: m,
Role: d.Get("role").(string),
}
}

func projectIamBindingMutexKey(pid, role string) string {
return fmt.Sprintf("google-project-iam-binding-%s-%s", pid, role)
}
Loading