Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(service-catalog): (CfnPortfolioPrincipalAssociation should support IAM_PATTERN) #24370

Closed
gbvanrenswoude opened this issue Feb 28, 2023 · 4 comments
Labels
@aws-cdk/aws-servicecatalog Related to AWS Service Catalog bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@gbvanrenswoude
Copy link

Describe the bug

Using

    new sc.CfnPortfolioPrincipalAssociation(
      this,
      'CfnPortfolioPrincipalAssociation',
      {
        portfolioId: portfolio.portfolioId,
        principalArn: `arn:aws:iam:::role/somerole`,
        principalType: 'IAM_PATTERN',
      },
    );

throws Property validation failure: [Value for property {/PrincipalType} does not match pattern {IAM}]

Expected Behavior

IAM is not the only supported pattern. Per https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/ServiceCatalog.html#associatePrincipalWithPortfolio-property (used in the AWS Custom Resources Framework of the CDK) usage of IAM_PATTERN is supported. Writing a Custom Resource using it yields the expected results:

    new cr.AwsCustomResource(this, "sharePortfolioWithPrincipalName", {
      installLatestAwsSdk: true,
      onCreate: {
        service: "ServiceCatalog",
        action: "associatePrincipalWithPortfolio",
        physicalResourceId: cr.PhysicalResourceId.of(
          "sharePortfolioWithPrincipalName"
        ),
        parameters: {
          PortfolioId: portfolio.portfolioId,
          PrincipalARN: `arn:aws:iam:::role/somerole`,
          PrincipalType: "IAM_PATTERN",
        },
      },
      onDelete: {
        service: "ServiceCatalog",
        action: "disassociatePrincipalFromPortfolio",
        physicalResourceId: cr.PhysicalResourceId.of(
          "sharePortfolioWithPrincipalName"
        ),
        parameters: {
          PortfolioId: portfolio.portfolioId,
          PrincipalARN: `arn:aws:iam:::role/somerole`,
          PrincipalType: "IAM_PATTERN",
        },
      },
      policy: cr.AwsCustomResourcePolicy.fromSdkCalls({
        resources: [portfolio.portfolioArn],
      }),
    });
mystack: creating CloudFormation changeset...

 ✅  mystack

✨  Deployment time: 1073.13s

Current Behavior

Throws Error: Property validation failure: [Value for property {/PrincipalType} does not match pattern {IAM}]

Reproduction Steps

See code attachments above

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.66.1

Framework Version

No response

Node.js Version

17

OS

MacOs

Language

Typescript

Language Version

TS 4

Other information

No response

@gbvanrenswoude gbvanrenswoude added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 28, 2023
@github-actions github-actions bot added the @aws-cdk/aws-servicecatalog Related to AWS Service Catalog label Feb 28, 2023
@gbvanrenswoude gbvanrenswoude changed the title (service-catalog): (short issue description) (service-catalog): (CfnPortfolioPrincipalAssociation should support IAM_PATTERN) Feb 28, 2023
@pahud
Copy link
Contributor

pahud commented Mar 1, 2023

From what I can see in the CFN doc, IAM_PATTERN should be accepted. I will try reproduce it in my account.

@pahud pahud added needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 1, 2023
@pahud pahud self-assigned this Mar 1, 2023
@pahud
Copy link
Contributor

pahud commented Mar 2, 2023

I can reproduce this issue with the code below:

export class ServicecatalogTsStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const portfolio = new sc.Portfolio(this, 'PF', {
      displayName: 'my-portfolio',
      providerName: 'my-provider',
    });

    new sc.CfnPortfolioPrincipalAssociation(
      this,
      'CfnPortfolioPrincipalAssociation',
      {
        portfolioId: portfolio.portfolioId,
        principalArn: `arn:aws:iam:::role/somerole`,
        principalType: 'IAM_PATTERN',
      },
    );
  }
}

And the error:

image

This seems to be a bug from CloudFormation. I'll escalate this internally.

@pahud pahud added needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. and removed needs-reproduction This issue needs reproduction. labels Mar 2, 2023
@pahud pahud assigned pahud and unassigned pahud Mar 2, 2023
@pahud pahud added the p1 label Mar 15, 2023
@pahud pahud removed their assignment Mar 15, 2023
@pahud
Copy link
Contributor

pahud commented Jun 23, 2023

Looking at the doc again. Seems the only accepted value is IAM so IAM_PATTERN would be an invalid value for cloudformation.

PrincipalType
The principal type. The supported value is IAM.

Allowed Values: IAM

Required: Yes

Type: String

@pahud pahud added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. p2 and removed p1 labels Jun 23, 2023
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-servicecatalog Related to AWS Service Catalog bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants