-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Remove need to destroy/create AWS instance for iam_role_profile change #11862
Conversation
@stack72 thanks, i'll look into it. Unfortunately there are no tests for EC2 instance IAM profiles so it's hard to know what knock on effects are had. I'll update accordingly |
<3 thanks for the work on this @myoung34 :) |
The documentation says that with the SDK v1.6.20 we can now "associate an IAM profile to running instances that do not have any." Do we need to check to verify that the Instance does not have an IAM profile attached? Do we know what the error is, if it does have a profile attached and we try to update it this way? |
@catsby @stack72 this now works
|
@catsby per your question: yes. I added a |
@stack72 any way to yank this in? |
Hi @myoung34 I promise i will get a look at this today - I think i am going to need to add a test to it to make sure it works as expected Paul |
Hi @myoung34 I added a test that covers the behaviour
I am going to merge this manually now to get this test added :) Thanks for all the work here Paul |
well shit. this broke terraforming ec2 at my company. we don't have permission to ec2:ReplaceIamInstanceProfileAssociation . i'll try and get the process wheels spinning on that, but is there any chance the old behavior can be honored or toggled besides using 8.7? |
It only runs replace if there's a change to the instance profile. The code change doesn't affect create or destroy. |
i could deal with it better if that were accurate, but ima stick to 8.7 until this blows over.
which translates to {
"allowed": false,
"explicitDeny": false,
"matchedStatements": {
"items": []
},
"failures": {
"items": []
},
"context": {
"principal": {
"id": "redacted",
"arn": "arn:aws:sts::redacted"
},
"action": "ec2:ReplaceIamInstanceProfileAssociation",
"resource": "arn:aws:ec2:us-east-1:redacted:instance/i-05ea1021172887bef",
"conditions": {
"items": [
{
... tags, properties |
@automaticgiant I think you misread. |
indeed. i suppose i should track down the commit/pr for the create behavior and despair there instead. |
the behavior exhibits on tf 8.8 but not 8.7. this was the only change to this file, so idk how to explain the observed behavior. could it be this but something about retrying unreliable iam api calls triggers an update of a partial create? |
it could be something in the underlying API calls that are not due to terraform. you can try That might actually be a reasonable assumption but that would mean it didnt work before since updates never updated IAM association before this change. the |
here's the important bits, as far as i can tell. btw, this was after removing the state file.
added more previous. sorry - have to be careful not to incur the wrath of infosec dept. |
@automaticgiant the only thing that stands out is that looks like it's working on an entity that already exists. The first request/response there is describe instance. Line 1: Line 11: Do you have the full trace? If it's not creating it, then it's very possible it is updating it. |
Is there a way how to enforce the old behaviour? We are not prepared for this :( |
Hi all We found an issue with the code that meant that when you created an instance with an IAM Instance Profile, we would try and immediately replace it (as described in #12898) but there is a PR to make this only happen on Update //cc @myoung34 @automaticgiant Hope this helps Paul |
Thanks @stack72 ! |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Resolves #11852