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

fix type and formatting of deployment_arns variable #31

Merged
merged 3 commits into from
Mar 3, 2020

Conversation

geota
Copy link
Contributor

@geota geota commented Aug 14, 2019

The current type of var.deployment_arns is set as map(string). This expects both the key and value to be of type string. The proper type is map(string, list(string)). I don't think terraform supports this level of type declaration. The object type does not seem to work here since we dont know the names of all the keys (i.e. they map to user defined arns).

I changed the type to map(any) which fixed the type error, but then fails when trying to format the variable into the underlying IAM policy document. The fix was easy here, changed format -> formatlist and added a flatten to collapse the nested lists.

This unblocked me and I was able to successfully apply the module when setting deployment_arns to the non-default empty object.

@maximmi
Copy link
Contributor

maximmi commented Mar 3, 2020

/codefresh run test

1 similar comment
@maximmi
Copy link
Contributor

maximmi commented Mar 3, 2020

/codefresh run test

@maximmi
Copy link
Contributor

maximmi commented Mar 3, 2020

/rebuild-readme

@maximmi
Copy link
Contributor

maximmi commented Mar 3, 2020

/rebuild-readme

@maximmi
Copy link
Contributor

maximmi commented Mar 3, 2020

/codefresh run test

Copy link
Contributor

@maximmi maximmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @geota

@maximmi maximmi merged commit a8e3b86 into cloudposse:master Mar 3, 2020
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.

3 participants