https://pulumi.com logo
#general
Title
# general
s

stocky-spoon-28903

07/17/2018, 4:33 AM
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

colossal-beach-47527

07/17/2018, 2:51 PM
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

stocky-spoon-28903

07/17/2018, 3:38 PM
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

colossal-beach-47527

07/17/2018, 4:13 PM
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

microscopic-florist-22719

07/17/2018, 5:37 PM
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

stocky-spoon-28903

07/17/2018, 5:56 PM
@microscopic-florist-22719 Probably easiest to just strip them out if Pulumi already does the correct thing?
m

microscopic-florist-22719

07/17/2018, 5:57 PM
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

stocky-spoon-28903

07/17/2018, 5:59 PM
So if
name
is explicitly set, does that override the automatic numbering system?
m

microscopic-florist-22719

07/17/2018, 6:17 PM
Yes.
w

white-balloon-205

07/17/2018, 6:22 PM
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

microscopic-florist-22719

07/17/2018, 6:28 PM
@white-balloon-205 related to best-practices docs, it's critical that we document the naming uniqueness requirements and URN format
👍 1
w

white-balloon-205

07/17/2018, 6:35 PM
s

stocky-spoon-28903

07/17/2018, 6:38 PM
So is there a way currently to specify a different name prefix than the resource name?
w

white-balloon-205

07/17/2018, 6:43 PM
No. (well - there are some tricks to generate your own stable randomness and then use
name
explicitly - but that certainly isn't ideal).
s

stocky-spoon-28903

07/17/2018, 6:46 PM
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

white-balloon-205

07/17/2018, 6:49 PM
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

stocky-spoon-28903

07/17/2018, 6:52 PM
Thanks for clarifying how this works right now
2 Views