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

Update IAM Policy Statements to support multiple resources #143

Merged
merged 1 commit into from
Mar 3, 2017
Merged

Update IAM Policy Statements to support multiple resources #143

merged 1 commit into from
Mar 3, 2017

Conversation

bpholt
Copy link
Contributor

@bpholt bpholt commented Mar 1, 2017

This should make writing policy statements that affect multiple resources a lot cleaner. Right now, AFAICT you have to break them out into multiple statements, which is hard to read in the generated template and, eventually, the generated policy. Multiple resources are supported by IAM.

This should be backwards-compatible with the exception that one has to

import com.monsanto.arch.cloudformation.model.resource.PolicyStatement._

to get the implicits. I'm open to moving them to another location that is more likely to already be imported if someone has an idea of where that might be.

The generated JSON will now always contain an array for the Resource field, but that should have no impact as far as CloudFormation/IAM are concerned, and matches the behavior of the Python AWACS library.

@bpholt bpholt changed the title update IAM Policy Statements to support multiple resources Update IAM Policy Statements to support multiple resources Mar 1, 2017
@bkrodgers
Copy link
Contributor

What if we put them in the Token companion object with the other implicits, i.e., somewhere around here:
https://github.com/MonsantoCo/cloudformation-template-generator/blob/master/src/main/scala/com/monsanto/arch/cloudformation/model/AmazonFunctionCall.scala#L227

For one thing, I think they're generic anyway and may come in handy in other cases. And for another, people are probably already importing either Token._ directly, or are getting it because that import is in model's package object. At least in my templates,

Can you think of any issues having this conversion available globally like that?

@bpholt
Copy link
Contributor Author

bpholt commented Mar 3, 2017

@bkrodgers Putting them in the Token companion object works for me. I was able to remove the explicit imports I had to add before, so I think it’s likely to provide the backward compatibility we’re looking for. Updated in 488e567.

@bkrodgers
Copy link
Contributor

Thanks, LGTM!

@bkrodgers bkrodgers merged commit e7ef1b8 into Bayer-Group:master Mar 3, 2017
@bpholt bpholt deleted the iam-resources branch March 10, 2017 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants