From 729e9fc501d3dee533de97b8a27cd5a2e22153e5 Mon Sep 17 00:00:00 2001 From: Paddy Date: Tue, 25 Jul 2017 11:43:49 -0700 Subject: [PATCH] Create an iam policy read/modify/write helper. We were repeating that logic a lot, so this helper just reads a policy, calls the passed modify function on the policy, then writes the policy back and takes care of the optimistic concurrency logic for the caller. So now all the caller has to do is the unique part, which is the modify function. --- google/resource_google_project_iam_binding.go | 92 +++---------------- google/resource_google_project_iam_policy.go | 38 ++++++++ 2 files changed, 53 insertions(+), 77 deletions(-) diff --git a/google/resource_google_project_iam_binding.go b/google/resource_google_project_iam_binding.go index ed0dcf56..f36e6e12 100644 --- a/google/resource_google_project_iam_binding.go +++ b/google/resource_google_project_iam_binding.go @@ -3,7 +3,6 @@ package google import ( "fmt" "log" - "time" "github.com/hashicorp/terraform/helper/schema" "google.golang.org/api/cloudresourcemanager/v1" @@ -55,35 +54,14 @@ func resourceGoogleProjectIamBindingCreate(d *schema.ResourceData, meta interfac 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) - + err = projectIamPolicyReadModifyWrite(d, config, pid, func(ep *cloudresourcemanager.Policy) error { // 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 { - // update was successful, yay - break - } else if isConflitError(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 - } - return fmt.Errorf("Error applying IAM policy to project: %v", err) + return nil + }) + if err != nil { + return err } - log.Printf("[DEBUG]: Set policy for project %q", pid) d.SetId(pid + ":" + p.Role) return resourceGoogleProjectIamBindingRead(d, meta) } @@ -133,15 +111,7 @@ func resourceGoogleProjectIamBindingUpdate(d *schema.ResourceData, meta interfac 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) - + err = projectIamPolicyReadModifyWrite(d, config, pid, func(p *cloudresourcemanager.Policy) error { var found bool for pos, b := range p.Bindings { if b.Role != binding.Role { @@ -154,23 +124,11 @@ func resourceGoogleProjectIamBindingUpdate(d *schema.ResourceData, meta interfac 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 + return nil + }) + if err != nil { + return err } - log.Printf("[DEBUG]: Set policy for project %q\n", pid) return resourceGoogleProjectIamBindingRead(d, meta) } @@ -186,15 +144,7 @@ func resourceGoogleProjectIamBindingDelete(d *schema.ResourceData, meta interfac 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) - + err = projectIamPolicyReadModifyWrite(d, config, pid, func(p *cloudresourcemanager.Policy) error { toRemove := -1 for pos, b := range p.Bindings { if b.Role != binding.Role { @@ -210,23 +160,11 @@ func resourceGoogleProjectIamBindingDelete(d *schema.ResourceData, meta interfac } p.Bindings = append(p.Bindings[:toRemove], p.Bindings[toRemove+1:]...) - - 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 + return nil + }) + if err != nil { + return err } - log.Printf("[DEBUG]: Set policy for project %q\n", pid) return resourceGoogleProjectIamBindingRead(d, meta) } diff --git a/google/resource_google_project_iam_policy.go b/google/resource_google_project_iam_policy.go index eea0f85c..b49710c6 100644 --- a/google/resource_google_project_iam_policy.go +++ b/google/resource_google_project_iam_policy.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "sort" + "time" "github.com/hashicorp/errwrap" "github.com/hashicorp/terraform/helper/schema" @@ -418,3 +419,40 @@ func (b sortableBindings) Swap(i, j int) { func (b sortableBindings) Less(i, j int) bool { return b[i].Role < b[j].Role } + +type iamPolicyModifyFunc func(p *cloudresourcemanager.Policy) error + +func projectIamPolicyReadModifyWrite(d *schema.ResourceData, config *Config, pid string, modify iamPolicyModifyFunc) error { + for { + backoff := time.Second + log.Printf("[DEBUG]: Retrieving policy for project %q\n", pid) + p, err := getProjectIamPolicy(pid, config) + if err != nil { + return err + } + log.Printf("[DEBUG]: Retrieved policy for project %q: %+v\n", pid, p) + + err = modify(p) + if err != nil { + return err + } + + log.Printf("[DEBUG]: Setting policy for project %q to %+v\n", pid, p) + err = setProjectIamPolicy(p, config, pid) + if err == nil { + break + } + if 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 + } + return fmt.Errorf("Error applying IAM policy to project: %v", err) + } + log.Printf("[DEBUG]: Set policy for project %q\n", pid) + return nil +}