https://pulumi.com logo
Join Slack
Powered by
Hi all, I’ve just pushed (to GitHub, not to NPM) a...
# general
s
Hi all, I’ve just pushed (to GitHub, not to NPM) another new component for AWS: https://github.com/jen20/pulumi-aws-instance-profile - feedback appreciated! There’s a bit more to do in terms of making it accept things like a list of 
Statements
optionally instead of complete `PolicyDocument`s, but this is a fairly common set of resources to find together in more traditional AWS infrastructure.
c
To be honest I’m not familiar with the resources your component describes, so I don’t have any meaningful feedback there. But one thing that I noticed was how you named the resources:
Copy code
... = new aws.iam.InstanceProfile(`${baseName}-instance-profile`, ...
The “name” you give something to a Pulumi program needs a little care, which aren’t well documented anywhere. (Note to self: write a blog post.) The first parameter to a resource is the name that you give to Pulumi, which will then be converted to some stable name for the resource, e.g. 
${baseName}-instance-profile-12345
. This however means that you cannot use the same “pulumi name” for another resource, as it woudl lead to a conflict. So if you created another resource — even with a different type — with 
${baseName}-instance-profile
, you would get an error. In your code this is probably just fine, since you are including the 
${baseName}
variable.
The only extra thing to be on the lookout for is how the resource gets named in the cloud resource provider, e.g. AWS. Will the resource default to using name 
${baseName}-instance-profile
as well? And must that name be unique within the same AWS account, or globally across all accounts? e.g. in AWS S3 bucket names must be globally unique. Many resource names must be unique within the (region x account), etc.
Again, your code is probably fine, but it’s something to keep in the back of your mind. If for some reason you need to add another stable variable, you can get the name of the Pulumi stack with something like 
pulumi.getStackName()
. e.g. 
${pulumi.getStackName()}-${baseName}-instance-profile
. That’s obviously a mouthful, but in situations where you need some stable identifier to disambiguate names, you have to use it.
Make sense? I’ll try to write some docs on this since it is subtle. But other than that your repo looks good, modulo maybe adding some more comments in the code. FWIW we have been discussing some notion of showing Pulumi packages on GitHub somewhere within app.pulumi.com. That feature is probably a ways off, but we definitely want to have some way to highlight package reuse. Since other people would benefit from the work you’ve done here.
s
Actually this was a question that came out of doing this work: it doesn’t appear to be possible to use the 
namePrefix
option since Pulumi appears to do something with the 
name
option.
Does Pulumi automatically do something with name prefixing?
c
I am not familiar enough with that part of the code. @white-balloon-205 or @lemon-spoon-91807 might be able to answer that for you.
👍 1
m
Does Pulumi automatically do something with name prefixing?
In general, TF-based providers implement auto-naming using a 
defaults
layer on top of the TF schemas. This auto-naming suffers from exactly the problem you've noticed: it conflicts with 
namePrefix
if that property is also set. I'm not sure what the best way is to address this, but we definitely need to do something.
s
@microscopic-florist-22719 Probably easiest to just strip them out if Pulumi already does the correct thing?
m
That's definitely on the table. I think we wanted to make sure that we can in fact cover all relevant scenarios before doing so.
s
So if 
name
is explicitly set, does that override the automatic numbering system?
m
Yes.
w
BTW - re: the original feedback in this thread, the approach in the repo of using 
${baseName}-instance-profile
is actually best practice and similar to what we use in most existing Pulumi packages. That is, combining a name provided by the user of the component with lightly-descriptive details on the child resource itself.
m
@white-balloon-205 related to best-practices docs, it's critical that we document the naming uniqueness requirements and URN format
👍 1
w
s
So is there a way currently to specify a different name prefix than the resource name?
w
No. (well - there are some tricks to generate your own stable randomness and then use 
name
explicitly - but that certainly isn't ideal).
s
Ah yes that doesn’t sound too fun. It would be nice to be able to set that, as the format of names that you’d want to use in resource names (to match nicely with URNs) may not meet the usual conventions for the resources themselves (e.g. URNs want 
hyphenated-names
but AWS roles want 
PascalCase
etc)
w
Yeah - we could in principle add something equivalent to 
namePrefix
for all resources - not sure we have an issue tracking that specific suggestion - feel free to open one in `pulumi-terraform‘.
s
Thanks for clarifying how this works right now
2 Views
Open in Slack
PreviousNext
Powered by