Use canonical user Id instead of iam user to fix s3 invalid policy issue

[hemanthudimella]

This PR fixes #50

[gyoza]

This will cause your applies to change the policy on every run.

[aknysh]

thanks @hemanthudimella

please rebuild README by executing these commands:

make init
make readme/deps
make readme

It will add the new variables and outputs to README.md automatically.

In general, any changes to README should be made in README.yaml (not in this case), and after that executing the commands above will rebuild README.yaml into README.md and add all new variables and outputs to README.md

thanks

[hemanthudimella]

Done. Rebuilt README.

[aknysh]

/codefresh run test

[aknysh]

@hemanthudimella
please see comments from @gyoza #51 (comment)

you just need to replace ” “, “_” on the old value

[hemanthudimella]

@gyoza @aknysh
Did you mean replace manually? That means every time I apply a change, terraform removes my manual change, and I will have to do it again.
And if you didn't mean manual, please note that replacing " " with "_" works for all new policies. But any policies that are existing wouldn't accept "_" and terraform apply fails. (Existing policies are still accepting " " and " " only)
From my chat with AWS support, the recommended way is to use canonical user id instead.
They were also mentioning that the syntax is not yet finalized and we can expect the "_" to be replaced or removed in future without any notice as they replaced " " with "_" without notice.
However I agree that terraform plan will always show a change in the policy, if we use canonical user id (even though there is no change in the policy). Hopefully terraform fixes this soon.
But until then using canonical user id is the only way (at least as far as i know) to make both existing and new policies work.

[gyoza]

Just commenting on what I found when I encountered this issue. Our CICD process does "things" when terraform reports items to change. So when this is happening when there are actually no changes is somewhat a big deal. This is probably a problem for more than just our process. For now I've opted to fork this module and hardcode appropriate changes until the provider is fixed.

…

On Tue, Sep 24, 2019, 5:29 PM Hemanth Thudimella ***@***.***> wrote: @gyoza <https://github.com/gyoza> @aknysh <https://github.com/aknysh> Did you mean replace manually? That means every time I apply a change, terraform removes my manual change, and I will have to do it again. And if you didn't mean manual, please note that replacing " " with "*" works for all new policies. But any policies that are existing wouldn't accept "*" and terraform apply fails. (Existing policies are still accepting " " and " " only) From my chat with AWS support, the recommended way is to use canonical user id instead. They were also mentioning that the syntax is not yet finalized and we can expect the "*" to be replaced or removed in future without any notice as they replaced " " with "*" without notice. However I agree that terraform plan will always show a change in the policy, if we use canonical user id (even though there is no change in the policy). Hopefully terraform fixes this soon. But until then using canonical user id is the only way (at least as far as i know) to make both existing and new policies work. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#51?email_source=notifications&email_token=AAEJWBVXW7VYEJKXDVTI673QLKWGZA5CNFSM4IYXO6UKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7QGJVA#issuecomment-534799572>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJWBTVDUL7KHP32C5GSB3QLKWGZANCNFSM4IYXO6UA> .

[hemanthudimella]

@gyoza Same here. I ended up forking the module. :)
@aknysh I suppose this module, at the moment, is unusable as is.

[aknysh]

/codefresh run test

[aknysh]

@hemanthudimella
please run terraform fmt (our CI/CD tests are failing).
After that, we'll merge the PR

[aknysh]

@hemanthudimella please rebase and run terraform fmt and then rebuild README:

make init
make readme/deps
make readme

[maximmi]

@hemanthudimella is this PR still actual due to changes made and merged after this one?

[hemanthudimella]

Redundant PR