diff --git a/.changelog/4658.txt b/.changelog/4658.txt new file mode 100644 index 0000000000..0647518c11 --- /dev/null +++ b/.changelog/4658.txt @@ -0,0 +1,3 @@ +```release-note:bug +cloud_identity: fix google_cloud_identity_group_membership import/update +``` diff --git a/google-beta/resource_cloud_identity_group_membership.go b/google-beta/resource_cloud_identity_group_membership.go index 241f886136..53e02317ae 100644 --- a/google-beta/resource_cloud_identity_group_membership.go +++ b/google-beta/resource_cloud_identity_group_membership.go @@ -18,6 +18,7 @@ import ( "fmt" "log" "reflect" + "regexp" "time" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -50,20 +51,12 @@ func resourceCloudIdentityGroupMembership() *schema.Resource { Description: `The name of the Group to create this membership in.`, }, "roles": { - Type: schema.TypeList, + Type: schema.TypeSet, Required: true, Description: `The MembershipRoles that apply to the Membership. Must not contain duplicate MembershipRoles with the same name.`, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "name": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{"OWNER", "MANAGER", "MEMBER"}, false), - Description: `The name of the MembershipRole. Must be one of OWNER, MANAGER, MEMBER. Possible values: ["OWNER", "MANAGER", "MEMBER"]`, - }, - }, - }, + Elem: cloudidentityGroupMembershipRolesSchema(), + // Default schema.HashSchema is used. }, "member_key": { Type: schema.TypeList, @@ -170,6 +163,19 @@ and must be in the form of 'identitysources/{identity_source_id}'.`, } } +func cloudidentityGroupMembershipRolesSchema() *schema.Resource { + return &schema.Resource{ + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice([]string{"OWNER", "MANAGER", "MEMBER"}, false), + Description: `The name of the MembershipRole. Must be one of OWNER, MANAGER, MEMBER. Possible values: ["OWNER", "MANAGER", "MEMBER"]`, + }, + }, + } +} + func resourceCloudIdentityGroupMembershipCreate(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) userAgent, err := generateUserAgentString(d, config.userAgent) @@ -306,46 +312,44 @@ func resourceCloudIdentityGroupMembershipUpdate(d *schema.ResourceData, meta int billingProject := "" - obj := make(map[string]interface{}) - memberKeyProp, err := expandCloudIdentityGroupMembershipMemberKey(d.Get("member_key"), d, config) - if err != nil { - return err - } else if v, ok := d.GetOkExists("member_key"); !isEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, memberKeyProp)) { - obj["memberKey"] = memberKeyProp - } - preferredMemberKeyProp, err := expandCloudIdentityGroupMembershipPreferredMemberKey(d.Get("preferred_member_key"), d, config) - if err != nil { - return err - } else if v, ok := d.GetOkExists("preferred_member_key"); !isEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, preferredMemberKeyProp)) { - obj["preferredMemberKey"] = preferredMemberKeyProp - } - rolesProp, err := expandCloudIdentityGroupMembershipRoles(d.Get("roles"), d, config) - if err != nil { - return err - } else if v, ok := d.GetOkExists("roles"); !isEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, rolesProp)) { - obj["roles"] = rolesProp - } + d.Partial(true) - url, err := replaceVars(d, config, "{{CloudIdentityBasePath}}{{name}}") - if err != nil { - return err - } + if d.HasChange("roles") { + obj := make(map[string]interface{}) - log.Printf("[DEBUG] Updating GroupMembership %q: %#v", d.Id(), obj) + rolesProp, err := expandCloudIdentityGroupMembershipRoles(d.Get("roles"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("roles"); !isEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, rolesProp)) { + obj["roles"] = rolesProp + } - // err == nil indicates that the billing_project value was found - if bp, err := getBillingProject(d, config); err == nil { - billingProject = bp - } + obj, err = resourceCloudIdentityGroupMembershipUpdateEncoder(d, meta, obj) + if err != nil { + return err + } - res, err := sendRequestWithTimeout(config, "PUT", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutUpdate)) + url, err := replaceVars(d, config, "{{CloudIdentityBasePath}}{{name}}:modifyMembershipRoles") + if err != nil { + return err + } + + // err == nil indicates that the billing_project value was found + if bp, err := getBillingProject(d, config); err == nil { + billingProject = bp + } + + res, err := sendRequestWithTimeout(config, "POST", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return fmt.Errorf("Error updating GroupMembership %q: %s", d.Id(), err) + } else { + log.Printf("[DEBUG] Finished updating GroupMembership %q: %#v", d.Id(), res) + } - if err != nil { - return fmt.Errorf("Error updating GroupMembership %q: %s", d.Id(), err) - } else { - log.Printf("[DEBUG] Finished updating GroupMembership %q: %#v", d.Id(), res) } + d.Partial(false) + return resourceCloudIdentityGroupMembershipRead(d, meta) } @@ -382,18 +386,25 @@ func resourceCloudIdentityGroupMembershipDelete(d *schema.ResourceData, meta int func resourceCloudIdentityGroupMembershipImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { config := meta.(*Config) - - // current import_formats can't import fields with forward slashes in their value - if err := parseImportId([]string{"(?P.+)"}, d, config); err != nil { + if err := parseImportId([]string{ + "(?P.+)", + }, d, config); err != nil { return nil, err } - name := d.Get("name").(string) + // Replace import id for the resource id + id, err := replaceVars(d, config, "{{name}}") + if err != nil { + return nil, fmt.Errorf("Error constructing id: %s", err) + } + d.SetId(id) - if err := d.Set("name", name); err != nil { - return nil, fmt.Errorf("Error setting name: %s", err) + // Configure "group" property, which does not appear in the response body. + group := regexp.MustCompile(`groups/[^/]+`).FindString(id) + if err := d.Set("group", group); err != nil { + return nil, fmt.Errorf("Error setting group property: %s", err) } - d.SetId(name) + return []*schema.ResourceData{d}, nil } @@ -460,14 +471,14 @@ func flattenCloudIdentityGroupMembershipRoles(v interface{}, d *schema.ResourceD return v } l := v.([]interface{}) - transformed := make([]interface{}, 0, len(l)) + transformed := schema.NewSet(schema.HashResource(cloudidentityGroupMembershipRolesSchema()), []interface{}{}) for _, raw := range l { original := raw.(map[string]interface{}) if len(original) < 1 { // Do not include empty json objects coming back from the api continue } - transformed = append(transformed, map[string]interface{}{ + transformed.Add(map[string]interface{}{ "name": flattenCloudIdentityGroupMembershipRolesName(original["name"], d, config), }) } @@ -550,6 +561,7 @@ func expandCloudIdentityGroupMembershipPreferredMemberKeyNamespace(v interface{} } func expandCloudIdentityGroupMembershipRoles(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) { + v = v.(*schema.Set).List() l := v.([]interface{}) req := make([]interface{}, 0, len(l)) for _, raw := range l { @@ -574,3 +586,18 @@ func expandCloudIdentityGroupMembershipRoles(v interface{}, d TerraformResourceD func expandCloudIdentityGroupMembershipRolesName(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) { return v, nil } + +func resourceCloudIdentityGroupMembershipUpdateEncoder(d *schema.ResourceData, meta interface{}, obj map[string]interface{}) (map[string]interface{}, error) { + // Return object for modifyMembershipRoles (we build request object from scratch, without using `obj`) + b, a := d.GetChange("roles") + before := b.(*schema.Set) + after := a.(*schema.Set) + // ref: https://cloud.google.com/identity/docs/reference/rest/v1/groups.memberships/modifyMembershipRoles#request-body + addRoles := after.Difference(before).List() + var removeRoles []string + for _, r := range before.Difference(after).List() { + removeRoles = append(removeRoles, r.(map[string]interface{})["name"].(string)) + } + req := map[string]interface{}{"addRoles": addRoles, "removeRoles": removeRoles} + return req, nil +} diff --git a/google-beta/resource_cloud_identity_group_membership_test.go b/google-beta/resource_cloud_identity_group_membership_test.go index 34f7187abc..b289f8612c 100644 --- a/google-beta/resource_cloud_identity_group_membership_test.go +++ b/google-beta/resource_cloud_identity_group_membership_test.go @@ -6,6 +6,175 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) +func TestAccCloudIdentityGroupMembership_update(t *testing.T) { + t.Parallel() + + context := map[string]interface{}{ + "org_domain": getTestOrgDomainFromEnv(t), + "cust_id": getTestCustIdFromEnv(t), + "identity_user": getTestIdentityUserFromEnv(t), + "random_suffix": randString(t, 10), + } + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudIdentityGroupMembershipDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccCloudIdentityGroupMembership_update1(context), + }, + { + ResourceName: "google_cloud_identity_group_membership.basic", + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccCloudIdentityGroupMembership_update2(context), + }, + { + ResourceName: "google_cloud_identity_group_membership.basic", + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccCloudIdentityGroupMembership_update1(context), + }, + { + ResourceName: "google_cloud_identity_group_membership.basic", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func testAccCloudIdentityGroupMembership_update1(context map[string]interface{}) string { + return Nprintf(` +resource "google_cloud_identity_group" "group" { + display_name = "tf-test-my-identity-group%{random_suffix}" + + parent = "customers/%{cust_id}" + + group_key { + id = "tf-test-my-identity-group%{random_suffix}@%{org_domain}" + } + + labels = { + "cloudidentity.googleapis.com/groups.discussion_forum" = "" + } +} + +resource "google_cloud_identity_group_membership" "basic" { + group = google_cloud_identity_group.group.id + + preferred_member_key { + id = "%{identity_user}@%{org_domain}" + } + + roles { + name = "MEMBER" + } + +} +`, context) +} + +func testAccCloudIdentityGroupMembership_update2(context map[string]interface{}) string { + return Nprintf(` +resource "google_cloud_identity_group" "group" { + display_name = "tf-test-my-identity-group%{random_suffix}" + + parent = "customers/%{cust_id}" + + group_key { + id = "tf-test-my-identity-group%{random_suffix}@%{org_domain}" + } + + labels = { + "cloudidentity.googleapis.com/groups.discussion_forum" = "" + } +} + +resource "google_cloud_identity_group_membership" "basic" { + group = google_cloud_identity_group.group.id + + preferred_member_key { + id = "%{identity_user}@%{org_domain}" + } + + roles { + name = "MEMBER" + } + + roles { + name = "MANAGER" + } +} +`, context) +} + +func TestAccCloudIdentityGroupMembership_import(t *testing.T) { + t.Parallel() + + context := map[string]interface{}{ + "org_domain": getTestOrgDomainFromEnv(t), + "cust_id": getTestCustIdFromEnv(t), + "identity_user": getTestIdentityUserFromEnv(t), + "random_suffix": randString(t, 10), + } + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudIdentityGroupMembershipDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccCloudIdentityGroupMembership_import(context), + }, + { + ResourceName: "google_cloud_identity_group_membership.basic", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func testAccCloudIdentityGroupMembership_import(context map[string]interface{}) string { + return Nprintf(` +resource "google_cloud_identity_group" "group" { + display_name = "tf-test-my-identity-group%{random_suffix}" + + parent = "customers/%{cust_id}" + + group_key { + id = "tf-test-my-identity-group%{random_suffix}@%{org_domain}" + } + + labels = { + "cloudidentity.googleapis.com/groups.discussion_forum" = "" + } +} + +resource "google_cloud_identity_group_membership" "basic" { + group = google_cloud_identity_group.group.id + + preferred_member_key { + id = "%{identity_user}@%{org_domain}" + } + + roles { + name = "MEMBER" + } + + roles { + name = "MANAGER" + } +} +`, context) +} + func TestAccCloudIdentityGroupMembership_cloudIdentityGroupMembershipWithMemberKey(t *testing.T) { t.Parallel() @@ -24,10 +193,9 @@ func TestAccCloudIdentityGroupMembership_cloudIdentityGroupMembershipWithMemberK Config: testAccCloudIdentityGroupMembership_cloudIdentityGroupMembershipWithMemberKey(context), }, { - ResourceName: "google_cloud_identity_group_membership.cloud_identity_group_membership_basic", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"group"}, + ResourceName: "google_cloud_identity_group_membership.cloud_identity_group_membership_basic", + ImportState: true, + ImportStateVerify: true, }, }, }) @@ -96,10 +264,9 @@ func TestAccCloudIdentityGroupMembership_cloudIdentityGroupMembershipUserWithMem Config: testAccCloudIdentityGroupMembership_cloudIdentityGroupMembershipUserWithMemberKey(context), }, { - ResourceName: "google_cloud_identity_group_membership.cloud_identity_group_membership_basic", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"group"}, + ResourceName: "google_cloud_identity_group_membership.cloud_identity_group_membership_basic", + ImportState: true, + ImportStateVerify: true, }, }, }) diff --git a/google-beta/resource_dataflow_flex_template_job_test.go b/google-beta/resource_dataflow_flex_template_job_test.go index 0d037482e0..df04d28bb0 100644 --- a/google-beta/resource_dataflow_flex_template_job_test.go +++ b/google-beta/resource_dataflow_flex_template_job_test.go @@ -8,7 +8,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" - compute "google.golang.org/api/compute/v1" + "google.golang.org/api/compute/v1" ) func TestAccDataflowFlexTemplateJob_basic(t *testing.T) {