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

adding fix for aws org feature_set update issue from #15462 #15473

Merged
merged 5 commits into from
Nov 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 21 additions & 15 deletions aws/resource_aws_organizations_organization.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package aws

import (
"context"
"fmt"
"log"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/organizations"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
Expand All @@ -24,6 +26,13 @@ func resourceAwsOrganizationsOrganization() *schema.Resource {
State: schema.ImportStatePassthrough,
},

CustomizeDiff: customdiff.Sequence(
customdiff.ForceNewIfChange("feature_set", func(_ context.Context, old, new, meta interface{}) bool {
// Only changes from ALL to CONSOLIDATED_BILLING for feature_set should force a new resource
return old.(string) == organizations.OrganizationFeatureSetAll && new.(string) == organizations.OrganizationFeatureSetConsolidatedBilling
}),
),

Schema: map[string]*schema.Schema{
"arn": {
Type: schema.TypeString,
Expand Down Expand Up @@ -142,24 +151,15 @@ func resourceAwsOrganizationsOrganization() *schema.Resource {
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validation.StringInSlice([]string{
organizations.PolicyTypeAiservicesOptOutPolicy,
organizations.PolicyTypeBackupPolicy,
organizations.PolicyTypeServiceControlPolicy,
organizations.PolicyTypeTagPolicy,
}, false),
Type: schema.TypeString,
ValidateFunc: validation.StringInSlice(organizations.PolicyType_Values(), false),
},
},
"feature_set": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Default: organizations.OrganizationFeatureSetAll,
ValidateFunc: validation.StringInSlice([]string{
organizations.OrganizationFeatureSetAll,
organizations.OrganizationFeatureSetConsolidatedBilling,
}, true),
Type: schema.TypeString,
Optional: true,
Default: organizations.OrganizationFeatureSetAll,
ValidateFunc: validation.StringInSlice(organizations.OrganizationFeatureSet_Values(), true),
},
},
}
Expand Down Expand Up @@ -400,6 +400,12 @@ func resourceAwsOrganizationsOrganizationUpdate(d *schema.ResourceData, meta int
}
}

if d.HasChange("feature_set") {
if _, err := conn.EnableAllFeatures(&organizations.EnableAllFeaturesInput{}); err != nil {
return fmt.Errorf("error enabling all features in Organization (%s): %w", d.Id(), err)
}
}

return resourceAwsOrganizationsOrganizationRead(d, meta)
}

Expand Down
92 changes: 90 additions & 2 deletions aws/resource_aws_organizations_organization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,76 @@ func testAccAwsOrganizationsOrganization_FeatureSet(t *testing.T) {
})
}

func testAccAwsOrganizationsOrganization_FeatureSetForcesNew(t *testing.T) {
var beforeValue, afterValue organizations.Organization
resourceName := "aws_organizations_organization.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsOrganizationsOrganizationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsOrganizationsOrganizationConfigFeatureSet(organizations.OrganizationFeatureSetAll),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsOrganizationExists(resourceName, &beforeValue),
resource.TestCheckResourceAttr(resourceName, "feature_set", organizations.OrganizationFeatureSetAll),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAwsOrganizationsOrganizationConfigFeatureSet(organizations.OrganizationFeatureSetConsolidatedBilling),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsOrganizationExists(resourceName, &afterValue),
resource.TestCheckResourceAttr(resourceName, "feature_set", organizations.OrganizationFeatureSetConsolidatedBilling),
testAccAwsOrganizationsOrganizationRecreated(&beforeValue, &afterValue),
),
},
},
})
}

func testAccAwsOrganizationsOrganization_FeatureSetUpdate(t *testing.T) {
var beforeValue, afterValue organizations.Organization
resourceName := "aws_organizations_organization.test"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAwsOrganizationsOrganizationDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsOrganizationsOrganizationConfigFeatureSet(organizations.OrganizationFeatureSetConsolidatedBilling),
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsOrganizationExists(resourceName, &beforeValue),
resource.TestCheckResourceAttr(resourceName, "feature_set", organizations.OrganizationFeatureSetConsolidatedBilling),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAwsOrganizationsOrganizationConfigFeatureSet(organizations.OrganizationFeatureSetAll),
ExpectNonEmptyPlan: true, // See note below on this perpetual difference
Check: resource.ComposeTestCheckFunc(
testAccCheckAwsOrganizationsOrganizationExists(resourceName, &afterValue),
// The below check cannot be performed here because the user must confirm the change
// via Console. Until then, the FeatureSet will not actually be toggled to ALL
// and will continue to show as CONSOLIDATED_BILLING when calling DescribeOrganization
// resource.TestCheckResourceAttr(resourceName, "feature_set", organizations.OrganizationFeatureSetAll),
testAccAwsOrganizationsOrganizationNotRecreated(&beforeValue, &afterValue),
),
},
},
})
}

func testAccCheckAwsOrganizationsOrganizationDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).organizationsconn

Expand Down Expand Up @@ -232,7 +302,7 @@ func testAccCheckAwsOrganizationsOrganizationDestroy(s *terraform.State) error {
return nil
}

func testAccCheckAwsOrganizationsOrganizationExists(n string, a *organizations.Organization) resource.TestCheckFunc {
func testAccCheckAwsOrganizationsOrganizationExists(n string, org *organizations.Organization) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
Expand All @@ -256,7 +326,7 @@ func testAccCheckAwsOrganizationsOrganizationExists(n string, a *organizations.O
return fmt.Errorf("Organization %q does not exist", rs.Primary.ID)
}

a = resp.Organization
*org = *resp.Organization

return nil
}
Expand Down Expand Up @@ -354,3 +424,21 @@ func testFlattenOrganizationsRootPolicyTypes(t *testing.T, index int, result []m
}
}
}

func testAccAwsOrganizationsOrganizationRecreated(before, after *organizations.Organization) resource.TestCheckFunc {
return func(s *terraform.State) error {
if aws.StringValue(before.Id) == aws.StringValue(after.Id) {
return fmt.Errorf("Organization (%s) not recreated", aws.StringValue(before.Id))
}
return nil
}
}

func testAccAwsOrganizationsOrganizationNotRecreated(before, after *organizations.Organization) resource.TestCheckFunc {
return func(s *terraform.State) error {
if aws.StringValue(before.Id) != aws.StringValue(after.Id) {
return fmt.Errorf("Organization (%s) recreated", aws.StringValue(before.Id))
}
return nil
}
}
4 changes: 3 additions & 1 deletion aws/resource_aws_organizations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ func TestAccAWSOrganizations_serial(t *testing.T) {
"basic": testAccAwsOrganizationsOrganization_basic,
"AwsServiceAccessPrincipals": testAccAwsOrganizationsOrganization_AwsServiceAccessPrincipals,
"EnabledPolicyTypes": testAccAwsOrganizationsOrganization_EnabledPolicyTypes,
"FeatureSet": testAccAwsOrganizationsOrganization_FeatureSet,
"FeatureSet_Basic": testAccAwsOrganizationsOrganization_FeatureSet,
"FeatureSet_Update": testAccAwsOrganizationsOrganization_FeatureSetUpdate,
"FeatureSet_ForcesNew": testAccAwsOrganizationsOrganization_FeatureSetForcesNew,
"DataSource": testAccDataSourceAwsOrganizationsOrganization_basic,
},
"Account": {
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/organizations_organization.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ description: |-

Provides a resource to create an organization.

!> **WARNING:** When migrating from a `feature_set` of `CONSOLIDATED_BILLING` to `ALL`, the Organization account owner will received an email stating the following: "You started the process to enable all features for your AWS organization. As part of that process, all member accounts that joined your organization by invitation must approve the change. You don’t need approval from member accounts that you directly created from within your AWS organization." After all member accounts have accepted the invitation, the Organization account owner must then finalize the changes via the [AWS Console](https://console.aws.amazon.com/organizations/home#/organization/settings/migration-progress). Until these steps are performed, Terraform will perpetually show a difference, and the `DescribeOrganization` API will continue to show the `FeatureSet` as `CONSOLIDATED_BILLING`. See the [AWS Organizations documentation](https://docs.aws.amazon.com/organizations/latest/userguide/orgs_manage_org_support-all-features.html) for more information.

## Example Usage

```hcl
Expand Down