From cd1c4a4c9954916881f0825d7f40404c5d10b9e1 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Wed, 3 Feb 2021 01:05:25 -0500 Subject: [PATCH 1/2] include relay_state in the UpdatePermissionSet request if available --- .changelog/17423.txt | 3 + aws/resource_aws_ssoadmin_permission_set.go | 7 +- ...source_aws_ssoadmin_permission_set_test.go | 69 +++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 .changelog/17423.txt diff --git a/.changelog/17423.txt b/.changelog/17423.txt new file mode 100644 index 00000000000..96033f45acf --- /dev/null +++ b/.changelog/17423.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_ssoadmin_permission_set: Properly update resource with `relay_state` argument +``` diff --git a/aws/resource_aws_ssoadmin_permission_set.go b/aws/resource_aws_ssoadmin_permission_set.go index 5d2b7542f14..0ca51ce9ae2 100644 --- a/aws/resource_aws_ssoadmin_permission_set.go +++ b/aws/resource_aws_ssoadmin_permission_set.go @@ -194,8 +194,11 @@ func resourceAwsSsoAdminPermissionSetUpdate(d *schema.ResourceData, meta interfa input.Description = aws.String(d.Get("description").(string)) } - if d.HasChange("relay_state") { - input.RelayState = aws.String(d.Get("relay_state").(string)) + // The AWS SSO API requires we send the RelayState value regardless if it's unchanged + // else the existing Permission Set's RelayState value will be cleared + // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/17411 + if v, ok := d.GetOk("relay_state"); ok { + input.RelayState = aws.String(v.(string)) } if d.HasChange("session_duration") { diff --git a/aws/resource_aws_ssoadmin_permission_set_test.go b/aws/resource_aws_ssoadmin_permission_set_test.go index 315af037111..5ed652c5907 100644 --- a/aws/resource_aws_ssoadmin_permission_set_test.go +++ b/aws/resource_aws_ssoadmin_permission_set_test.go @@ -272,6 +272,47 @@ func TestAccAWSSSOAdminPermissionSet_updateSessionDuration(t *testing.T) { }) } +// TestAccAWSSSOAdminPermissionSet_relayState_updateSessionDuration validates +// the resource's unchanged values (primarily relay_state) after updating the session_duration argument +// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/17411 +func TestAccAWSSSOAdminPermissionSet_relayState_updateSessionDuration(t *testing.T) { + resourceName := "aws_ssoadmin_permission_set.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSSSOAdminInstances(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSSOAdminPermissionSetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSSOAdminPermissionSetRelayStateConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSOAdminPermissionSetExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "description", rName), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "relay_state", "https://example.com"), + resource.TestCheckResourceAttr(resourceName, "session_duration", "PT1H"), + ), + }, + { + Config: testAccAWSSSOAdminPermissionSetRelayStateConfig_updateSessionDuration(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSOAdminPermissionSetExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "description", rName), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "relay_state", "https://example.com"), + resource.TestCheckResourceAttr(resourceName, "session_duration", "PT2H"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccAWSSSOAdminPermissionSet_mixedPolicyAttachments(t *testing.T) { resourceName := "aws_ssoadmin_permission_set.test" rName := acctest.RandomWithPrefix("tf-acc-test") @@ -416,6 +457,34 @@ resource "aws_ssoadmin_permission_set" "test" { `, rName) } +func testAccAWSSSOAdminPermissionSetRelayStateConfig(rName string) string { + return fmt.Sprintf(` +data "aws_ssoadmin_instances" "test" {} + +resource "aws_ssoadmin_permission_set" "test" { + description = %[1]q + name = %[1]q + instance_arn = tolist(data.aws_ssoadmin_instances.test.arns)[0] + relay_state = "https://example.com" + session_duration = "PT1H" +} +`, rName) +} + +func testAccAWSSSOAdminPermissionSetRelayStateConfig_updateSessionDuration(rName string) string { + return fmt.Sprintf(` +data "aws_ssoadmin_instances" "test" {} + +resource "aws_ssoadmin_permission_set" "test" { + description = %[1]q + name = %[1]q + instance_arn = tolist(data.aws_ssoadmin_instances.test.arns)[0] + relay_state = "https://example.com" + session_duration = "PT2H" +} +`, rName) +} + func testAccAWSSSOAdminPermissionSetConfigTagsSingle(rName, tagKey1, tagValue1 string) string { return fmt.Sprintf(` data "aws_ssoadmin_instances" "test" {} From e6556f6d0536ed199c66921f8c2ac8176802fbd1 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Thu, 4 Feb 2021 15:40:41 -0500 Subject: [PATCH 2/2] use d.GetOk to retrieve input values on update --- aws/resource_aws_ssoadmin_permission_set.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/aws/resource_aws_ssoadmin_permission_set.go b/aws/resource_aws_ssoadmin_permission_set.go index 0ca51ce9ae2..15da5c76fc1 100644 --- a/aws/resource_aws_ssoadmin_permission_set.go +++ b/aws/resource_aws_ssoadmin_permission_set.go @@ -190,19 +190,21 @@ func resourceAwsSsoAdminPermissionSetUpdate(d *schema.ResourceData, meta interfa PermissionSetArn: aws.String(arn), } - if d.HasChange("description") { - input.Description = aws.String(d.Get("description").(string)) - } - // The AWS SSO API requires we send the RelayState value regardless if it's unchanged - // else the existing Permission Set's RelayState value will be cleared + // else the existing Permission Set's RelayState value will be cleared; + // for consistency, we'll check for the "presence of" instead of "if changed" for all input fields // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/17411 + + if v, ok := d.GetOk("description"); ok { + input.Description = aws.String(v.(string)) + } + if v, ok := d.GetOk("relay_state"); ok { input.RelayState = aws.String(v.(string)) } - if d.HasChange("session_duration") { - input.SessionDuration = aws.String(d.Get("session_duration").(string)) + if v, ok := d.GetOk("session_duration"); ok { + input.SessionDuration = aws.String(v.(string)) } _, err := conn.UpdatePermissionSet(input)