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

aws-cdk-lib/aws-route53: Use an existing role for performing DeleteExisting #22607

Closed
1 of 2 tasks
fjinkis opened this issue Oct 21, 2022 · 15 comments
Closed
1 of 2 tasks
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1

Comments

@fjinkis
Copy link

fjinkis commented Oct 21, 2022

Describe the feature

When you define a aws_route53.RecordSet you can enable the property deleteExisting to remove - or overwrite - a record if it already exists. The "problem" is each stack creates a new IAM Role in order to perform that operation. What I propose is being able to reuse an existing predefined role like we can do with other resources - as Lambda or Launch templates -

Lambda snippet

import { aws_lambda as lambda, aws_iam as iam } from "aws-cdk-lib";

// code here

new lambda.Function(this, "AlarmsHandlerFunction", {
          ...otherLambdaConfig,
          //       ⬇️
          role: iam.Role.fromRoleName(
            this,
            "FunctionRole",
            "SomePredefinedRole",
            { mutable: false }
          )
});

Launch template snippet

import { aws_ec2 as ec2, aws_iam as iam } from "aws-cdk-lib";

// code here

new ec2.LaunchTemplate(this, "AsgLaunchTemplate", {
        ...otherLaunchTemplateConfig,
        //       ⬇️
        role: iam.Role.fromRoleName(
          this,
          "InstanceProfileRole",
          "SomePredefinedRole",
          { mutable: false }
        )
});

Use Case

  • When your CDK user doesn't have access to IAM
  • When you want to keep roles tidy and with descriptive names
  • When you have many stacks and you want to reuse and/or group roles

Proposed Solution

Following the same logic of other resources, it'd be great if we can do something like this:

import { aws_route53 as route53, aws_iam as iam } from "aws-cdk-lib";


// code here

new route53.ARecord(this, "Domain", {
        target: route53.RecordTarget.fromIpAddresses("10.10.0.1"),
        zone: route53.HostedZone.fromLookup(this, "Zone", {
          domainName: "example.com",
        }),
        recordName: "example1",
        comment: "Something descriptive",
        ttl: 300,
        deleteExisting: true,
        //       ⬇️
        role: iam.Role.fromRoleName(
          this,
          "DnsChangeRole",
          "SomePredefinedRole",
          { mutable: false }
        )
});

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.33.0

Environment details (OS name and version, etc.)

NixOS Porcupine

@fjinkis fjinkis added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 21, 2022
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Oct 21, 2022
@MrSyn88
Copy link
Contributor

MrSyn88 commented Oct 28, 2022

I'm looking into this

@MrSyn88
Copy link
Contributor

MrSyn88 commented Nov 3, 2022

Or actually, did you already start working on this @fjinkis ?

@fjinkis
Copy link
Author

fjinkis commented Nov 3, 2022

@MrSyn88 No, I just suggested it as a new feature but I didn't start developing anything

@pahud
Copy link
Contributor

pahud commented Nov 3, 2022

@fjinkis Actually the deleteExisting feature is provided by a custom resource which will be generated or imported by the getOrCreateProvider, which means each stack will only have one CustomResourceProvider for this and technically only one IAM role for that provider will be provisioned behind the scene. And if you look at how the role is created it's essentially provisioned with the minimal required privileges. Is there any scenario or benefits you need to use an existing role for the provider rather than the hassle-free auto-generated one?

@fjinkis
Copy link
Author

fjinkis commented Nov 3, 2022

@pahud I find it useful in at least the following two scenarios:

  1. When the CDK user doesn't have access to IAM. I think that it could be common for companies that provide automation services but they don't handle roles or policies because those IAM resources are managed by the client directly (kinda least privilege principle).
  2. I'm working on a solution where each ECS service has its own stack, so I would like to have just one role to handle the DNS records, because I don't like very much the idea of having many roles doing the same thing to the same hosted zone

Do these cases make sense to you?

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 9, 2022

The request makes sense. All roles should be configurable.

@rix0rrr rix0rrr added effort/medium Medium work item – several days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Nov 9, 2022
@rix0rrr rix0rrr removed their assignment Nov 9, 2022
@pahud
Copy link
Contributor

pahud commented Nov 15, 2022

@fjinkis It makes perfect sense.

Looking into the code, the custom resource provider is vended by getOrCreateProvider() which does not support passing existing iam role and it always creates a new role. To make it possible I think we have to first allow getOrCreateProvider() accept a new role prop. Maybe we should create a separate PR for that first.

@pahud
Copy link
Contributor

pahud commented Nov 18, 2022

Hi @fjinkis

CDK v2.51.0 just introduced a IAM feature that allows you to use existing roles for a CDK stack.

check out this doc for more details - https://github.com/aws/aws-cdk/tree/main/packages/%40aws-cdk/aws-iam#customizing-role-creation

And check out my sample below:

image

As you can see no new roles will be created by CDK and the existing role will be used instead. I believe this should satisfy your scenario. Please let me know if it works for you.

@fjinkis
Copy link
Author

fjinkis commented Nov 22, 2022

Hey @pahud it worked! I'd like to apologize for publishing an issue that was finally supported. Although this request was focus to IAM, if you have a moment and if it's not a big deal, could you share with me some thoughts on any way to use a predefined Lambda to perform the record delation (and not one for each stack)?

@pahud
Copy link
Contributor

pahud commented Nov 29, 2022

@fjinkis Sounds like you are trying to use a shared predefined lambda function as the single custom resource provider across multiple stacks? I wonder what motivates you for that? To reduce the number of lambda functions?

@fjinkis
Copy link
Author

fjinkis commented Nov 29, 2022

Hi @pahud! Yeah, I think that is for a better management and also to reduce the lambda resources which does essentially the same thing

@pahud
Copy link
Contributor

pahud commented Nov 29, 2022

Gocha! I think current design is pretty optimal as there's only one shared lambda provider per stack. If you define 100 ARecord resources with deleteExisting enabled in a single stack, you would have 100 custom resources sharing one lambda handler only in this stack. If you have 10 stacks each has 100 ARecords, you have 100x10=1000 custom resources sharing 10x1=10 lambda handlers. Are you planning to reduce the number of custom resources or lambda handlers in this scenario?

@pahud pahud added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 29, 2022
@github-actions
Copy link

github-actions bot commented Dec 2, 2022

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 the closing-soon This issue will automatically close in 4 days unless further comments are made. label Dec 2, 2022
@fjinkis
Copy link
Author

fjinkis commented Dec 6, 2022

I completely agree with you about that! Probably, since it's a custom resource, we cannot specify what lambda should it use. However, responding to your question, my intention was reducing all those common resources that do the same thing over and over: in this case the lambda provider and IAM role

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Dec 6, 2022
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests

5 participants