From 253c128e858917d1e0d8f17fa9ba88ce89396318 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 12:45:19 -0400 Subject: [PATCH 01/41] r/servicecatalog_portfolio: Remove unnecessary length limits --- aws/resource_aws_servicecatalog_portfolio.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/aws/resource_aws_servicecatalog_portfolio.go b/aws/resource_aws_servicecatalog_portfolio.go index 43bfbff9ec8..58626b178ba 100644 --- a/aws/resource_aws_servicecatalog_portfolio.go +++ b/aws/resource_aws_servicecatalog_portfolio.go @@ -41,9 +41,8 @@ func resourceAwsServiceCatalogPortfolio() *schema.Resource { Computed: true, }, "name": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringLenBetween(1, 20), + Type: schema.TypeString, + Required: true, }, "description": { Type: schema.TypeString, @@ -52,9 +51,8 @@ func resourceAwsServiceCatalogPortfolio() *schema.Resource { ValidateFunc: validation.StringLenBetween(0, 2000), }, "provider_name": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.StringLenBetween(1, 20), + Type: schema.TypeString, + Optional: true, }, "tags": tagsSchema(), "tags_all": tagsSchemaComputed(), From 255f8c789eec6988c22662a1b938accebe19b129 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 12:45:42 -0400 Subject: [PATCH 02/41] provider: Add new servicecatalog resources --- aws/provider.go | 1 + 1 file changed, 1 insertion(+) diff --git a/aws/provider.go b/aws/provider.go index 9cb1937d824..5d807542caa 100644 --- a/aws/provider.go +++ b/aws/provider.go @@ -1006,6 +1006,7 @@ func Provider() *schema.Provider { "aws_securityhub_product_subscription": resourceAwsSecurityHubProductSubscription(), "aws_securityhub_standards_subscription": resourceAwsSecurityHubStandardsSubscription(), "aws_servicecatalog_portfolio": resourceAwsServiceCatalogPortfolio(), + "aws_servicecatalog_portfolio_share": resourceAwsServiceCatalogPortfolioShare(), "aws_servicecatalog_product": resourceAwsServiceCatalogProduct(), "aws_servicecatalog_tag_option": resourceAwsServiceCatalogTagOption(), "aws_service_discovery_http_namespace": resourceAwsServiceDiscoveryHttpNamespace(), From c17914e83651b9ec99eb84f384557cc841ab84e4 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 12:47:09 -0400 Subject: [PATCH 03/41] servicecatalog: Add status for orgs/portfolio_share --- .../service/servicecatalog/waiter/status.go | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/aws/internal/service/servicecatalog/waiter/status.go b/aws/internal/service/servicecatalog/waiter/status.go index 25ebfd22468..b879effe862 100644 --- a/aws/internal/service/servicecatalog/waiter/status.go +++ b/aws/internal/service/servicecatalog/waiter/status.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/service/servicecatalog" "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/servicecatalog/finder" ) func ProductStatus(conn *servicecatalog.ServiceCatalog, acceptLanguage, productID string) resource.StateRefreshFunc { @@ -68,3 +69,72 @@ func TagOptionStatus(conn *servicecatalog.ServiceCatalog, id string) resource.St return output.TagOptionDetail, servicecatalog.StatusAvailable, err } } + +func PortfolioShareStatusWithToken(conn *servicecatalog.ServiceCatalog, token string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + input := &servicecatalog.DescribePortfolioShareStatusInput{ + PortfolioShareToken: aws.String(token), + } + output, err := conn.DescribePortfolioShareStatus(input) + + if tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { + return nil, StatusNotFound, err + } + + if err != nil { + return nil, servicecatalog.ShareStatusError, fmt.Errorf("error describing portfolio share status: %w", err) + } + + if output == nil { + return nil, StatusUnavailable, fmt.Errorf("error describing portfolio share status: empty response") + } + + return output, aws.StringValue(output.Status), err + } +} + +func PortfolioShareStatus(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, accountID, orgNodeValue string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + output, err := finder.PortfolioShare(conn, portfolioID, shareType, accountID, orgNodeValue) + + if tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { + return nil, StatusNotFound, err + } + + if err != nil { + return nil, servicecatalog.ShareStatusError, fmt.Errorf("error finding portfolio share: %w", err) + } + + if output == nil { + return nil, StatusUnavailable, fmt.Errorf("error finding portfolio share: empty response") + } + + if !aws.BoolValue(output.Accepted) { + return output, servicecatalog.ShareStatusInProgress, err + } + + return output, servicecatalog.ShareStatusCompleted, err + } +} + +func OrganizationsAccessStatus(conn *servicecatalog.ServiceCatalog) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + input := &servicecatalog.GetAWSOrganizationsAccessStatusInput{} + + output, err := conn.GetAWSOrganizationsAccessStatus(input) + + if tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { + return nil, StatusNotFound, err + } + + if err != nil { + return nil, OrganizationAccessStatusError, fmt.Errorf("error getting Organizations Access: %w", err) + } + + if output == nil { + return nil, StatusUnavailable, fmt.Errorf("error getting Organizations Access: empty response") + } + + return output, aws.StringValue(output.AccessStatus), err + } +} From 4a6285513abf72c6ec23165daee2d0f8bcd4a75d Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 12:47:26 -0400 Subject: [PATCH 04/41] servicecatalog: Add waiter for orgs/portfolio_share --- .../service/servicecatalog/waiter/waiter.go | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/aws/internal/service/servicecatalog/waiter/waiter.go b/aws/internal/service/servicecatalog/waiter/waiter.go index 01b3f8a7341..2e74a79e515 100644 --- a/aws/internal/service/servicecatalog/waiter/waiter.go +++ b/aws/internal/service/servicecatalog/waiter/waiter.go @@ -15,11 +15,17 @@ const ( TagOptionReadyTimeout = 3 * time.Minute TagOptionDeleteTimeout = 3 * time.Minute + PortfolioShareCreateTimeout = 3 * time.Minute + + OrganizationsAccessStableTimeout = 3 * time.Minute + StatusNotFound = "NOT_FOUND" StatusUnavailable = "UNAVAILABLE" // AWS documentation is wrong, says that status will be "AVAILABLE" but it is actually "CREATED" ProductStatusCreated = "CREATED" + + OrganizationAccessStatusError = "ERROR" ) func ProductReady(conn *servicecatalog.ServiceCatalog, acceptLanguage, productID string) (*servicecatalog.DescribeProductAsAdminOutput, error) { @@ -89,3 +95,88 @@ func TagOptionDeleted(conn *servicecatalog.ServiceCatalog, id string) error { return err } + +func PortfolioShareReady(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, accountID, orgNodeValue string) (*servicecatalog.PortfolioShareDetail, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, StatusNotFound, StatusUnavailable}, + Target: []string{servicecatalog.ShareStatusCompleted}, + Refresh: PortfolioShareStatus(conn, portfolioID, shareType, accountID, orgNodeValue), + Timeout: PortfolioShareCreateTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*servicecatalog.PortfolioShareDetail); ok { + return output, err + } + + return nil, err +} + +func PortfolioShareCreatedWithToken(conn *servicecatalog.ServiceCatalog, token string) (*servicecatalog.DescribePortfolioShareStatusOutput, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, StatusNotFound, StatusUnavailable}, + Target: []string{servicecatalog.ShareStatusCompleted}, + Refresh: PortfolioShareStatusWithToken(conn, token), + Timeout: PortfolioShareCreateTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*servicecatalog.DescribePortfolioShareStatusOutput); ok { + return output, err + } + + return nil, err +} + +func PortfolioShareDeleted(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, accountID, orgNodeValue string) (*servicecatalog.PortfolioShareDetail, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, servicecatalog.ShareStatusCompleted, StatusUnavailable}, + Target: []string{StatusNotFound}, + Refresh: PortfolioShareStatus(conn, portfolioID, shareType, accountID, orgNodeValue), + Timeout: PortfolioShareCreateTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*servicecatalog.PortfolioShareDetail); ok { + return output, err + } + + return nil, err +} + +func PortfolioShareDeletedWithToken(conn *servicecatalog.ServiceCatalog, token string) (*servicecatalog.DescribePortfolioShareStatusOutput, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{servicecatalog.ShareStatusCompleted, servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, StatusNotFound, StatusUnavailable}, + Target: []string{StatusNotFound}, + Refresh: PortfolioShareStatusWithToken(conn, token), + Timeout: PortfolioShareCreateTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*servicecatalog.DescribePortfolioShareStatusOutput); ok { + return output, err + } + + return nil, err +} + +func OrganizationsAccessStable(conn *servicecatalog.ServiceCatalog) (string, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{servicecatalog.AccessStatusUnderChange, StatusNotFound, StatusUnavailable}, + Target: []string{servicecatalog.AccessStatusEnabled, servicecatalog.AccessStatusDisabled}, + Refresh: OrganizationsAccessStatus(conn), + Timeout: OrganizationsAccessStableTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(string); ok { + return output, err + } + + return "", err +} From 7455a842b3353adcea5d958d1433c04227d34cdf Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 12:48:11 -0400 Subject: [PATCH 05/41] servicecatalog: Add finder for portfolio share --- .../service/servicecatalog/finder/finder.go | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 aws/internal/service/servicecatalog/finder/finder.go diff --git a/aws/internal/service/servicecatalog/finder/finder.go b/aws/internal/service/servicecatalog/finder/finder.go new file mode 100644 index 00000000000..cf66526f7e1 --- /dev/null +++ b/aws/internal/service/servicecatalog/finder/finder.go @@ -0,0 +1,40 @@ +package finder + +import ( + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/servicecatalog" +) + +func PortfolioShare(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, accountID, orgNodeValue string) (*servicecatalog.PortfolioShareDetail, error) { + input := &servicecatalog.DescribePortfolioSharesInput{ + PortfolioId: aws.String(portfolioID), + Type: aws.String(shareType), + } + var result *servicecatalog.PortfolioShareDetail + + err := conn.DescribePortfolioSharesPages(input, func(page *servicecatalog.DescribePortfolioSharesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, deet := range page.PortfolioShareDetails { + if deet == nil { + continue + } + + if aws.StringValue(deet.PrincipalId) == accountID && accountID != "" { + result = deet + return false + } + + if aws.StringValue(deet.PrincipalId) == orgNodeValue && orgNodeValue != "" { + result = deet + return false + } + } + + return !lastPage + }) + + return result, err +} From 1758402fb9c738b820e56cb8ee37f8abef1adeaf Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 12:57:58 -0400 Subject: [PATCH 06/41] docs/r/servicecatalog_portfolio_share: Initial docs --- ...rvicecatalog_portfolio_share.html.markdown | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 website/docs/r/servicecatalog_portfolio_share.html.markdown diff --git a/website/docs/r/servicecatalog_portfolio_share.html.markdown b/website/docs/r/servicecatalog_portfolio_share.html.markdown new file mode 100644 index 00000000000..0887a4328e4 --- /dev/null +++ b/website/docs/r/servicecatalog_portfolio_share.html.markdown @@ -0,0 +1,65 @@ +--- +subcategory: "Service Catalog" +layout: "aws" +page_title: "AWS: aws_servicecatalog_portfolio_share" +description: |- + Manages a Service Catalog Portfolio Share +--- + +# Resource: aws_servicecatalog_portfolio_share + +Manages a Service Catalog Portfolio Share. Shares the specified portfolio with the specified account or organization node. You can share portfolios to an organization, an organizational unit, or a specific account. + +If the portfolio share with the specified account or organization node already exists, using this resource to re-create the share will have no effect and will not return an error. You can then use this resource to update the share. + +~> **NOTE:** Shares to an organization node can only be created by the management account of an organization or by a delegated administrator. If a delegated admin is de-registered, they can no longer create portfolio shares. + +~> **NOTE:** AWSOrganizationsAccess must be enabled in order to create a portfolio share to an organization node. + +~> **NOTE:** You can't share a shared resource, including portfolios that contain a shared product. + +## Example Usage + +### Basic Usage + +```terraform +resource "aws_servicecatalog_portfolio_share" "example" { + account_id = "012128675309" + portfolio_id = aws_servicecatalog_portfolio.example.id +} +``` + +## Argument Reference + +The following arguments are required: + +* `account_id` - (Required if `organization_node` is not included) AWS account ID. For example, `123456789012`. +* `organization_node` - (Required if `account_id` is not included) Configuration block with an organization node to whom you are going to share. Detailed below. +* `portfolio_id` - (Required) Portfolio identifier. +* `type` - (Required) Type of portfolio share. Valid values are `ACCOUNT` (an external account), `ORGANIZATION` (a share to every account in an organization), `ORGANIZATIONAL_UNIT`, `ORGANIZATION_MEMBER_ACCOUNT` (a share to an account in an organization). + +The following arguments are optional: + +* `accept_language` - (Optional) Language code. Valid values: `en` (English), `jp` (Japanese), `zh` (Chinese). Default value is `en`. +* `share_tag_options` - (Optional) Whether to enable sharing of `aws_servicecatalog_tag_option` resources when creating the portfolio share. + +### organization_node + +The following arguments are supported: + +* `type` - (Optional) Organization node type. Valid values are `ACCOUNT` (this value corresponds to `ORGANIZATION_MEMBER_ACCOUNT`), `ORGANIZATION`, and `ORGANIZATIONAL_UNIT`. +* `value` - (Optional) Identifier of the organization node. + +## Attributes Reference + +In addition to all arguments above, the following attributes are exported: + +* `accepted` - Whether the shared portfolio is imported by the recipient account. If the recipient is in an organization node, the share is automatically imported, and the field is always set to true. + +## Import + +`aws_servicecatalog_portfolio_share` can be imported using the portfolio share ID, e.g. + +``` +$ terraform import aws_servicecatalog_portfolio_share.example arn:aws:catalog:us-east-1:123456789012:portfolio share/prod-dnigbtea24ste +``` From 57650626b45846284b6cf27a42408b642e0274c2 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 12:58:58 -0400 Subject: [PATCH 07/41] r/servicecatalog_portfolio_share: New resource --- ...urce_aws_servicecatalog_portfolio_share.go | 379 ++++++++++++++++++ 1 file changed, 379 insertions(+) create mode 100644 aws/resource_aws_servicecatalog_portfolio_share.go diff --git a/aws/resource_aws_servicecatalog_portfolio_share.go b/aws/resource_aws_servicecatalog_portfolio_share.go new file mode 100644 index 00000000000..b87d99dfa04 --- /dev/null +++ b/aws/resource_aws_servicecatalog_portfolio_share.go @@ -0,0 +1,379 @@ +package aws + +import ( + "fmt" + "log" + "strings" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/servicecatalog" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "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" + iamwaiter "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter" + tfservicecatalog "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/servicecatalog" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/servicecatalog/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" +) + +func resourceAwsServiceCatalogPortfolioShare() *schema.Resource { + return &schema.Resource{ + Create: resourceAwsServiceCatalogPortfolioShareCreate, + Read: resourceAwsServiceCatalogPortfolioShareRead, + Update: resourceAwsServiceCatalogPortfolioShareUpdate, + Delete: resourceAwsServiceCatalogPortfolioShareDelete, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, + + Schema: map[string]*schema.Schema{ + "accept_language": { + Type: schema.TypeString, + Optional: true, + Default: "en", + ValidateFunc: validation.StringInSlice(tfservicecatalog.AcceptLanguage_Values(), false), + }, + "accepted": { + Type: schema.TypeBool, + Computed: true, + }, + "account_id": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validateAwsAccountId, + ExactlyOneOf: []string{ + "organization_node", + "account_id", + }, + }, + "organization_node": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + ExactlyOneOf: []string{ + "organization_node", + "account_id", + }, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "type": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringInSlice(servicecatalog.OrganizationNodeType_Values(), false), + }, + "value": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + "portfolio_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "share_tag_options": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "type": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(servicecatalog.DescribePortfolioShareType_Values(), false), + }, + }, + } +} + +func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).scconn + + input := &servicecatalog.CreatePortfolioShareInput{ + PortfolioId: aws.String(d.Get("portfolio_id").(string)), + } + + if v, ok := d.GetOk("accept_language"); ok { + input.AcceptLanguage = aws.String(v.(string)) + } + + if v, ok := d.GetOk("account_id"); ok { + input.AccountId = aws.String(v.(string)) + } + + if v, ok := d.GetOk("organization_node"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.OrganizationNode = expandServiceCatalogOrganizationNode(v.([]interface{})[0].(map[string]interface{})) + } + + if v, ok := d.GetOk("share_tag_options"); ok { + input.ShareTagOptions = aws.Bool(v.(bool)) + } + + var output *servicecatalog.CreatePortfolioShareOutput + err := resource.Retry(iamwaiter.PropagationTimeout, func() *resource.RetryError { + var err error + + output, err = conn.CreatePortfolioShare(input) + + if tfawserr.ErrMessageContains(err, servicecatalog.ErrCodeInvalidParametersException, "profile does not exist") { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if tfresource.TimedOut(err) { + output, err = conn.CreatePortfolioShare(input) + } + + if err != nil { + return fmt.Errorf("error creating Service Catalog Portfolio Share: %w", err) + } + + if output == nil { + return fmt.Errorf("error creating Service Catalog Portfolio Share: empty response") + } + + d.SetId(serviceCatalogPortfolioShareID( + d.Get("portfolio_id").(string), + d.Get("account_id").(string), + input.OrganizationNode, + )) + + // only get a token if organization node, otherwise check without token + if output.PortfolioShareToken != nil { + if _, err := waiter.PortfolioShareCreatedWithToken(conn, aws.StringValue(output.PortfolioShareToken)); err != nil { + return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be ready: %w", d.Id(), err) + } + } else { + orgNodeValue := "" + + if input.OrganizationNode != nil && input.OrganizationNode.Value != nil { + orgNodeValue = aws.StringValue(input.OrganizationNode.Value) + } + + if _, err := waiter.PortfolioShareReady(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("account_id").(string), orgNodeValue); err != nil { + return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be ready: %w", d.Id(), err) + } + } + + return resourceAwsServiceCatalogPortfolioShareRead(d, meta) +} + +func resourceAwsServiceCatalogPortfolioShareRead(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).scconn + + orgNodeValue := "" + + if v, ok := d.GetOk("organization_node"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + orgNode := expandServiceCatalogOrganizationNode(v.([]interface{})[0].(map[string]interface{})) + + if orgNode.Value != nil { + orgNodeValue = aws.StringValue(orgNode.Value) + } + } + + output, err := waiter.PortfolioShareReady(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("account_id").(string), orgNodeValue) + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { + log.Printf("[WARN] Service Catalog Portfolio Share (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return fmt.Errorf("error describing Service Catalog Portfolio Share (%s): %w", d.Id(), err) + } + + if output == nil { + return fmt.Errorf("error getting Service Catalog Portfolio Share (%s): empty response", d.Id()) + } + + d.Set("accepted", output.Accepted) + d.Set("share_tag_options", output.ShareTagOptions) + d.Set("type", output.Type) + + switch aws.StringValue(output.Type) { + case servicecatalog.DescribePortfolioShareTypeAccount: + d.Set("account_id", output.PrincipalId) + d.Set("organization_node", nil) + default: + d.Set("account_id", nil) + + orgNode := &servicecatalog.OrganizationNode{ + Type: output.Type, + Value: output.PrincipalId, + } + + if err := d.Set("organization_node", flattenServiceCatalogOrganizationNode(orgNode)); err != nil { + return fmt.Errorf("error setting organization_node: %w", err) + } + } + + return nil +} + +func resourceAwsServiceCatalogPortfolioShareUpdate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).scconn + + if d.HasChanges("accept_language", "account_id", "organization_node", "share_tag_options") { + input := &servicecatalog.UpdatePortfolioShareInput{ + PortfolioId: aws.String(d.Get("portfolio_id").(string)), + } + + if v, ok := d.GetOk("accept_language"); ok { + input.AcceptLanguage = aws.String(v.(string)) + } + + if v, ok := d.GetOk("account_id"); ok { + input.AccountId = aws.String(v.(string)) + } + + if v, ok := d.GetOk("organization_node"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.OrganizationNode = expandServiceCatalogOrganizationNode(v.([]interface{})[0].(map[string]interface{})) + } + + if v, ok := d.GetOk("share_tag_options"); ok { + input.ShareTagOptions = aws.Bool(v.(bool)) + } + + err := resource.Retry(iamwaiter.PropagationTimeout, func() *resource.RetryError { + _, err := conn.UpdatePortfolioShare(input) + + if tfawserr.ErrMessageContains(err, servicecatalog.ErrCodeInvalidParametersException, "profile does not exist") { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if tfresource.TimedOut(err) { + _, err = conn.UpdatePortfolioShare(input) + } + + if err != nil { + return fmt.Errorf("error updating Service Catalog Portfolio Share (%s): %w", d.Id(), err) + } + } + + return resourceAwsServiceCatalogPortfolioShareRead(d, meta) +} + +func resourceAwsServiceCatalogPortfolioShareDelete(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).scconn + + input := &servicecatalog.DeletePortfolioShareInput{ + PortfolioId: aws.String(d.Get("portfolio_id").(string)), + } + + if v, ok := d.GetOk("accept_language"); ok { + input.AcceptLanguage = aws.String(v.(string)) + } + + if v, ok := d.GetOk("account_id"); ok { + input.AccountId = aws.String(v.(string)) + } + + if v, ok := d.GetOk("organization_node"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.OrganizationNode = expandServiceCatalogOrganizationNode(v.([]interface{})[0].(map[string]interface{})) + } + + output, err := conn.DeletePortfolioShare(input) + + if tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { + return nil + } + + if err != nil { + return fmt.Errorf("error deleting Service Catalog Portfolio Share (%s): %w", d.Id(), err) + } + + // only get a token if organization node, otherwise check without token + if output.PortfolioShareToken != nil { + if _, err := waiter.PortfolioShareDeletedWithToken(conn, aws.StringValue(output.PortfolioShareToken)); err != nil { + return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be deleted: %w", d.Id(), err) + } + } else { + orgNodeValue := "" + + if input.OrganizationNode != nil && input.OrganizationNode.Value != nil { + orgNodeValue = aws.StringValue(input.OrganizationNode.Value) + } + + if _, err := waiter.PortfolioShareDeleted(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("account_id").(string), orgNodeValue); err != nil { + return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be deleted: %w", d.Id(), err) + } + } + + return nil +} + +func expandServiceCatalogOrganizationNode(tfMap map[string]interface{}) *servicecatalog.OrganizationNode { + if tfMap == nil { + return nil + } + + apiObject := &servicecatalog.OrganizationNode{} + + if v, ok := tfMap["type"].(string); ok && v != "" { + apiObject.Type = aws.String(v) + } + + if v, ok := tfMap["value"].(string); ok && v != "" { + apiObject.Value = aws.String(v) + } + + return apiObject +} + +func flattenServiceCatalogOrganizationNode(apiObject *servicecatalog.OrganizationNode) map[string]interface{} { + if apiObject == nil { + return nil + } + + tfMap := map[string]interface{}{} + + if v := apiObject.Type; v != nil { + tfMap["type"] = aws.StringValue(v) + } + + if v := apiObject.Value; v != nil { + tfMap["value"] = aws.StringValue(v) + } + + return tfMap +} + +func serviceCatalogPortfolioShareID(portfolioID, accountID string, orgNode *servicecatalog.OrganizationNode) string { + var pieces []string + + if portfolioID != "" { + pieces = append(pieces, portfolioID) + } + + if accountID != "" { + pieces = append(pieces, accountID) + } + + if orgNode != nil { + if orgNode.Type != nil { + pieces = append(pieces, aws.StringValue(orgNode.Type)) + } + + if orgNode.Value != nil { + pieces = append(pieces, aws.StringValue(orgNode.Value)) + } + } + + return strings.Join(pieces, ":") +} From 9cee5633e7fe7059a686a3cfe89aeba858a94a7b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 12:59:39 -0400 Subject: [PATCH 08/41] tests/r/servicecatalog_portfolio_share: Tests for new resource --- ...aws_servicecatalog_portfolio_share_test.go | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 aws/resource_aws_servicecatalog_portfolio_share_test.go diff --git a/aws/resource_aws_servicecatalog_portfolio_share_test.go b/aws/resource_aws_servicecatalog_portfolio_share_test.go new file mode 100644 index 00000000000..d631954d3c3 --- /dev/null +++ b/aws/resource_aws_servicecatalog_portfolio_share_test.go @@ -0,0 +1,139 @@ +package aws + +import ( + "fmt" + "testing" + + "github.com/aws/aws-sdk-go/service/servicecatalog" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/servicecatalog/finder" +) + +func TestAccAWSServiceCatalogPortfolioShare_basic(t *testing.T) { + resourceName := "aws_servicecatalog_portfolio_share.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, servicecatalog.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckAwsServiceCatalogPortfolioShareDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSServiceCatalogPortfolioShareConfig_basic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "accept_language", "en"), + resource.TestCheckResourceAttr(resourceName, "accepted", "true"), + resource.TestCheckResourceAttr(resourceName, "account_id", "distributör"), + resource.TestCheckResourceAttr(resourceName, "organization_node.#", "1"), + resource.TestCheckResourceAttr(resourceName, "organization_node.0.type", servicecatalog.DescribePortfolioShareTypeOrganization), + resource.TestCheckResourceAttr(resourceName, "organization_node.0.value", "true"), + resource.TestCheckResourceAttr(resourceName, "portfolio_id", "8675309"), + resource.TestCheckResourceAttr(resourceName, "share_tag_options", "true"), + resource.TestCheckResourceAttr(resourceName, "type", servicecatalog.DescribePortfolioShareTypeOrganization), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "accept_language", + "provisioning_artifact_parameters", + }, + }, + }, + }) +} + +func testAccCheckAwsServiceCatalogPortfolioShareDestroy(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).scconn + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_servicecatalog_portfolio_share" { + continue + } + + output, err := finder.PortfolioShare( + conn, + rs.Primary.Attributes["portfolio_id"], + rs.Primary.Attributes["share_type"], + rs.Primary.Attributes["account_id"], + rs.Primary.Attributes["organization_node.0.value"], + ) + + if tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { + return nil + } + + if err != nil { + return fmt.Errorf("error getting Service Catalog Portfolio Share (%s): %w", rs.Primary.ID, err) + } + + if output != nil { + return fmt.Errorf("Service Catalog Portfolio Share (%s) still exists", rs.Primary.ID) + } + } + + return nil +} + +func testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[resourceName] + + if !ok { + return fmt.Errorf("resource not found: %s", resourceName) + } + + conn := testAccProvider.Meta().(*AWSClient).scconn + + _, err := finder.PortfolioShare( + conn, + rs.Primary.Attributes["portfolio_id"], + rs.Primary.Attributes["share_type"], + rs.Primary.Attributes["account_id"], + rs.Primary.Attributes["organization_node.0.value"], + ) + + if tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { + return fmt.Errorf("Service Catalog Portfolio Share (%s) not found", rs.Primary.ID) + } + + if err != nil { + return fmt.Errorf("error getting Service Catalog Portfolio Share (%s): %w", rs.Primary.ID, err) + } + + return nil + } +} + +func testAccAWSServiceCatalogPortfolioShareConfig_basic(rName string) string { + return fmt.Sprintf(` +resource "aws_servicecatalog_organizations_access" "test" { + enabled = "true" +} + +resource "aws_servicecatalog_portfolio" "test" { + name = %[1]q + description = %[1]q + provider_name = %[1]q +} + +resource "aws_servicecatalog_portfolio_share" "test" { + accept_language = "en" + portfolio_id = aws_servicecatalog_portfolio.test.id + share_tag_options = true + type = "ORGANIZATION" + + organization_node { + type = "ORGANIZATION" + value = "arn:aws:organizations::111122223333:organization/o-abcdefghijkl" + } +} +`, rName) +} From faa71ad437c5ce40a27c1f8807bb6cddfcf38c51 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 13:00:16 -0400 Subject: [PATCH 09/41] docs/r/servicecatalog_organizations_access: Docs --- ...catalog_organizations_access.html.markdown | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 website/docs/r/servicecatalog_organizations_access.html.markdown diff --git a/website/docs/r/servicecatalog_organizations_access.html.markdown b/website/docs/r/servicecatalog_organizations_access.html.markdown new file mode 100644 index 00000000000..1347e9faf33 --- /dev/null +++ b/website/docs/r/servicecatalog_organizations_access.html.markdown @@ -0,0 +1,35 @@ +--- +subcategory: "Service Catalog" +layout: "aws" +page_title: "AWS: aws_servicecatalog_organizations_access" +description: |- + Manages Service Catalog Organizations Access +--- + +# Resource: aws_servicecatalog_organizations_access + +Manages Service Catalog AWS Organizations Access, a portfolio sharing feature through AWS Organizations. This allows Service Catalog to receive updates on your organization in order to sync your shares with the current structure. This resource will prompt AWS to set `organizations:EnableAWSServiceAccess` on your behalf so that your shares can be in sync with any changes in your AWS Organizations structure. + +~> **NOTE:** This resource can only be used by the management account in the organization. In other words, a delegated administrator is not authorized to use the resource. + +## Example Usage + +### Basic Usage + +```terraform +resource "aws_servicecatalog_organizations_access" "example" { + enabled = "true" +} +``` + +## Argument Reference + +The following arguments are required: + +* `enabled` - (Required) Whether to enable AWS Organizations access. + +## Attributes Reference + +In addition to all arguments above, the following attributes are exported: + +* `id` - Account ID for the account using the resource. From fdbfe4a1b14a51f625e5dc8bc37021f89115f62e Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 13:01:02 -0400 Subject: [PATCH 10/41] tests/r/servicecatalog_organizations_acess: Test new resource --- ...ervicecatalog_organizations_access_test.go | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 aws/resource_aws_servicecatalog_organizations_access_test.go diff --git a/aws/resource_aws_servicecatalog_organizations_access_test.go b/aws/resource_aws_servicecatalog_organizations_access_test.go new file mode 100644 index 00000000000..c93d8894b4c --- /dev/null +++ b/aws/resource_aws_servicecatalog_organizations_access_test.go @@ -0,0 +1,95 @@ +package aws + +import ( + "fmt" + "testing" + + "github.com/aws/aws-sdk-go/service/servicecatalog" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/servicecatalog/waiter" +) + +func TestAccAWSServiceCatalogOrganizationsAccess_basic(t *testing.T) { + resourceName := "aws_servicecatalog_organizations_access.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, servicecatalog.EndpointsID), + Providers: testAccProviders, + CheckDestroy: testAccCheckAwsServiceCatalogOrganizationsAccessDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSServiceCatalogOrganizationsAccessConfig_basic(), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsServiceCatalogOrganizationsAccessExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "enabled", "true"), + ), + }, + }, + }) +} + +func testAccCheckAwsServiceCatalogOrganizationsAccessDestroy(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).scconn + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_servicecatalog_organizations_access" { + continue + } + + output, err := waiter.OrganizationsAccessStable(conn) + + if err != nil { + return fmt.Errorf("error describing Service Catalog AWS Organizations Access (%s): %w", rs.Primary.ID, err) + } + + if output == "" { + return fmt.Errorf("error getting Service Catalog AWS Organizations Access (%s): empty response", rs.Primary.ID) + } + + return nil + } + + return nil +} + +func testAccCheckAwsServiceCatalogOrganizationsAccessExists(resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[resourceName] + + if !ok { + return fmt.Errorf("resource not found: %s", resourceName) + } + + conn := testAccProvider.Meta().(*AWSClient).scconn + + output, err := waiter.OrganizationsAccessStable(conn) + + if err != nil { + return fmt.Errorf("error describing Service Catalog AWS Organizations Access (%s): %w", rs.Primary.ID, err) + } + + if output == "" { + return fmt.Errorf("error getting Service Catalog AWS Organizations Access (%s): empty response", rs.Primary.ID) + } + + if output != servicecatalog.AccessStatusEnabled && rs.Primary.Attributes["enabled"] == "true" { + return fmt.Errorf("error getting Service Catalog AWS Organizations Access (%s): wrong setting", rs.Primary.ID) + } + + if output == servicecatalog.AccessStatusEnabled && rs.Primary.Attributes["enabled"] == "false" { + return fmt.Errorf("error getting Service Catalog AWS Organizations Access (%s): wrong setting", rs.Primary.ID) + } + + return nil + } +} + +func testAccAWSServiceCatalogOrganizationsAccessConfig_basic() string { + return ` +resource "aws_servicecatalog_organizations_access" "test" { + enabled = "true" +} +` +} From 63e85357aab4c44ffe00adf9c85c132c76001152 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 13:01:23 -0400 Subject: [PATCH 11/41] r/servicecatalog_organizations_acess: New resource --- ...aws_servicecatalog_organizations_access.go | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 aws/resource_aws_servicecatalog_organizations_access.go diff --git a/aws/resource_aws_servicecatalog_organizations_access.go b/aws/resource_aws_servicecatalog_organizations_access.go new file mode 100644 index 00000000000..d00d0f144aa --- /dev/null +++ b/aws/resource_aws_servicecatalog_organizations_access.go @@ -0,0 +1,108 @@ +package aws + +import ( + "fmt" + "log" + + "github.com/aws/aws-sdk-go/service/servicecatalog" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/servicecatalog/waiter" +) + +func resourceAwsServiceCatalogOrganizationsAccess() *schema.Resource { + return &schema.Resource{ + Create: resourceAwsServiceCatalogOrganizationsAccessCreate, + Read: resourceAwsServiceCatalogOrganizationsAccessRead, + Delete: resourceAwsServiceCatalogOrganizationsAccessDelete, + + Schema: map[string]*schema.Schema{ + "enabled": { + Type: schema.TypeBool, + Required: true, + ForceNew: true, + }, + }, + } +} + +func resourceAwsServiceCatalogOrganizationsAccessCreate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).scconn + + // During create, if enabled = "true", then Enable Access and vice versa + // During delete, the opposite + + if _, ok := d.GetOk("enabled"); ok { + _, err := conn.EnableAWSOrganizationsAccess(&servicecatalog.EnableAWSOrganizationsAccessInput{}) + + if err != nil { + return fmt.Errorf("error enabling Service Catalog AWS Organizations Access: %w", err) + } + + return resourceAwsServiceCatalogOrganizationsAccessRead(d, meta) + } + + _, err := conn.DisableAWSOrganizationsAccess(&servicecatalog.DisableAWSOrganizationsAccessInput{}) + + if err != nil { + return fmt.Errorf("error enabling Service Catalog AWS Organizations Access: %w", err) + } + + d.SetId(meta.(*AWSClient).accountid) + + return resourceAwsServiceCatalogOrganizationsAccessRead(d, meta) +} + +func resourceAwsServiceCatalogOrganizationsAccessRead(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).scconn + + output, err := waiter.OrganizationsAccessStable(conn) + + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { + // theoretically this should not be possible + log.Printf("[WARN] Service Catalog Organizations Access (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return fmt.Errorf("error describing Service Catalog AWS Organizations Access (%s): %w", d.Id(), err) + } + + if output == "" { + return fmt.Errorf("error getting Service Catalog AWS Organizations Access (%s): empty response", d.Id()) + } + + if output == servicecatalog.AccessStatusEnabled { + d.Set("enabled", true) + return nil + } + + d.Set("enabled", false) + return nil +} + +func resourceAwsServiceCatalogOrganizationsAccessDelete(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).scconn + + // During create, if enabled = "true", then Enable Access and vice versa + // During delete, the opposite + + if _, ok := d.GetOk("enabled"); !ok { + _, err := conn.EnableAWSOrganizationsAccess(&servicecatalog.EnableAWSOrganizationsAccessInput{}) + + if err != nil { + return fmt.Errorf("error disabling Service Catalog AWS Organizations Access: %w", err) + } + + return resourceAwsServiceCatalogOrganizationsAccessRead(d, meta) + } + + _, err := conn.DisableAWSOrganizationsAccess(&servicecatalog.DisableAWSOrganizationsAccessInput{}) + + if err != nil { + return fmt.Errorf("error disabling Service Catalog AWS Organizations Access: %w", err) + } + + return resourceAwsServiceCatalogOrganizationsAccessRead(d, meta) +} From cdf8b43bbf3220301961cba832a498b416576e79 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 13:04:21 -0400 Subject: [PATCH 12/41] r/servicecatalog_portfolio_share: Add changelog --- .changelog/19278.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/19278.txt diff --git a/.changelog/19278.txt b/.changelog/19278.txt new file mode 100644 index 00000000000..2d83b0f17b7 --- /dev/null +++ b/.changelog/19278.txt @@ -0,0 +1,7 @@ +```release-note:new-resource +aws_servicecatalog_organizations_access +``` + +```release-note:new-resource +aws_servicecatalog_portfolio_share +``` \ No newline at end of file From 0a3598e620bc81440f19a4f343cd35fac99b2cf8 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 14:56:49 -0400 Subject: [PATCH 13/41] tests/validators: Add test for servicecatalog principal --- aws/validators_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/aws/validators_test.go b/aws/validators_test.go index 1102b44fdd6..835ad78bc0b 100644 --- a/aws/validators_test.go +++ b/aws/validators_test.go @@ -436,6 +436,58 @@ func TestValidatePrincipal(t *testing.T) { } } +func TestValidateServiceCatalogSharePrincipal(t *testing.T) { + v := "" + _, errors := validateServiceCatalogSharePrincipal(v, "arn") + if len(errors) == 0 { + t.Fatalf("%q should not be validated as a principal %d: %q", v, len(errors), errors) + } + + validNames := []string{ + "123456789012", // lintignore:AWSAT005 // Example Account ID (Valid looking but not real) + "111122223333", // lintignore:AWSAT005 // Example Account ID (Valid looking but not real) + "arn:aws:organizations::111122223333:organization/o-abcdefghijkl", // lintignore:AWSAT005 // organization + "arn:aws:organizations::111122223333:ou/o-abcdefghijkl/ou-ab00-cdefgh", // lintignore:AWSAT005 // ou + "arn:aws-us-gov:organizations::111122223333:ou/o-abcdefghijkl/ou-ab00-cdefgh", // lintignore:AWSAT005 // GovCloud ou + } + for _, v := range validNames { + _, errors := validateServiceCatalogSharePrincipal(v, "arn") + if len(errors) != 0 { + t.Fatalf("%q should be a valid principal: %q", v, errors) + } + } + + invalidNames := []string{ + "IAM_ALLOWED_PRINCIPALS", // Special principal + "IAM_NOT_ALLOWED_PRINCIPALS", // doesn't exist + "arn", + "1234567890125", //not an account id + "arn:aws", + "arn:aws:logs", //lintignore:AWSAT005 + "arn:aws:logs:region:*:*", //lintignore:AWSAT005 + "arn:aws:elasticbeanstalk:us-east-1:123456789012:environment/My App/MyEnvironment", // lintignore:AWSAT003,AWSAT005 // not a user or role + "arn:aws:iam::aws:policy/CloudWatchReadOnlyAccess", // lintignore:AWSAT005 // not a user or role + "arn:aws:rds:eu-west-1:123456789012:db:mysql-db", // lintignore:AWSAT003,AWSAT005 // not a user or role + "arn:aws:s3:::my_corporate_bucket/exampleobject.png", // lintignore:AWSAT005 // not a user or role + "arn:aws:events:us-east-1:319201112229:rule/rule_name", // lintignore:AWSAT003,AWSAT005 // not a user or role + "arn:aws-us-gov:ec2:us-gov-west-1:123456789012:instance/i-12345678", // lintignore:AWSAT003,AWSAT005 // not a user or role + "arn:aws-us-gov:s3:::bucket/object", // lintignore:AWSAT005 // not a user or role + "arn:aws-us-gov:iam::357342307427:role/tf-acc-test-3217321001347236965", // lintignore:AWSAT005 // IAM Role + "arn:aws:iam::123456789012:user/David", // lintignore:AWSAT005 // IAM User + "arn:aws-us-gov:iam:us-west-2:357342307427:role/tf-acc-test-3217321001347236965", // lintignore:AWSAT003,AWSAT005 // Non-global IAM Role? + "arn:aws:iam:us-east-1:123456789012:user/David", // lintignore:AWSAT003,AWSAT005 // Non-global IAM User? + "arn:aws:iam::111122223333:saml-provider/idp1:group/data-scientists", // lintignore:AWSAT005 // SAML group + "arn:aws:iam::111122223333:saml-provider/idp1:user/Paul", // lintignore:AWSAT005 // SAML user + "arn:aws:quicksight:us-east-1:111122223333:group/default/data_scientists", // lintignore:AWSAT003,AWSAT005 // quicksight group + } + for _, v := range invalidNames { + _, errors := validateServiceCatalogSharePrincipal(v, "arn") + if len(errors) == 0 { + t.Fatalf("%q should be an invalid principal", v) + } + } +} + func TestValidateEC2AutomateARN(t *testing.T) { validNames := []string{ "arn:aws:automate:us-east-1:ec2:reboot", //lintignore:AWSAT003,AWSAT005 From 7fbba43b8b3e6d44357dbc00a90ed0a77fcc0420 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 14:57:08 -0400 Subject: [PATCH 14/41] validators: Add validator for servicecatalog principal --- aws/validators.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/aws/validators.go b/aws/validators.go index f997a79dd7f..f2b09daad56 100644 --- a/aws/validators.go +++ b/aws/validators.go @@ -697,6 +697,32 @@ func validatePrincipal(v interface{}, k string) (ws []string, errors []error) { return ws, errors } +func validateServiceCatalogSharePrincipal(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + // either account ID, or organization or organization unit + + wsAccount, errorsAccount := validateAwsAccountId(v, k) + + if len(errorsAccount) == 0 { + return wsAccount, errorsAccount + } + + wsARN, errorsARN := validateArn(v, k) + ws = append(ws, wsARN...) + errors = append(errors, errorsARN...) + + pattern := `^arn:[\w-]+:organizations:.*:(ou|organization)/` + if !regexp.MustCompile(pattern).MatchString(value) { + errors = append(errors, fmt.Errorf("%q does not look like an OU or organization: %q", k, value)) + } + + if len(errors) > 0 { + errors = append(errors, errorsAccount...) + } + + return ws, errors +} + func validateArn(v interface{}, k string) (ws []string, errors []error) { value := v.(string) From 26e8e38156d80bff0d44bd6d314dbc03d64781ae Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 14:59:44 -0400 Subject: [PATCH 15/41] r/servicecatalog_portfolio_share: Rework schema to simplify --- ...urce_aws_servicecatalog_portfolio_share.go | 198 ++++-------------- 1 file changed, 41 insertions(+), 157 deletions(-) diff --git a/aws/resource_aws_servicecatalog_portfolio_share.go b/aws/resource_aws_servicecatalog_portfolio_share.go index b87d99dfa04..cf504081593 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share.go +++ b/aws/resource_aws_servicecatalog_portfolio_share.go @@ -38,42 +38,19 @@ func resourceAwsServiceCatalogPortfolioShare() *schema.Resource { Type: schema.TypeBool, Computed: true, }, - "account_id": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validateAwsAccountId, - ExactlyOneOf: []string{ - "organization_node", - "account_id", - }, - }, - "organization_node": { - Type: schema.TypeList, - Optional: true, - MaxItems: 1, - ExactlyOneOf: []string{ - "organization_node", - "account_id", - }, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "type": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.StringInSlice(servicecatalog.OrganizationNodeType_Values(), false), - }, - "value": { - Type: schema.TypeString, - Optional: true, - }, - }, - }, - }, "portfolio_id": { Type: schema.TypeString, Required: true, ForceNew: true, }, + // maintaining organization_node as a separate config block makes weird configs with duplicate types + // also, principal_id is true to API since describe gives "PrincipalId" + "principal_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validateServiceCatalogSharePrincipal, + }, "share_tag_options": { Type: schema.TypeBool, Optional: true, @@ -82,6 +59,7 @@ func resourceAwsServiceCatalogPortfolioShare() *schema.Resource { "type": { Type: schema.TypeString, Required: true, + ForceNew: true, ValidateFunc: validation.StringInSlice(servicecatalog.DescribePortfolioShareType_Values(), false), }, }, @@ -99,12 +77,20 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta input.AcceptLanguage = aws.String(v.(string)) } - if v, ok := d.GetOk("account_id"); ok { - input.AccountId = aws.String(v.(string)) - } + if v, ok := d.GetOk("type"); ok && v.(string) == servicecatalog.DescribePortfolioShareTypeAccount { + input.AccountId = aws.String(d.Get("principal_id").(string)) + } else { + orgNode := &servicecatalog.OrganizationNode{} + orgNode.Value = aws.String(d.Get("principal_id").(string)) + + if v.(string) == servicecatalog.DescribePortfolioShareTypeOrganizationMemberAccount { + // portfolio_share type ORGANIZATION_MEMBER_ACCOUNT = org node type ACCOUNT + orgNode.Type = aws.String(servicecatalog.OrganizationNodeTypeAccount) + } else { + orgNode.Type = aws.String(d.Get("type").(string)) + } - if v, ok := d.GetOk("organization_node"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { - input.OrganizationNode = expandServiceCatalogOrganizationNode(v.([]interface{})[0].(map[string]interface{})) + input.OrganizationNode = orgNode } if v, ok := d.GetOk("share_tag_options"); ok { @@ -140,11 +126,7 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta return fmt.Errorf("error creating Service Catalog Portfolio Share: empty response") } - d.SetId(serviceCatalogPortfolioShareID( - d.Get("portfolio_id").(string), - d.Get("account_id").(string), - input.OrganizationNode, - )) + d.SetId(strings.Join([]string{d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string)}, ":")) // only get a token if organization node, otherwise check without token if output.PortfolioShareToken != nil { @@ -152,13 +134,7 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be ready: %w", d.Id(), err) } } else { - orgNodeValue := "" - - if input.OrganizationNode != nil && input.OrganizationNode.Value != nil { - orgNodeValue = aws.StringValue(input.OrganizationNode.Value) - } - - if _, err := waiter.PortfolioShareReady(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("account_id").(string), orgNodeValue); err != nil { + if _, err := waiter.PortfolioShareReady(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string)); err != nil { return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be ready: %w", d.Id(), err) } } @@ -169,17 +145,7 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta func resourceAwsServiceCatalogPortfolioShareRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).scconn - orgNodeValue := "" - - if v, ok := d.GetOk("organization_node"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { - orgNode := expandServiceCatalogOrganizationNode(v.([]interface{})[0].(map[string]interface{})) - - if orgNode.Value != nil { - orgNodeValue = aws.StringValue(orgNode.Value) - } - } - - output, err := waiter.PortfolioShareReady(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("account_id").(string), orgNodeValue) + output, err := waiter.PortfolioShareReady(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string)) if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { log.Printf("[WARN] Service Catalog Portfolio Share (%s) not found, removing from state", d.Id()) @@ -198,23 +164,7 @@ func resourceAwsServiceCatalogPortfolioShareRead(d *schema.ResourceData, meta in d.Set("accepted", output.Accepted) d.Set("share_tag_options", output.ShareTagOptions) d.Set("type", output.Type) - - switch aws.StringValue(output.Type) { - case servicecatalog.DescribePortfolioShareTypeAccount: - d.Set("account_id", output.PrincipalId) - d.Set("organization_node", nil) - default: - d.Set("account_id", nil) - - orgNode := &servicecatalog.OrganizationNode{ - Type: output.Type, - Value: output.PrincipalId, - } - - if err := d.Set("organization_node", flattenServiceCatalogOrganizationNode(orgNode)); err != nil { - return fmt.Errorf("error setting organization_node: %w", err) - } - } + d.Set("principal_id", output.PrincipalId) return nil } @@ -222,7 +172,7 @@ func resourceAwsServiceCatalogPortfolioShareRead(d *schema.ResourceData, meta in func resourceAwsServiceCatalogPortfolioShareUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).scconn - if d.HasChanges("accept_language", "account_id", "organization_node", "share_tag_options") { + if d.HasChanges("accept_language", "share_tag_options") { input := &servicecatalog.UpdatePortfolioShareInput{ PortfolioId: aws.String(d.Get("portfolio_id").(string)), } @@ -231,14 +181,6 @@ func resourceAwsServiceCatalogPortfolioShareUpdate(d *schema.ResourceData, meta input.AcceptLanguage = aws.String(v.(string)) } - if v, ok := d.GetOk("account_id"); ok { - input.AccountId = aws.String(v.(string)) - } - - if v, ok := d.GetOk("organization_node"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { - input.OrganizationNode = expandServiceCatalogOrganizationNode(v.([]interface{})[0].(map[string]interface{})) - } - if v, ok := d.GetOk("share_tag_options"); ok { input.ShareTagOptions = aws.Bool(v.(bool)) } @@ -280,12 +222,20 @@ func resourceAwsServiceCatalogPortfolioShareDelete(d *schema.ResourceData, meta input.AcceptLanguage = aws.String(v.(string)) } - if v, ok := d.GetOk("account_id"); ok { - input.AccountId = aws.String(v.(string)) - } + if v, ok := d.GetOk("type"); ok && v.(string) == servicecatalog.DescribePortfolioShareTypeAccount { + input.AccountId = aws.String(d.Get("principal_id").(string)) + } else { + orgNode := &servicecatalog.OrganizationNode{} + orgNode.Value = aws.String(d.Get("principal_id").(string)) + + if v.(string) == servicecatalog.DescribePortfolioShareTypeOrganizationMemberAccount { + // portfolio_share type ORGANIZATION_MEMBER_ACCOUNT = org node type ACCOUNT + orgNode.Type = aws.String(servicecatalog.OrganizationNodeTypeAccount) + } else { + orgNode.Type = aws.String(d.Get("type").(string)) + } - if v, ok := d.GetOk("organization_node"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { - input.OrganizationNode = expandServiceCatalogOrganizationNode(v.([]interface{})[0].(map[string]interface{})) + input.OrganizationNode = orgNode } output, err := conn.DeletePortfolioShare(input) @@ -304,76 +254,10 @@ func resourceAwsServiceCatalogPortfolioShareDelete(d *schema.ResourceData, meta return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be deleted: %w", d.Id(), err) } } else { - orgNodeValue := "" - - if input.OrganizationNode != nil && input.OrganizationNode.Value != nil { - orgNodeValue = aws.StringValue(input.OrganizationNode.Value) - } - - if _, err := waiter.PortfolioShareDeleted(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("account_id").(string), orgNodeValue); err != nil { + if _, err := waiter.PortfolioShareDeleted(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string)); err != nil { return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be deleted: %w", d.Id(), err) } } return nil } - -func expandServiceCatalogOrganizationNode(tfMap map[string]interface{}) *servicecatalog.OrganizationNode { - if tfMap == nil { - return nil - } - - apiObject := &servicecatalog.OrganizationNode{} - - if v, ok := tfMap["type"].(string); ok && v != "" { - apiObject.Type = aws.String(v) - } - - if v, ok := tfMap["value"].(string); ok && v != "" { - apiObject.Value = aws.String(v) - } - - return apiObject -} - -func flattenServiceCatalogOrganizationNode(apiObject *servicecatalog.OrganizationNode) map[string]interface{} { - if apiObject == nil { - return nil - } - - tfMap := map[string]interface{}{} - - if v := apiObject.Type; v != nil { - tfMap["type"] = aws.StringValue(v) - } - - if v := apiObject.Value; v != nil { - tfMap["value"] = aws.StringValue(v) - } - - return tfMap -} - -func serviceCatalogPortfolioShareID(portfolioID, accountID string, orgNode *servicecatalog.OrganizationNode) string { - var pieces []string - - if portfolioID != "" { - pieces = append(pieces, portfolioID) - } - - if accountID != "" { - pieces = append(pieces, accountID) - } - - if orgNode != nil { - if orgNode.Type != nil { - pieces = append(pieces, aws.StringValue(orgNode.Type)) - } - - if orgNode.Value != nil { - pieces = append(pieces, aws.StringValue(orgNode.Value)) - } - } - - return strings.Join(pieces, ":") -} From cdd59c765b1d05fb01779cf2b98c529af9f1bc2b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 15:00:30 -0400 Subject: [PATCH 16/41] tests/r/servicecatalog_portfolio_share: Rework tests for new schema --- ...aws_servicecatalog_portfolio_share_test.go | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/aws/resource_aws_servicecatalog_portfolio_share_test.go b/aws/resource_aws_servicecatalog_portfolio_share_test.go index d631954d3c3..9aac1b5b288 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share_test.go +++ b/aws/resource_aws_servicecatalog_portfolio_share_test.go @@ -14,6 +14,7 @@ import ( func TestAccAWSServiceCatalogPortfolioShare_basic(t *testing.T) { resourceName := "aws_servicecatalog_portfolio_share.test" + compareName := "aws_servicecatalog_portfolio.test" rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ @@ -28,11 +29,8 @@ func TestAccAWSServiceCatalogPortfolioShare_basic(t *testing.T) { testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName), resource.TestCheckResourceAttr(resourceName, "accept_language", "en"), resource.TestCheckResourceAttr(resourceName, "accepted", "true"), - resource.TestCheckResourceAttr(resourceName, "account_id", "distributör"), - resource.TestCheckResourceAttr(resourceName, "organization_node.#", "1"), - resource.TestCheckResourceAttr(resourceName, "organization_node.0.type", servicecatalog.DescribePortfolioShareTypeOrganization), - resource.TestCheckResourceAttr(resourceName, "organization_node.0.value", "true"), - resource.TestCheckResourceAttr(resourceName, "portfolio_id", "8675309"), + resource.TestCheckResourceAttr(resourceName, "principal_id", "arn:aws:organizations::111122223333:organization/o-abcdefghijkl"), + resource.TestCheckResourceAttrPair(resourceName, "portfolio_id", compareName, "id"), resource.TestCheckResourceAttr(resourceName, "share_tag_options", "true"), resource.TestCheckResourceAttr(resourceName, "type", servicecatalog.DescribePortfolioShareTypeOrganization), ), @@ -62,8 +60,7 @@ func testAccCheckAwsServiceCatalogPortfolioShareDestroy(s *terraform.State) erro conn, rs.Primary.Attributes["portfolio_id"], rs.Primary.Attributes["share_type"], - rs.Primary.Attributes["account_id"], - rs.Primary.Attributes["organization_node.0.value"], + rs.Primary.Attributes["principal_id"], ) if tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { @@ -96,8 +93,7 @@ func testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName string) reso conn, rs.Primary.Attributes["portfolio_id"], rs.Primary.Attributes["share_type"], - rs.Primary.Attributes["account_id"], - rs.Primary.Attributes["organization_node.0.value"], + rs.Primary.Attributes["principal_id"], ) if tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { @@ -129,11 +125,7 @@ resource "aws_servicecatalog_portfolio_share" "test" { portfolio_id = aws_servicecatalog_portfolio.test.id share_tag_options = true type = "ORGANIZATION" - - organization_node { - type = "ORGANIZATION" - value = "arn:aws:organizations::111122223333:organization/o-abcdefghijkl" - } + principal_id = "arn:aws:organizations::111122223333:organization/o-abcdefghijkl" } `, rName) } From 9aa9c29767a11d1b44c741321eaf6689e7c5564b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 15:01:16 -0400 Subject: [PATCH 17/41] internal/servicecatalog: Rework finder for new schema --- aws/internal/service/servicecatalog/finder/finder.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/aws/internal/service/servicecatalog/finder/finder.go b/aws/internal/service/servicecatalog/finder/finder.go index cf66526f7e1..d78c3e7495f 100644 --- a/aws/internal/service/servicecatalog/finder/finder.go +++ b/aws/internal/service/servicecatalog/finder/finder.go @@ -5,7 +5,7 @@ import ( "github.com/aws/aws-sdk-go/service/servicecatalog" ) -func PortfolioShare(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, accountID, orgNodeValue string) (*servicecatalog.PortfolioShareDetail, error) { +func PortfolioShare(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, principalID string) (*servicecatalog.PortfolioShareDetail, error) { input := &servicecatalog.DescribePortfolioSharesInput{ PortfolioId: aws.String(portfolioID), Type: aws.String(shareType), @@ -22,12 +22,7 @@ func PortfolioShare(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, continue } - if aws.StringValue(deet.PrincipalId) == accountID && accountID != "" { - result = deet - return false - } - - if aws.StringValue(deet.PrincipalId) == orgNodeValue && orgNodeValue != "" { + if aws.StringValue(deet.PrincipalId) == principalID { result = deet return false } From 478202d6452483940fe27fcef50492ad6ed8d01b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 15:01:50 -0400 Subject: [PATCH 18/41] internal/servicecatalog: Rework status for new schema --- aws/internal/service/servicecatalog/waiter/status.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/internal/service/servicecatalog/waiter/status.go b/aws/internal/service/servicecatalog/waiter/status.go index b879effe862..259f9a4563b 100644 --- a/aws/internal/service/servicecatalog/waiter/status.go +++ b/aws/internal/service/servicecatalog/waiter/status.go @@ -93,9 +93,9 @@ func PortfolioShareStatusWithToken(conn *servicecatalog.ServiceCatalog, token st } } -func PortfolioShareStatus(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, accountID, orgNodeValue string) resource.StateRefreshFunc { +func PortfolioShareStatus(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, principalID string) resource.StateRefreshFunc { return func() (interface{}, string, error) { - output, err := finder.PortfolioShare(conn, portfolioID, shareType, accountID, orgNodeValue) + output, err := finder.PortfolioShare(conn, portfolioID, shareType, principalID) if tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { return nil, StatusNotFound, err From 96d4bd424e57e5172104c561b0c76a8fbd0b6fe0 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 15:01:58 -0400 Subject: [PATCH 19/41] internal/servicecatalog: Rework waiter for new schema --- aws/internal/service/servicecatalog/waiter/waiter.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aws/internal/service/servicecatalog/waiter/waiter.go b/aws/internal/service/servicecatalog/waiter/waiter.go index 2e74a79e515..a80c4f998dd 100644 --- a/aws/internal/service/servicecatalog/waiter/waiter.go +++ b/aws/internal/service/servicecatalog/waiter/waiter.go @@ -96,11 +96,11 @@ func TagOptionDeleted(conn *servicecatalog.ServiceCatalog, id string) error { return err } -func PortfolioShareReady(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, accountID, orgNodeValue string) (*servicecatalog.PortfolioShareDetail, error) { +func PortfolioShareReady(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, principalID string) (*servicecatalog.PortfolioShareDetail, error) { stateConf := &resource.StateChangeConf{ Pending: []string{servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, StatusNotFound, StatusUnavailable}, Target: []string{servicecatalog.ShareStatusCompleted}, - Refresh: PortfolioShareStatus(conn, portfolioID, shareType, accountID, orgNodeValue), + Refresh: PortfolioShareStatus(conn, portfolioID, shareType, principalID), Timeout: PortfolioShareCreateTimeout, } @@ -130,11 +130,11 @@ func PortfolioShareCreatedWithToken(conn *servicecatalog.ServiceCatalog, token s return nil, err } -func PortfolioShareDeleted(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, accountID, orgNodeValue string) (*servicecatalog.PortfolioShareDetail, error) { +func PortfolioShareDeleted(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, principalID string) (*servicecatalog.PortfolioShareDetail, error) { stateConf := &resource.StateChangeConf{ Pending: []string{servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, servicecatalog.ShareStatusCompleted, StatusUnavailable}, Target: []string{StatusNotFound}, - Refresh: PortfolioShareStatus(conn, portfolioID, shareType, accountID, orgNodeValue), + Refresh: PortfolioShareStatus(conn, portfolioID, shareType, principalID), Timeout: PortfolioShareCreateTimeout, } From 14d340fbbb2e625a2b1dbf61c552ae77d355cb53 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 15:10:06 -0400 Subject: [PATCH 20/41] docs/r/servicecatalog_portfolio_share: Update for schema rework --- ...servicecatalog_portfolio_share.html.markdown | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/website/docs/r/servicecatalog_portfolio_share.html.markdown b/website/docs/r/servicecatalog_portfolio_share.html.markdown index 0887a4328e4..8e75409a206 100644 --- a/website/docs/r/servicecatalog_portfolio_share.html.markdown +++ b/website/docs/r/servicecatalog_portfolio_share.html.markdown @@ -24,8 +24,9 @@ If the portfolio share with the specified account or organization node already e ```terraform resource "aws_servicecatalog_portfolio_share" "example" { - account_id = "012128675309" + principal_id = "012128675309" portfolio_id = aws_servicecatalog_portfolio.example.id + type = "ACCOUNT" } ``` @@ -33,9 +34,8 @@ resource "aws_servicecatalog_portfolio_share" "example" { The following arguments are required: -* `account_id` - (Required if `organization_node` is not included) AWS account ID. For example, `123456789012`. -* `organization_node` - (Required if `account_id` is not included) Configuration block with an organization node to whom you are going to share. Detailed below. * `portfolio_id` - (Required) Portfolio identifier. +* `principal_id` - (Required) Identifier of the principal with whom you will share the portfolio. Valid values AWS account IDs and ARNs of AWS Organizations and organizational units. * `type` - (Required) Type of portfolio share. Valid values are `ACCOUNT` (an external account), `ORGANIZATION` (a share to every account in an organization), `ORGANIZATIONAL_UNIT`, `ORGANIZATION_MEMBER_ACCOUNT` (a share to an account in an organization). The following arguments are optional: @@ -43,23 +43,16 @@ The following arguments are optional: * `accept_language` - (Optional) Language code. Valid values: `en` (English), `jp` (Japanese), `zh` (Chinese). Default value is `en`. * `share_tag_options` - (Optional) Whether to enable sharing of `aws_servicecatalog_tag_option` resources when creating the portfolio share. -### organization_node - -The following arguments are supported: - -* `type` - (Optional) Organization node type. Valid values are `ACCOUNT` (this value corresponds to `ORGANIZATION_MEMBER_ACCOUNT`), `ORGANIZATION`, and `ORGANIZATIONAL_UNIT`. -* `value` - (Optional) Identifier of the organization node. - ## Attributes Reference In addition to all arguments above, the following attributes are exported: -* `accepted` - Whether the shared portfolio is imported by the recipient account. If the recipient is in an organization node, the share is automatically imported, and the field is always set to true. +* `accepted` - Whether the shared portfolio is imported by the recipient account. If the recipient is organizational, the share is automatically imported, and the field is always set to true. ## Import `aws_servicecatalog_portfolio_share` can be imported using the portfolio share ID, e.g. ``` -$ terraform import aws_servicecatalog_portfolio_share.example arn:aws:catalog:us-east-1:123456789012:portfolio share/prod-dnigbtea24ste +$ terraform import aws_servicecatalog_portfolio_share.example port-12344321:ACCOUNT:123456789012 ``` From 92253f3a6979be1c42a96714f8209d0fba83a6dd Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:01:51 -0400 Subject: [PATCH 21/41] docs/r/servicecatalog_portfolio_share: Add wait arg --- website/docs/r/servicecatalog_portfolio_share.html.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/website/docs/r/servicecatalog_portfolio_share.html.markdown b/website/docs/r/servicecatalog_portfolio_share.html.markdown index 8e75409a206..fd9547c7aca 100644 --- a/website/docs/r/servicecatalog_portfolio_share.html.markdown +++ b/website/docs/r/servicecatalog_portfolio_share.html.markdown @@ -42,6 +42,7 @@ The following arguments are optional: * `accept_language` - (Optional) Language code. Valid values: `en` (English), `jp` (Japanese), `zh` (Chinese). Default value is `en`. * `share_tag_options` - (Optional) Whether to enable sharing of `aws_servicecatalog_tag_option` resources when creating the portfolio share. +* `wait_for_acceptance` - (Optional) Whether to wait (up to the timeout) for the share to be accepted. Organizational shares are automatically accepted. ## Attributes Reference From 5d259d798be7e1bf82acebcbf712860e459614e4 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:02:56 -0400 Subject: [PATCH 22/41] internal/servicecatalog: Adjust not found errors --- aws/internal/service/servicecatalog/waiter/status.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aws/internal/service/servicecatalog/waiter/status.go b/aws/internal/service/servicecatalog/waiter/status.go index 259f9a4563b..6807e057f57 100644 --- a/aws/internal/service/servicecatalog/waiter/status.go +++ b/aws/internal/service/servicecatalog/waiter/status.go @@ -106,7 +106,9 @@ func PortfolioShareStatus(conn *servicecatalog.ServiceCatalog, portfolioID, shar } if output == nil { - return nil, StatusUnavailable, fmt.Errorf("error finding portfolio share: empty response") + return nil, StatusNotFound, &resource.NotFoundError{ + Message: fmt.Sprintf("error finding portfolio share (%s:%s:%s): empty response", portfolioID, shareType, principalID), + } } if !aws.BoolValue(output.Accepted) { From a4ea865eae9042f909bb574b58802f469aca706f Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:03:09 -0400 Subject: [PATCH 23/41] internal/servicecatalog: Adjust not found errors --- .../service/servicecatalog/waiter/waiter.go | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/aws/internal/service/servicecatalog/waiter/waiter.go b/aws/internal/service/servicecatalog/waiter/waiter.go index a80c4f998dd..f49fa3d9870 100644 --- a/aws/internal/service/servicecatalog/waiter/waiter.go +++ b/aws/internal/service/servicecatalog/waiter/waiter.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/service/servicecatalog" "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) const ( @@ -96,10 +97,16 @@ func TagOptionDeleted(conn *servicecatalog.ServiceCatalog, id string) error { return err } -func PortfolioShareReady(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, principalID string) (*servicecatalog.PortfolioShareDetail, error) { +func PortfolioShareReady(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, principalID string, acceptRequired bool) (*servicecatalog.PortfolioShareDetail, error) { + targets := []string{servicecatalog.ShareStatusCompleted} + + if !acceptRequired { + targets = append(targets, servicecatalog.ShareStatusInProgress) + } + stateConf := &resource.StateChangeConf{ Pending: []string{servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, StatusNotFound, StatusUnavailable}, - Target: []string{servicecatalog.ShareStatusCompleted}, + Target: targets, Refresh: PortfolioShareStatus(conn, portfolioID, shareType, principalID), Timeout: PortfolioShareCreateTimeout, } @@ -113,10 +120,16 @@ func PortfolioShareReady(conn *servicecatalog.ServiceCatalog, portfolioID, share return nil, err } -func PortfolioShareCreatedWithToken(conn *servicecatalog.ServiceCatalog, token string) (*servicecatalog.DescribePortfolioShareStatusOutput, error) { +func PortfolioShareCreatedWithToken(conn *servicecatalog.ServiceCatalog, token string, acceptRequired bool) (*servicecatalog.DescribePortfolioShareStatusOutput, error) { + targets := []string{servicecatalog.ShareStatusCompleted} + + if !acceptRequired { + targets = append(targets, servicecatalog.ShareStatusInProgress) + } + stateConf := &resource.StateChangeConf{ Pending: []string{servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, StatusNotFound, StatusUnavailable}, - Target: []string{servicecatalog.ShareStatusCompleted}, + Target: targets, Refresh: PortfolioShareStatusWithToken(conn, token), Timeout: PortfolioShareCreateTimeout, } @@ -140,6 +153,10 @@ func PortfolioShareDeleted(conn *servicecatalog.ServiceCatalog, portfolioID, sha outputRaw, err := stateConf.WaitForState() + if tfresource.NotFound(err) { + return nil, nil + } + if output, ok := outputRaw.(*servicecatalog.PortfolioShareDetail); ok { return output, err } From ea8274780d8b05b1b7b6032c56ca0aee7f733a8a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:04:03 -0400 Subject: [PATCH 24/41] tests/r/servicecatalog_portfolio_share: Add new account test --- ...aws_servicecatalog_portfolio_share_test.go | 68 +++++++++++++++++-- 1 file changed, 64 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_servicecatalog_portfolio_share_test.go b/aws/resource_aws_servicecatalog_portfolio_share_test.go index 9aac1b5b288..5285e855883 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share_test.go +++ b/aws/resource_aws_servicecatalog_portfolio_share_test.go @@ -8,11 +8,49 @@ import ( "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "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/terraform" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/servicecatalog/finder" ) func TestAccAWSServiceCatalogPortfolioShare_basic(t *testing.T) { + var providers []*schema.Provider + resourceName := "aws_servicecatalog_portfolio_share.test" + compareName := "aws_servicecatalog_portfolio.test" + dataSourceName := "data.aws_caller_identity.alternate" + rName := acctest.RandomWithPrefix("tf-acc-test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccAlternateAccountPreCheck(t) }, + ErrorCheck: testAccErrorCheck(t, servicecatalog.EndpointsID), + ProviderFactories: testAccProviderFactoriesAlternate(&providers), + CheckDestroy: testAccCheckAwsServiceCatalogPortfolioShareDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSServiceCatalogPortfolioShareConfig_basic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "accept_language", "en"), + resource.TestCheckResourceAttr(resourceName, "accepted", "false"), + resource.TestCheckResourceAttrPair(resourceName, "principal_id", dataSourceName, "account_id"), + resource.TestCheckResourceAttrPair(resourceName, "portfolio_id", compareName, "id"), + resource.TestCheckResourceAttr(resourceName, "share_tag_options", "true"), + resource.TestCheckResourceAttr(resourceName, "type", servicecatalog.DescribePortfolioShareTypeAccount), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "accept_language", + }, + }, + }, + }) +} + +func TestAccAWSServiceCatalogPortfolioShare_organization(t *testing.T) { resourceName := "aws_servicecatalog_portfolio_share.test" compareName := "aws_servicecatalog_portfolio.test" rName := acctest.RandomWithPrefix("tf-acc-test") @@ -24,7 +62,7 @@ func TestAccAWSServiceCatalogPortfolioShare_basic(t *testing.T) { CheckDestroy: testAccCheckAwsServiceCatalogPortfolioShareDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSServiceCatalogPortfolioShareConfig_basic(rName), + Config: testAccAWSServiceCatalogPortfolioShareConfig_organization(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName), resource.TestCheckResourceAttr(resourceName, "accept_language", "en"), @@ -41,7 +79,6 @@ func TestAccAWSServiceCatalogPortfolioShare_basic(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{ "accept_language", - "provisioning_artifact_parameters", }, }, }, @@ -59,7 +96,7 @@ func testAccCheckAwsServiceCatalogPortfolioShareDestroy(s *terraform.State) erro output, err := finder.PortfolioShare( conn, rs.Primary.Attributes["portfolio_id"], - rs.Primary.Attributes["share_type"], + rs.Primary.Attributes["type"], rs.Primary.Attributes["principal_id"], ) @@ -92,7 +129,7 @@ func testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName string) reso _, err := finder.PortfolioShare( conn, rs.Primary.Attributes["portfolio_id"], - rs.Primary.Attributes["share_type"], + rs.Primary.Attributes["type"], rs.Primary.Attributes["principal_id"], ) @@ -109,6 +146,29 @@ func testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName string) reso } func testAccAWSServiceCatalogPortfolioShareConfig_basic(rName string) string { + return composeConfig(testAccAlternateAccountProviderConfig(), fmt.Sprintf(` +data "aws_caller_identity" "alternate" { + provider = "awsalternate" +} + +resource "aws_servicecatalog_portfolio" "test" { + name = %[1]q + description = %[1]q + provider_name = %[1]q +} + +resource "aws_servicecatalog_portfolio_share" "test" { + accept_language = "en" + portfolio_id = aws_servicecatalog_portfolio.test.id + share_tag_options = true + type = "ACCOUNT" + principal_id = data.aws_caller_identity.alternate.account_id + wait_for_acceptance = false +} +`, rName)) +} + +func testAccAWSServiceCatalogPortfolioShareConfig_organization(rName string) string { return fmt.Sprintf(` resource "aws_servicecatalog_organizations_access" "test" { enabled = "true" From 75e28a7ddd86660afc41b42fb13c15f660b5240f Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:04:55 -0400 Subject: [PATCH 25/41] r/servicecatalog_portfolio_share: Adjust for wait option --- ...urce_aws_servicecatalog_portfolio_share.go | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_servicecatalog_portfolio_share.go b/aws/resource_aws_servicecatalog_portfolio_share.go index cf504081593..5e123b416e4 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share.go +++ b/aws/resource_aws_servicecatalog_portfolio_share.go @@ -62,6 +62,11 @@ func resourceAwsServiceCatalogPortfolioShare() *schema.Resource { ForceNew: true, ValidateFunc: validation.StringInSlice(servicecatalog.DescribePortfolioShareType_Values(), false), }, + "wait_for_acceptance": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, }, } } @@ -128,13 +133,18 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta d.SetId(strings.Join([]string{d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string)}, ":")) + waitForAcceptance := false + if v, ok := d.GetOk("wait_for_acceptance"); ok { + waitForAcceptance = v.(bool) + } + // only get a token if organization node, otherwise check without token if output.PortfolioShareToken != nil { - if _, err := waiter.PortfolioShareCreatedWithToken(conn, aws.StringValue(output.PortfolioShareToken)); err != nil { + if _, err := waiter.PortfolioShareCreatedWithToken(conn, aws.StringValue(output.PortfolioShareToken), waitForAcceptance); err != nil { return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be ready: %w", d.Id(), err) } } else { - if _, err := waiter.PortfolioShareReady(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string)); err != nil { + if _, err := waiter.PortfolioShareReady(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string), waitForAcceptance); err != nil { return fmt.Errorf("error waiting for Service Catalog Portfolio Share (%s) to be ready: %w", d.Id(), err) } } @@ -145,7 +155,18 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta func resourceAwsServiceCatalogPortfolioShareRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).scconn - output, err := waiter.PortfolioShareReady(conn, d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string)) + portfolioID, shareType, principalID, err := resourceServiceCatalogPortfolioShareParseId(d.Id()) + + if err != nil { + return fmt.Errorf("could not parse ID (%s): %w", d.Id(), err) + } + + waitForAcceptance := false + if v, ok := d.GetOk("wait_for_acceptance"); ok { + waitForAcceptance = v.(bool) + } + + output, err := waiter.PortfolioShareReady(conn, portfolioID, shareType, principalID, waitForAcceptance) if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, servicecatalog.ErrCodeResourceNotFoundException) { log.Printf("[WARN] Service Catalog Portfolio Share (%s) not found, removing from state", d.Id()) @@ -162,9 +183,11 @@ func resourceAwsServiceCatalogPortfolioShareRead(d *schema.ResourceData, meta in } d.Set("accepted", output.Accepted) + d.Set("portfolio_id", portfolioID) + d.Set("principal_id", output.PrincipalId) d.Set("share_tag_options", output.ShareTagOptions) d.Set("type", output.Type) - d.Set("principal_id", output.PrincipalId) + d.Set("wait_for_acceptance", waitForAcceptance) return nil } @@ -261,3 +284,13 @@ func resourceAwsServiceCatalogPortfolioShareDelete(d *schema.ResourceData, meta return nil } + +func resourceServiceCatalogPortfolioShareParseId(id string) (string, string, string, error) { + parts := strings.SplitN(id, ":", 3) + + if len(parts) != 3 || parts[0] == "" || parts[1] == "" || parts[2] == "" { + return "", "", "", fmt.Errorf("unexpected format of ID (%s), expected portfolioID:type:principalID", id) + } + + return parts[0], parts[1], parts[2], nil +} From 547f518285211be5dac0af12e0ec8f71dec326bc Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:22:31 -0400 Subject: [PATCH 26/41] envvar: Add skipping option func --- aws/internal/envvar/testing_funcs.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/aws/internal/envvar/testing_funcs.go b/aws/internal/envvar/testing_funcs.go index 841ec3d0115..f9938f8ca3b 100644 --- a/aws/internal/envvar/testing_funcs.go +++ b/aws/internal/envvar/testing_funcs.go @@ -50,3 +50,18 @@ func TestSkipIfEmpty(t testing.T, name string, usageMessage string) string { return value } + +// TestFailIfAllEmpty verifies that at least one environment variable is non-empty or fails the test. +// +// If at lease one environment variable is non-empty, returns the first name and value. +func TestSkipIfAllEmpty(t testing.T, names []string, usageMessage string) (string, string) { + t.Helper() + + name, value, err := RequireOneOf(names, usageMessage) + if err != nil { + t.Skipf("skipping test because %s.", err) + return "", "" + } + + return name, value +} From 34ccacb6b919b10f810fdce4c3bd413379964f74 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:23:05 -0400 Subject: [PATCH 27/41] provider: Skip when prequisite alt acct envvars not set --- aws/provider_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/provider_test.go b/aws/provider_test.go index 3acef649007..86aca94af99 100644 --- a/aws/provider_test.go +++ b/aws/provider_test.go @@ -698,10 +698,10 @@ func testAccGetThirdRegionPartition() string { } func testAccAlternateAccountPreCheck(t *testing.T) { - envvar.TestFailIfAllEmpty(t, []string{envvar.AwsAlternateProfile, envvar.AwsAlternateAccessKeyId}, "credentials for running acceptance testing in alternate AWS account") + envvar.TestSkipIfAllEmpty(t, []string{envvar.AwsAlternateProfile, envvar.AwsAlternateAccessKeyId}, "credentials for running acceptance testing in alternate AWS account") if os.Getenv(envvar.AwsAlternateAccessKeyId) != "" { - envvar.TestFailIfEmpty(t, envvar.AwsAlternateSecretAccessKey, "static credentials value when using "+envvar.AwsAlternateAccessKeyId) + envvar.TestSkipIfEmpty(t, envvar.AwsAlternateSecretAccessKey, "static credentials value when using "+envvar.AwsAlternateAccessKeyId) } } From c1a29fd8f13776cac829a913d5c239ab327634bb Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:23:51 -0400 Subject: [PATCH 28/41] tests/r/servicecatalog_portfolio_share: Add prechecks --- ...source_aws_servicecatalog_portfolio_share_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_servicecatalog_portfolio_share_test.go b/aws/resource_aws_servicecatalog_portfolio_share_test.go index 5285e855883..da6439c3c48 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share_test.go +++ b/aws/resource_aws_servicecatalog_portfolio_share_test.go @@ -21,7 +21,11 @@ func TestAccAWSServiceCatalogPortfolioShare_basic(t *testing.T) { rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t); testAccAlternateAccountPreCheck(t) }, + PreCheck: func() { + testAccPreCheck(t) + testAccAlternateAccountPreCheck(t) + testAccPartitionHasServicePreCheck(servicecatalog.EndpointsID, t) + }, ErrorCheck: testAccErrorCheck(t, servicecatalog.EndpointsID), ProviderFactories: testAccProviderFactoriesAlternate(&providers), CheckDestroy: testAccCheckAwsServiceCatalogPortfolioShareDestroy, @@ -56,7 +60,11 @@ func TestAccAWSServiceCatalogPortfolioShare_organization(t *testing.T) { rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, + PreCheck: func() { + testAccPreCheck(t) + testAccOrganizationsAccountPreCheck(t) + testAccPartitionHasServicePreCheck(servicecatalog.EndpointsID, t) + }, ErrorCheck: testAccErrorCheck(t, servicecatalog.EndpointsID), Providers: testAccProviders, CheckDestroy: testAccCheckAwsServiceCatalogPortfolioShareDestroy, From a1bfca5bb379d2053684979a24bd2e55e1f5edf7 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:27:00 -0400 Subject: [PATCH 29/41] tests/r/servicecatalog_portfolio_share: Hardcoded fix --- aws/resource_aws_servicecatalog_portfolio_share_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aws/resource_aws_servicecatalog_portfolio_share_test.go b/aws/resource_aws_servicecatalog_portfolio_share_test.go index da6439c3c48..7a17f69226f 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share_test.go +++ b/aws/resource_aws_servicecatalog_portfolio_share_test.go @@ -178,6 +178,8 @@ resource "aws_servicecatalog_portfolio_share" "test" { func testAccAWSServiceCatalogPortfolioShareConfig_organization(rName string) string { return fmt.Sprintf(` +data "aws_partition" "current" {} + resource "aws_servicecatalog_organizations_access" "test" { enabled = "true" } @@ -193,7 +195,7 @@ resource "aws_servicecatalog_portfolio_share" "test" { portfolio_id = aws_servicecatalog_portfolio.test.id share_tag_options = true type = "ORGANIZATION" - principal_id = "arn:aws:organizations::111122223333:organization/o-abcdefghijkl" + principal_id = "arn:${data.aws_partition.current.partition}:organizations::111122223333:organization/o-abcdefghijkl" } `, rName) } From 44861f3cc73f5f33af8f11bdc92e421032538c3f Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:28:14 -0400 Subject: [PATCH 30/41] envvar: Update skip func docs --- aws/internal/envvar/testing_funcs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/internal/envvar/testing_funcs.go b/aws/internal/envvar/testing_funcs.go index f9938f8ca3b..ce81e250a60 100644 --- a/aws/internal/envvar/testing_funcs.go +++ b/aws/internal/envvar/testing_funcs.go @@ -51,7 +51,7 @@ func TestSkipIfEmpty(t testing.T, name string, usageMessage string) string { return value } -// TestFailIfAllEmpty verifies that at least one environment variable is non-empty or fails the test. +// TestSkipIfAllEmpty verifies that at least one environment variable is non-empty or skips the test. // // If at lease one environment variable is non-empty, returns the first name and value. func TestSkipIfAllEmpty(t testing.T, names []string, usageMessage string) (string, string) { From 56e278b3e1fdb5e61c5db37c135e84e51454ab91 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:39:00 -0400 Subject: [PATCH 31/41] provider: Add servicecatalog resource --- aws/provider.go | 1 + 1 file changed, 1 insertion(+) diff --git a/aws/provider.go b/aws/provider.go index 5d807542caa..b3aa0cd1736 100644 --- a/aws/provider.go +++ b/aws/provider.go @@ -1005,6 +1005,7 @@ func Provider() *schema.Provider { "aws_securityhub_organization_admin_account": resourceAwsSecurityHubOrganizationAdminAccount(), "aws_securityhub_product_subscription": resourceAwsSecurityHubProductSubscription(), "aws_securityhub_standards_subscription": resourceAwsSecurityHubStandardsSubscription(), + "aws_servicecatalog_organizations_access": resourceAwsServiceCatalogOrganizationsAccess(), "aws_servicecatalog_portfolio": resourceAwsServiceCatalogPortfolio(), "aws_servicecatalog_portfolio_share": resourceAwsServiceCatalogPortfolioShare(), "aws_servicecatalog_product": resourceAwsServiceCatalogProduct(), From cd74e57e3d5f9440e656d826b4f767de3938c4ec Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 7 May 2021 17:50:06 -0400 Subject: [PATCH 32/41] tests/r/servicecatalog_portfolio_share: Remove hardcoded partition --- aws/resource_aws_servicecatalog_portfolio_share_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_servicecatalog_portfolio_share_test.go b/aws/resource_aws_servicecatalog_portfolio_share_test.go index 7a17f69226f..bd643a5b4af 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share_test.go +++ b/aws/resource_aws_servicecatalog_portfolio_share_test.go @@ -75,7 +75,7 @@ func TestAccAWSServiceCatalogPortfolioShare_organization(t *testing.T) { testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName), resource.TestCheckResourceAttr(resourceName, "accept_language", "en"), resource.TestCheckResourceAttr(resourceName, "accepted", "true"), - resource.TestCheckResourceAttr(resourceName, "principal_id", "arn:aws:organizations::111122223333:organization/o-abcdefghijkl"), + resource.TestCheckResourceAttr(resourceName, "principal_id", fmt.Sprintf("arn:%s:organizations::111122223333:organization/o-abcdefghijkl", testAccGetPartition())), resource.TestCheckResourceAttrPair(resourceName, "portfolio_id", compareName, "id"), resource.TestCheckResourceAttr(resourceName, "share_tag_options", "true"), resource.TestCheckResourceAttr(resourceName, "type", servicecatalog.DescribePortfolioShareTypeOrganization), From a6166f565749b99fcc0d4eab709e484602576cea Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Tue, 11 May 2021 13:41:59 -0400 Subject: [PATCH 33/41] r/servicecatalog_portfolio_share: Move id stuff to internal --- aws/internal/service/servicecatalog/id.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 aws/internal/service/servicecatalog/id.go diff --git a/aws/internal/service/servicecatalog/id.go b/aws/internal/service/servicecatalog/id.go new file mode 100644 index 00000000000..291156be679 --- /dev/null +++ b/aws/internal/service/servicecatalog/id.go @@ -0,0 +1,20 @@ +package servicecatalog + +import ( + "fmt" + "strings" +) + +func ParsePortfolioShareID(id string) (string, string, string, error) { + parts := strings.SplitN(id, ":", 3) + + if len(parts) != 3 || parts[0] == "" || parts[1] == "" || parts[2] == "" { + return "", "", "", fmt.Errorf("unexpected format of ID (%s), expected portfolioID:type:principalID", id) + } + + return parts[0], parts[1], parts[2], nil +} + +func PortfolioShareID(portfolioID, shareType, principalID string) string { + return strings.Join([]string{portfolioID, shareType, principalID}, ":") +} From 26c06edd42174275bfa3e433715119faaf449be5 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Tue, 11 May 2021 13:43:26 -0400 Subject: [PATCH 34/41] r/servicecat_org_access: Clean up messages, delete --- ...source_aws_servicecatalog_organizations_access.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/aws/resource_aws_servicecatalog_organizations_access.go b/aws/resource_aws_servicecatalog_organizations_access.go index d00d0f144aa..a8faf81c1f9 100644 --- a/aws/resource_aws_servicecatalog_organizations_access.go +++ b/aws/resource_aws_servicecatalog_organizations_access.go @@ -29,6 +29,8 @@ func resourceAwsServiceCatalogOrganizationsAccess() *schema.Resource { func resourceAwsServiceCatalogOrganizationsAccessCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).scconn + d.SetId(meta.(*AWSClient).accountid) + // During create, if enabled = "true", then Enable Access and vice versa // During delete, the opposite @@ -45,11 +47,9 @@ func resourceAwsServiceCatalogOrganizationsAccessCreate(d *schema.ResourceData, _, err := conn.DisableAWSOrganizationsAccess(&servicecatalog.DisableAWSOrganizationsAccessInput{}) if err != nil { - return fmt.Errorf("error enabling Service Catalog AWS Organizations Access: %w", err) + return fmt.Errorf("error disabling Service Catalog AWS Organizations Access: %w", err) } - d.SetId(meta.(*AWSClient).accountid) - return resourceAwsServiceCatalogOrganizationsAccessRead(d, meta) } @@ -92,10 +92,10 @@ func resourceAwsServiceCatalogOrganizationsAccessDelete(d *schema.ResourceData, _, err := conn.EnableAWSOrganizationsAccess(&servicecatalog.EnableAWSOrganizationsAccessInput{}) if err != nil { - return fmt.Errorf("error disabling Service Catalog AWS Organizations Access: %w", err) + return fmt.Errorf("error enabling Service Catalog AWS Organizations Access: %w", err) } - return resourceAwsServiceCatalogOrganizationsAccessRead(d, meta) + return nil } _, err := conn.DisableAWSOrganizationsAccess(&servicecatalog.DisableAWSOrganizationsAccessInput{}) @@ -104,5 +104,5 @@ func resourceAwsServiceCatalogOrganizationsAccessDelete(d *schema.ResourceData, return fmt.Errorf("error disabling Service Catalog AWS Organizations Access: %w", err) } - return resourceAwsServiceCatalogOrganizationsAccessRead(d, meta) + return nil } From 6d12cff4bf8579447e327d776335a34b26ba0d12 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Tue, 11 May 2021 13:44:10 -0400 Subject: [PATCH 35/41] r/servicecat_portfolio: Enforce field lengths --- aws/resource_aws_servicecatalog_portfolio.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_servicecatalog_portfolio.go b/aws/resource_aws_servicecatalog_portfolio.go index 58626b178ba..dc0dc20741a 100644 --- a/aws/resource_aws_servicecatalog_portfolio.go +++ b/aws/resource_aws_servicecatalog_portfolio.go @@ -41,8 +41,9 @@ func resourceAwsServiceCatalogPortfolio() *schema.Resource { Computed: true, }, "name": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringLenBetween(1, 100), }, "description": { Type: schema.TypeString, @@ -51,8 +52,9 @@ func resourceAwsServiceCatalogPortfolio() *schema.Resource { ValidateFunc: validation.StringLenBetween(0, 2000), }, "provider_name": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringLenBetween(1, 50), }, "tags": tagsSchema(), "tags_all": tagsSchemaComputed(), From ea01f5cc7659a3e533365e9842f507a11f0ee7b6 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Tue, 11 May 2021 13:45:14 -0400 Subject: [PATCH 36/41] r/servicecat_portfolio_share: Use internal ID helper --- ...resource_aws_servicecatalog_portfolio_share.go | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/aws/resource_aws_servicecatalog_portfolio_share.go b/aws/resource_aws_servicecatalog_portfolio_share.go index 5e123b416e4..20146987696 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share.go +++ b/aws/resource_aws_servicecatalog_portfolio_share.go @@ -3,7 +3,6 @@ package aws import ( "fmt" "log" - "strings" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/servicecatalog" @@ -131,7 +130,7 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta return fmt.Errorf("error creating Service Catalog Portfolio Share: empty response") } - d.SetId(strings.Join([]string{d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string)}, ":")) + d.SetId(tfservicecatalog.PortfolioShareID(d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string))) waitForAcceptance := false if v, ok := d.GetOk("wait_for_acceptance"); ok { @@ -155,7 +154,7 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta func resourceAwsServiceCatalogPortfolioShareRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).scconn - portfolioID, shareType, principalID, err := resourceServiceCatalogPortfolioShareParseId(d.Id()) + portfolioID, shareType, principalID, err := tfservicecatalog.ParsePortfolioShareID(d.Id()) if err != nil { return fmt.Errorf("could not parse ID (%s): %w", d.Id(), err) @@ -284,13 +283,3 @@ func resourceAwsServiceCatalogPortfolioShareDelete(d *schema.ResourceData, meta return nil } - -func resourceServiceCatalogPortfolioShareParseId(id string) (string, string, string, error) { - parts := strings.SplitN(id, ":", 3) - - if len(parts) != 3 || parts[0] == "" || parts[1] == "" || parts[2] == "" { - return "", "", "", fmt.Errorf("unexpected format of ID (%s), expected portfolioID:type:principalID", id) - } - - return parts[0], parts[1], parts[2], nil -} From 1eb17f242851bfe9d6754bdd28c42a6bd5104221 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 13 May 2021 16:35:21 -0400 Subject: [PATCH 37/41] Add 'testAccOrganizationManagementAccountPreCheck'. --- .../service/organizations/finder/finder.go | 25 +++++++++++++++++++ aws/internal/service/sts/finder/finder.go | 25 +++++++++++++++++++ aws/provider_test.go | 20 +++++++++++++++ ...ervicecatalog_organizations_access_test.go | 6 ++++- 4 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 aws/internal/service/organizations/finder/finder.go create mode 100644 aws/internal/service/sts/finder/finder.go diff --git a/aws/internal/service/organizations/finder/finder.go b/aws/internal/service/organizations/finder/finder.go new file mode 100644 index 00000000000..e9ee347b189 --- /dev/null +++ b/aws/internal/service/organizations/finder/finder.go @@ -0,0 +1,25 @@ +package finder + +import ( + "github.com/aws/aws-sdk-go/service/organizations" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +func Organization(conn *organizations.Organizations) (*organizations.Organization, error) { + input := &organizations.DescribeOrganizationInput{} + + output, err := conn.DescribeOrganization(input) + + if err != nil { + return nil, err + } + + if output == nil || output.Organization == nil { + return nil, &resource.NotFoundError{ + Message: "Empty result", + LastRequest: input, + } + } + + return output.Organization, nil +} diff --git a/aws/internal/service/sts/finder/finder.go b/aws/internal/service/sts/finder/finder.go new file mode 100644 index 00000000000..3ec4da3ef74 --- /dev/null +++ b/aws/internal/service/sts/finder/finder.go @@ -0,0 +1,25 @@ +package finder + +import ( + "github.com/aws/aws-sdk-go/service/sts" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +func CallerIdentity(conn *sts.STS) (*sts.GetCallerIdentityOutput, error) { + input := &sts.GetCallerIdentityInput{} + + output, err := conn.GetCallerIdentity(input) + + if err != nil { + return nil, err + } + + if output == nil { + return nil, &resource.NotFoundError{ + Message: "Empty result", + LastRequest: input, + } + } + + return output, nil +} diff --git a/aws/provider_test.go b/aws/provider_test.go index 86aca94af99..7f9495aa6f6 100644 --- a/aws/provider_test.go +++ b/aws/provider_test.go @@ -24,6 +24,8 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/terraform-providers/terraform-provider-aws/aws/internal/envvar" + organizationsfinder "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/organizations/finder" + stsfinder "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sts/finder" ) const ( @@ -792,6 +794,24 @@ func testAccOrganizationsEnabledPreCheck(t *testing.T) { } } +func testAccOrganizationManagementAccountPreCheck(t *testing.T) { + organization, err := organizationsfinder.Organization(testAccProvider.Meta().(*AWSClient).organizationsconn) + + if err != nil { + t.Fatalf("error describing AWS Organization: %s", err) + } + + callerIdentity, err := stsfinder.CallerIdentity(testAccProvider.Meta().(*AWSClient).stsconn) + + if err != nil { + t.Fatalf("error getting current identity: %s", err) + } + + if aws.StringValue(organization.MasterAccountId) != aws.StringValue(callerIdentity.Account) { + t.Skip("this AWS account must be the management account of an AWS Organization") + } +} + func testAccPreCheckIamServiceLinkedRole(t *testing.T, pathPrefix string) { conn := testAccProvider.Meta().(*AWSClient).iamconn diff --git a/aws/resource_aws_servicecatalog_organizations_access_test.go b/aws/resource_aws_servicecatalog_organizations_access_test.go index c93d8894b4c..e71e06b4474 100644 --- a/aws/resource_aws_servicecatalog_organizations_access_test.go +++ b/aws/resource_aws_servicecatalog_organizations_access_test.go @@ -14,7 +14,11 @@ func TestAccAWSServiceCatalogOrganizationsAccess_basic(t *testing.T) { resourceName := "aws_servicecatalog_organizations_access.test" resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, + PreCheck: func() { + testAccPreCheck(t) + testAccOrganizationsEnabledPreCheck(t) + testAccOrganizationManagementAccountPreCheck(t) + }, ErrorCheck: testAccErrorCheck(t, servicecatalog.EndpointsID), Providers: testAccProviders, CheckDestroy: testAccCheckAwsServiceCatalogOrganizationsAccessDestroy, From 40d9bbe614d3586b47070202ce0f4123e9904866 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 13 May 2021 16:35:46 -0400 Subject: [PATCH 38/41] Fix type casting error. --- aws/internal/service/servicecatalog/waiter/waiter.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aws/internal/service/servicecatalog/waiter/waiter.go b/aws/internal/service/servicecatalog/waiter/waiter.go index f49fa3d9870..24d634243f4 100644 --- a/aws/internal/service/servicecatalog/waiter/waiter.go +++ b/aws/internal/service/servicecatalog/waiter/waiter.go @@ -3,6 +3,7 @@ package waiter import ( "time" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/servicecatalog" "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -191,8 +192,8 @@ func OrganizationsAccessStable(conn *servicecatalog.ServiceCatalog) (string, err outputRaw, err := stateConf.WaitForState() - if output, ok := outputRaw.(string); ok { - return output, err + if output, ok := outputRaw.(*servicecatalog.GetAWSOrganizationsAccessStatusOutput); ok { + return aws.StringValue(output.AccessStatus), err } return "", err From 0b71dd4186457dfc45cfbf76a1fe6c46cd0495ad Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 14 May 2021 09:07:32 -0400 Subject: [PATCH 39/41] r/servicecat_portfolio_share: Revise org test --- ...aws_servicecatalog_portfolio_share_test.go | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/aws/resource_aws_servicecatalog_portfolio_share_test.go b/aws/resource_aws_servicecatalog_portfolio_share_test.go index bd643a5b4af..ab54a344377 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share_test.go +++ b/aws/resource_aws_servicecatalog_portfolio_share_test.go @@ -54,28 +54,24 @@ func TestAccAWSServiceCatalogPortfolioShare_basic(t *testing.T) { }) } -func TestAccAWSServiceCatalogPortfolioShare_organization(t *testing.T) { +func TestAccAWSServiceCatalogPortfolioShare_organizationalUnit(t *testing.T) { resourceName := "aws_servicecatalog_portfolio_share.test" compareName := "aws_servicecatalog_portfolio.test" rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { - testAccPreCheck(t) - testAccOrganizationsAccountPreCheck(t) - testAccPartitionHasServicePreCheck(servicecatalog.EndpointsID, t) - }, + PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, ErrorCheck: testAccErrorCheck(t, servicecatalog.EndpointsID), Providers: testAccProviders, CheckDestroy: testAccCheckAwsServiceCatalogPortfolioShareDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSServiceCatalogPortfolioShareConfig_organization(rName), + Config: testAccAWSServiceCatalogPortfolioShareConfig_organizationalUnit(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName), resource.TestCheckResourceAttr(resourceName, "accept_language", "en"), resource.TestCheckResourceAttr(resourceName, "accepted", "true"), - resource.TestCheckResourceAttr(resourceName, "principal_id", fmt.Sprintf("arn:%s:organizations::111122223333:organization/o-abcdefghijkl", testAccGetPartition())), + resource.TestCheckResourceAttrPair(resourceName, "principal_id", "aws_organizations_organizational_unit.test", "arn"), resource.TestCheckResourceAttrPair(resourceName, "portfolio_id", compareName, "id"), resource.TestCheckResourceAttr(resourceName, "share_tag_options", "true"), resource.TestCheckResourceAttr(resourceName, "type", servicecatalog.DescribePortfolioShareTypeOrganization), @@ -176,7 +172,7 @@ resource "aws_servicecatalog_portfolio_share" "test" { `, rName)) } -func testAccAWSServiceCatalogPortfolioShareConfig_organization(rName string) string { +func testAccAWSServiceCatalogPortfolioShareConfig_organizationalUnit(rName string) string { return fmt.Sprintf(` data "aws_partition" "current" {} @@ -190,12 +186,19 @@ resource "aws_servicecatalog_portfolio" "test" { provider_name = %[1]q } +resource "aws_organizations_organization" "test" {} + +resource "aws_organizations_organizational_unit" "test" { + name = %[1]q + parent_id = aws_organizations_organization.test.roots[0].id +} + resource "aws_servicecatalog_portfolio_share" "test" { accept_language = "en" portfolio_id = aws_servicecatalog_portfolio.test.id share_tag_options = true - type = "ORGANIZATION" - principal_id = "arn:${data.aws_partition.current.partition}:organizations::111122223333:organization/o-abcdefghijkl" + type = "ORGANIZATIONAL_UNIT" + principal_id = aws_organizations_organizational_unit.test.arn } `, rName) } From cb8d63393d776ca80c78fc0669ac3b55d941f8d2 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 14 May 2021 09:34:28 -0400 Subject: [PATCH 40/41] r/servicecat_portfolio_share: Finder contains string --- aws/internal/service/servicecatalog/finder/finder.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/aws/internal/service/servicecatalog/finder/finder.go b/aws/internal/service/servicecatalog/finder/finder.go index d78c3e7495f..c7347d6e558 100644 --- a/aws/internal/service/servicecatalog/finder/finder.go +++ b/aws/internal/service/servicecatalog/finder/finder.go @@ -1,6 +1,8 @@ package finder import ( + "strings" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/servicecatalog" ) @@ -22,7 +24,7 @@ func PortfolioShare(conn *servicecatalog.ServiceCatalog, portfolioID, shareType, continue } - if aws.StringValue(deet.PrincipalId) == principalID { + if strings.Contains(principalID, aws.StringValue(deet.PrincipalId)) { result = deet return false } From 5635137c8b27a2218d67c4d4f0f953a4d2ccb5a2 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 14 May 2021 14:21:13 -0400 Subject: [PATCH 41/41] r/aws_servicecatalog_portfolio_share: Get tests working for organization management account. Acceptance test output: % make testacc TEST=./aws TESTARGS='-run=TestAccAWSServiceCatalogOrganizationsAccess_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSServiceCatalogOrganizationsAccess_ -timeout 180m === RUN TestAccAWSServiceCatalogOrganizationsAccess_basic === PAUSE TestAccAWSServiceCatalogOrganizationsAccess_basic === CONT TestAccAWSServiceCatalogOrganizationsAccess_basic --- PASS: TestAccAWSServiceCatalogOrganizationsAccess_basic (16.23s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 19.053s % make testacc TEST=./aws TESTARGS='-run=TestAccAWSServiceCatalogPortfolioShare_' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSServiceCatalogPortfolioShare_ -timeout 180m === RUN TestAccAWSServiceCatalogPortfolioShare_basic === PAUSE TestAccAWSServiceCatalogPortfolioShare_basic === RUN TestAccAWSServiceCatalogPortfolioShare_organizationalUnit === PAUSE TestAccAWSServiceCatalogPortfolioShare_organizationalUnit === CONT TestAccAWSServiceCatalogPortfolioShare_basic === CONT TestAccAWSServiceCatalogPortfolioShare_organizationalUnit --- PASS: TestAccAWSServiceCatalogPortfolioShare_basic (19.43s) --- PASS: TestAccAWSServiceCatalogPortfolioShare_organizationalUnit (24.29s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 27.222s --- aws/internal/service/servicecatalog/id.go | 4 ++-- .../service/servicecatalog/waiter/waiter.go | 6 +++--- ...source_aws_servicecatalog_portfolio_share.go | 17 +++++++++++++++-- ...e_aws_servicecatalog_portfolio_share_test.go | 15 ++++++++++----- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/aws/internal/service/servicecatalog/id.go b/aws/internal/service/servicecatalog/id.go index 291156be679..43cfe94a16f 100644 --- a/aws/internal/service/servicecatalog/id.go +++ b/aws/internal/service/servicecatalog/id.go @@ -5,7 +5,7 @@ import ( "strings" ) -func ParsePortfolioShareID(id string) (string, string, string, error) { +func PortfolioShareParseResourceID(id string) (string, string, string, error) { parts := strings.SplitN(id, ":", 3) if len(parts) != 3 || parts[0] == "" || parts[1] == "" || parts[2] == "" { @@ -15,6 +15,6 @@ func ParsePortfolioShareID(id string) (string, string, string, error) { return parts[0], parts[1], parts[2], nil } -func PortfolioShareID(portfolioID, shareType, principalID string) string { +func PortfolioShareCreateResourceID(portfolioID, shareType, principalID string) string { return strings.Join([]string{portfolioID, shareType, principalID}, ":") } diff --git a/aws/internal/service/servicecatalog/waiter/waiter.go b/aws/internal/service/servicecatalog/waiter/waiter.go index 24d634243f4..590d3478140 100644 --- a/aws/internal/service/servicecatalog/waiter/waiter.go +++ b/aws/internal/service/servicecatalog/waiter/waiter.go @@ -52,7 +52,7 @@ func ProductDeleted(conn *servicecatalog.ServiceCatalog, acceptLanguage, product Pending: []string{servicecatalog.StatusCreating, servicecatalog.StatusAvailable, ProductStatusCreated, StatusUnavailable}, Target: []string{StatusNotFound}, Refresh: ProductStatus(conn, acceptLanguage, productID), - Timeout: ProductReadyTimeout, + Timeout: ProductDeleteTimeout, } _, err := stateConf.WaitForState() @@ -167,8 +167,8 @@ func PortfolioShareDeleted(conn *servicecatalog.ServiceCatalog, portfolioID, sha func PortfolioShareDeletedWithToken(conn *servicecatalog.ServiceCatalog, token string) (*servicecatalog.DescribePortfolioShareStatusOutput, error) { stateConf := &resource.StateChangeConf{ - Pending: []string{servicecatalog.ShareStatusCompleted, servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, StatusNotFound, StatusUnavailable}, - Target: []string{StatusNotFound}, + Pending: []string{servicecatalog.ShareStatusNotStarted, servicecatalog.ShareStatusInProgress, StatusNotFound, StatusUnavailable}, + Target: []string{servicecatalog.ShareStatusCompleted}, Refresh: PortfolioShareStatusWithToken(conn, token), Timeout: PortfolioShareCreateTimeout, } diff --git a/aws/resource_aws_servicecatalog_portfolio_share.go b/aws/resource_aws_servicecatalog_portfolio_share.go index 20146987696..2f12c649afc 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share.go +++ b/aws/resource_aws_servicecatalog_portfolio_share.go @@ -3,8 +3,10 @@ package aws import ( "fmt" "log" + "strings" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/servicecatalog" "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -49,6 +51,17 @@ func resourceAwsServiceCatalogPortfolioShare() *schema.Resource { Required: true, ForceNew: true, ValidateFunc: validateServiceCatalogSharePrincipal, + DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + newARN, err := arn.Parse(new) + + if err != nil { + return old == new + } + + parts := strings.Split(newARN.Resource, "/") + + return old == parts[len(parts)-1] + }, }, "share_tag_options": { Type: schema.TypeBool, @@ -130,7 +143,7 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta return fmt.Errorf("error creating Service Catalog Portfolio Share: empty response") } - d.SetId(tfservicecatalog.PortfolioShareID(d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string))) + d.SetId(tfservicecatalog.PortfolioShareCreateResourceID(d.Get("portfolio_id").(string), d.Get("type").(string), d.Get("principal_id").(string))) waitForAcceptance := false if v, ok := d.GetOk("wait_for_acceptance"); ok { @@ -154,7 +167,7 @@ func resourceAwsServiceCatalogPortfolioShareCreate(d *schema.ResourceData, meta func resourceAwsServiceCatalogPortfolioShareRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).scconn - portfolioID, shareType, principalID, err := tfservicecatalog.ParsePortfolioShareID(d.Id()) + portfolioID, shareType, principalID, err := tfservicecatalog.PortfolioShareParseResourceID(d.Id()) if err != nil { return fmt.Errorf("could not parse ID (%s): %w", d.Id(), err) diff --git a/aws/resource_aws_servicecatalog_portfolio_share_test.go b/aws/resource_aws_servicecatalog_portfolio_share_test.go index ab54a344377..62afd2a22d8 100644 --- a/aws/resource_aws_servicecatalog_portfolio_share_test.go +++ b/aws/resource_aws_servicecatalog_portfolio_share_test.go @@ -60,7 +60,12 @@ func TestAccAWSServiceCatalogPortfolioShare_organizationalUnit(t *testing.T) { rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) }, + PreCheck: func() { + testAccPreCheck(t) + testAccOrganizationsEnabledPreCheck(t) + testAccOrganizationManagementAccountPreCheck(t) + testAccPartitionHasServicePreCheck(servicecatalog.EndpointsID, t) + }, ErrorCheck: testAccErrorCheck(t, servicecatalog.EndpointsID), Providers: testAccProviders, CheckDestroy: testAccCheckAwsServiceCatalogPortfolioShareDestroy, @@ -71,10 +76,10 @@ func TestAccAWSServiceCatalogPortfolioShare_organizationalUnit(t *testing.T) { testAccCheckAwsServiceCatalogPortfolioShareExists(resourceName), resource.TestCheckResourceAttr(resourceName, "accept_language", "en"), resource.TestCheckResourceAttr(resourceName, "accepted", "true"), - resource.TestCheckResourceAttrPair(resourceName, "principal_id", "aws_organizations_organizational_unit.test", "arn"), + resource.TestCheckResourceAttrPair(resourceName, "principal_id", "aws_organizations_organizational_unit.test", "id"), resource.TestCheckResourceAttrPair(resourceName, "portfolio_id", compareName, "id"), resource.TestCheckResourceAttr(resourceName, "share_tag_options", "true"), - resource.TestCheckResourceAttr(resourceName, "type", servicecatalog.DescribePortfolioShareTypeOrganization), + resource.TestCheckResourceAttr(resourceName, "type", servicecatalog.DescribePortfolioShareTypeOrganizationalUnit), ), }, { @@ -186,11 +191,11 @@ resource "aws_servicecatalog_portfolio" "test" { provider_name = %[1]q } -resource "aws_organizations_organization" "test" {} +data "aws_organizations_organization" "test" {} resource "aws_organizations_organizational_unit" "test" { name = %[1]q - parent_id = aws_organizations_organization.test.roots[0].id + parent_id = data.aws_organizations_organization.test.roots[0].id } resource "aws_servicecatalog_portfolio_share" "test" {