-
Notifications
You must be signed in to change notification settings - Fork 2
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
pulumi: Add pulumi-lambda-cert module #2
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really awesome 🎉
I love seeing the creation of new reusable components like this, especially when published to NPM 👍
A few minor comments, mostly questions, inline.
Thank you for letting me take a look!
/** | ||
* Tags is a dictionary object representing tags to be applied to an AWS resource. | ||
*/ | ||
export interface Tags { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we merged your aws.Tags
change yet? (We should 😄) Would that let you just use that directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been merged, but isn't part of the 0.14.5
release that is current (https://github.com/pulumi/pulumi-aws/commits/v0.14.5) - so I didn't want to depend on it until I can make this package depend on a stable release. I'll open an issue to fix this up though!
|
||
This package provides a component named `LambdaCert`, which can be used to create the resources | ||
needed to run [Lambda Cert][lambdacert], a function for maintaining Let's Encrypt certificates using | ||
AWS Lambda. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is super cool that you can do this. One thing I've wondered from time to time, do you think we should eventually look into a Let's Encrypt resource provider, so that you can declare a certificate as a first class resource?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more awkward. Terraform has an ACME provider, but I've personally found it to be of no use, as it introduces a runtime dependency to go and renew certificates. Since the validity period of certificates issued by Let's Encrypt is on the low side, this needs doing fairly often, hence the reason I built a Lambda to do it. This may not be an issue for things with frequent iterations, but personally I like to have things "self-maintain" without requiring operator input.
The (missing) companion to this is a service which runs on instances making use of the certificates to download the certificate from the bucket each day, compare against the version the instance has, and reload the TLS configuration of the target service however necessary.
Using this is also only necessary when it's desirable to maintain TLS without interception all the way to the service instances - Vault is my canonical example. Most things running in AWS in particular can happily terminate TLS at the load balancer, API Gateway etc, and use ACM-issued certificates, which are strictly better.
pulumi/src/index.ts
Outdated
} | ||
|
||
private constructor(name: string, opts?: ResourceOptions) { | ||
super("operator-error:aws:LambdaCert", name, {}, opts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing to mention, which I am largely mentioning not because I think you should necessarily do it, but because I'd love feedback on this, is that components can have tracked inputs and outputs.
For example, instead of {}
, I think you'd supply the inputs
passed to create
. And then you'd make a call to this.registerResourceOutputs(...)
with the resulting LambdaCertOutputs
.
This would cause the state to get recorded in the resulting checkpoint state. But the reason I'm questioning this whole area of our model is that I don't see how that'd immediately improve your life in any way here. (A minor point, it would allow us to show that information in the pulumi.com service UI ...)
Is that even remotely interesting in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having the information shown in the service UI is definitely interesting. I'll look at adding this.
export const keyBucketArn = outputs.then(o => o.keyBucketArn); | ||
export const keyBucketName = outputs.then(o => o.keyBucketName); | ||
export const kmsKeyArn = outputs.then(o => o.kmsKeyArn); | ||
export const kmsKeyId = outputs.then(o => o.kmsKeyId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish we had a better way of doing this sort of thing with less boilerplate 🙁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened an issue with a suggestion which may work here - I haven't looked at implementing it yet though. It would definitely be nice to eliminate the boilerplate here.
Thanks for reviewing, @joeduffy! There are a few things here I'll follow up on before merging and publishing the first version of this. |
This pull request adds a new Pulumi module,
@operator-error/pulumi-lambda-cert
which creates the resources necessary to run lambda-cert.