aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.56k stars 3.87k forks source link

[aws-certificatemanager] Add fromCertificateName method to Certificate object #10792

Open covertbert opened 3 years ago

covertbert commented 3 years ago

There is currently a fromCertificateArn method on the Certificate object but the problem with using this is that the ARN could change if the resource is recreated leading to any stack referencing this ARN breaking. My proposal gives, I believe, a more robust way of referencing a certificate that would survive the recreation of this resource.

Use Case

In order to reference pre-existing certificates by their name

Proposed Solution

Create a fromCertificateName method on the Certificate object


This is a :rocket: Feature Request

njlynch commented 3 years ago

Hi @covertbert ,

Can you expand a bit on what you're looking for here? What do you imagine the input to fromCertificateName is? Certificates don't have intrinsic "names"; there are domain names associated with the certificate, or the certificate ARN. The former are not guaranteed to be unique (you can have multiple certs with the same domain names), and the latter isn't terribly portable.

The most common request of this type lands on being able to use a consistent record locator across multiple regions and/or accounts. The best recommendation I've heard to that effect before is to use SSM parameters, and store the ARNs in a consistently-named parameter in each region where you need the certificate.

Can you elaborate a bit on what interface you're expecting here, and how you'd use it?

covertbert commented 3 years ago

What you describe in your second paragraph is actually a great idea that I hadn't thought of!

With regards to the certificate name, if you look in the UI there is a name column (other than the domain name). I've admittedly not actually used it as in my mind I was thinking of using the domain name but I hadn't thought about the fact that they're not necessarily unique.

njlynch commented 3 years ago

The other option -- if you're defining your certificate and other infrastructure in the same region, but different stacks -- is just to pass the reference across stacks.

class MyCertStack extends cdk.Stack {
  public readonly certificate: acm.ICertificate;
  constructor(...) {
    this.certificate = new acm.Certificate(...);
  }
}

interface MyOtherStackProps extends cdk.StackProps {
  readonly certificate: acm.ICertificate;
}

class MyOtherStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props: MyOtherStackProps) {
    // Use props.certificate here to reference the cert.
  }
}

The trouble with both the cert "name" (e.g., tag) or domain names is the lack of guaranteed uniqueness, as well as the fact that both would require a lookup for runtime context. If neither of the above options solve your problem, we can keep this open as a request for a context lookup.

covertbert commented 3 years ago

Unfortunately the certificate I'm trying to access will always be in a different region which is why I've ended up down this rabbit hole in the first place (and also the certificate isn't created using the CDK but by standard CloudFormation templates anyway)!

I guess using SSM parameters does solve my problem but it just doesn't feel particularly elegant however I'm willing to give it a go.

Feels like there should be a nice way to do what I'm trying to do as I think it's pretty common to have certificates in a different region from where you're defining other resources.

njlynch commented 3 years ago

Ok, we can keep this open for a context lookup-based fromCertificateName, that either operates on Tags or the Domain Names. Either option would need to handle the cases where either no such names exist, or multiple records exist with the same names.

heiba commented 3 years ago

+1 We really need to be able to lookup ACM certificates without hardcoding ARNs in our CDK code.

@njlynch would be great if you could update us on this request and whether someone in the CDK team is working on it ?

njlynch commented 3 years ago

Hi @heiba. We are unable to work on this immediately, and can't provide an update on when we will be able to get to it.

We use :+1:s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

lucasgherculano commented 2 years ago

How many more likes this one needs to get started? Really look into this

skinny85 commented 2 years ago

@lucasgherculano often, the quickest way to get a feature into the CDK is by contributing a PR adding it. Here's our "Contributing" guide: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md - the process is easier than you might think!

Thanks, Adam

vobarian commented 1 year ago

What do you imagine the input to fromCertificateName is? Certificates don't have intrinsic "names"; there are domain names associated with the certificate, or the certificate ARN. The former are not guaranteed to be unique (you can have multiple certs with the same domain names), and the latter isn't terribly portable.

Terraform has solved this problem by allowing various filter parameters for the lookup: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/acm_certificate

It seems like a good model to follow. A nice addition could be filtering by tags. Even as a minimal first take, the ability to return the most recently issued cert for a given domain name with a status of "issued" would be nice.

dmoughabghab commented 1 year ago

How does CDK work under the hood? Is it AWS cli calls? I would consider doing this PR if so.

Seems simple enough, I'd use

aws acm list-certificates --query CertificateSummaryList[].[CertificateArn,DomainName] \ --output text | grep domainname.com| cut -f1

Taken from https://gist.github.com/bashtoni/995c0683bb18fd19eaefdc296a9401d8

to get the arn and store it in the cdk.context.json

My only complaint would be that AWS didn't hire me years ago when I applied and now I'd be working for them for free haha 😅

krisztianpinter commented 1 year ago

how do you guys plan to make it happen? aren't you limited by what CloudFormation can do? i'm trying to figure this out for CloudFormation for a while, but it seems to be not possible. everyone recommends to export the ref from the other stack (if was created via cf), or just use the arn as parameter.

dmoughabghab commented 1 year ago

Reading the link https://docs.aws.amazon.com/cdk/v2/guide/context.html

I suspect you can do something like this

`export fromCertificateName(certName: string, context: Context) { let certDetails = this.node.tryGetContext('certDetails'); //Probably need to refine this if(!certDetails) { const certVal = runThisCliCommandSomehow(aws acm list-certificates --query CertificateSummaryList[].[CertificateArn,DomainName] \ -- output text | grep domainname.com| cut -f1) // Maybe use a package and you might store more than just the arn context.node.setContext('someUniqueIdUsingTheDomainName', certVal) }

return { variables around the cert } : ICert }`

Honestly doesn't even seem that hard, just need some tips from the AWS team as I suspect in such a large code base they have established patterns and helper methods

No idea why the code wont format

EDIT: On second thought, the main issue is probably when the region/account of the cert is different then the region running cdk (think pipeline), there would also need to be a way to make use of the authentication being used on the stack

github-actions[bot] commented 3 months ago

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.

mhvelplund commented 1 week ago

Is the underlying problem that CDK just generates CloudFormation templates, and the only way to refer to a Certificate is by its ARN that includes a random id that you can't guess? Thus you either need a custom CF resource to help you do the lookup in a lambda, or you need to get the ARN from some other source, such as a context variable.