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

alks_iamrole type change goes unnoticed #17

Open
jantman opened this issue Jan 5, 2018 · 6 comments
Open

alks_iamrole type change goes unnoticed #17

jantman opened this issue Jan 5, 2018 · 6 comments
Assignees

Comments

@jantman
Copy link

jantman commented Jan 5, 2018

We're using version 0.9.11 with Terraform 0.9.11.

Scenario

  1. We create an alks_iamrole of type = "Amazon EC2".
  2. Everything works right; some time passes.
  3. Someone (via a ticket) changes the Trust Policy on this role.
  4. Terraform plan shows no changes

Expected Behavior

Terraform plan shows either a different "type" for the role, or an unknown "type".

I don't know the ALKS API very well, but it feels like this violates the idempotent and authoritative nature of terraform.

@brianantonelli
Copy link
Member

Thanks for reporting this, digging into it now! :)

@brianantonelli
Copy link
Member

Alright, I've resolved the issue. I created a alks_iamrole with type set to Amazon EC2 and then changed the trust policy to set it to Lambda. Now when I run terraform plan it detects the change and forces a new resource:

brianantonelli in ~/Dev/go/src/github.com/Cox-Automotive/terraform-provider-alks/examples on develop*
🍔  terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

alks_iamrole.test_role: Refreshing state... (ID: aba-test-123456)
aws_iam_role_policy_attachment.sr-attach: Refreshing state... (ID: aba-test-123456-20180110192336719600000001)
aws_iam_role_policy.test_policy: Refreshing state... (ID: aba-test-123456:test_policy)

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

-/+ alks_iamrole.test_role (new resource required)
      id:                       "aba-test-123456" => <computed> (forces new resource)
      arn:                      "arn:aws:iam::120678615247:role/acct-managed/aba-test-123456" => <computed>
      include_default_policies: "false" => "false"
      ip_arn:                   "arn:aws:iam::120678615247:instance-profile/acct-managed/aba-test-123456" => <computed>
      name:                     "aba-test-123456" => "aba-test-123456"
      role_added_to_ip:         "true" => <computed>
      type:                     "" => "Amazon EC2" (forces new resource)


Plan: 1 to add, 0 to change, 1 to destroy.

------------------------------------------------------------------------

Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run.

@brianantonelli
Copy link
Member

brianantonelli commented Jan 10, 2018

Actually there's still an issue, the ALKS API isn't currently returning the role type as null so this issue can't be fixed until the API is updated. I'll cut a ticket with them to get it resolved..

@jantman
Copy link
Author

jantman commented Jan 10, 2018

Ah ok, thanks! Admittedly this is a real edge case that theoretically can only be caused by human error, so I don't think there's a lot of pressure to fix it. But it's definitely not expected behavior, so I thought it best to open the issue.

Thanks so much for looking into this!

@ekozlowski
Copy link
Contributor

This needs to have the role actually passed into the ALKS API, instead of the role name. Right now, ALKS doesn't support this, though there are stories on the backlog for doing this. See this issue specifically.

There was some discussion there around "looking up" the type of role, and returning it in the ALKS API - unfortunately, if any modifications were made to the role type, the lookup would fail.

The solution here is to allow the policy for the role to be passed into the API, and validate the policy meets our requirements. This would allow the ALKS Terraform provider to detect drift, and apply policy updates.

I'll bring this up with the team again, and ensure we have priority on the appropriate backlog stories. 👍

@aaron-seitz
Copy link
Contributor

aaron-seitz commented Apr 5, 2019

Internal Tools team has had an initial discussion on this item - we agree it's still something that makes sense to look into. Some more digging will need to be done on the ALKS side to determine the level of effort getting a workable solution will take.

A Spike has been made in the Internal Tools backlog for investigating this in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants