github_actions_organization_secret always applies the value, even if it's unchanged

[greggilbert]

Terraform Version

Terraform v0.13.5

Affected Resource(s)

Please list the resources as a list, for example:

• github_actions_organization_secret

Terraform Configuration Files

resource "github_actions_organization_secret" "this" {
  for_each = toset(local.secrets)

  secret_name     = each.value
  visibility      = "selected"
  plaintext_value = "(placeholder)"   # we'll change these values outside of Terraform

  selected_repository_ids = [
    for repo in data.github_repository.repos : repo.repo_id
  ]

  lifecycle {
    ignore_changes = [
      plaintext_value
    ]
  }
}

Expected Behavior

In this example, I'm giving providing a list of repositories that can access the secret. You can see I'm using (placeholder) as the plaintext value, with the idea that I would go into Github outside of TF and add in the real value once.

When I change the list of repositories, I expect that the real value I inputted would remain the same.

Actual Behavior

However, it actually replaces the real value with the placeholder.

Steps to Reproduce

1. Use the github_actions_organization_secret resource to create a value like FOO with a default value of (placeholder)
2. terraform apply the change/creation
3. Go into your Github organization and edit the secret to have a value like Hello World
4. Using the above config, modify the selected_repository_ids and apply the change
5. In your Github Actions workflow, create a step like this:

     - name: DEBUG!
       env:
         MY_SECRET: ${{ secrets.FOO }}
       run: |
         echo ${MY_SECRET} | sed 's/./& /g' 

6. Push and/or trigger the Github Action
7. Note how the value that gets printed out is (placeholder) and not the expected Hello World

[SanderKnape]

We're running into this issue as well. Any thoughts on how this could be fixed?

[marcinwyszynski]

Well, this is probably linked to my earlier fix. The whole idea of declaring resources with Terraform is for Terraform to make sure that the remote state matches the declared state. If you want Terraform to be blind to external changes, please explicitly set ignore_changes on this field.

[jvanbrunschot]

I think that's the whole problem, that ignore_changes don't work.

[marcinwyszynski]

@jvanbrunschot have you tried ignoring updated_at, too?

[jvanbrunschot]

@marcinwyszynski sory for the slow response but adding both plaintext_valueand updated_at won't work either.

[sivapalan]

We also have a related use case where ignore_changes is not respected (after the change introduced in #740). In our case, we want to manage the secrets with Terraform, but use an external tool for managing selected_repository_ids for each secret.

The updated_at field is updated each time a repository is added or removed from the list, so that makes this provider assume that the secret has been changed, and thereby forces a destroy and recreation of the secret.

@jcudit What are your thoughts about introducing an additional attribute allowing users to choose the preferred behaviour?

Example:

destroy_on_drift - (Optional) Destroy and recreate secret if secret has been updated externally. Default is true.

[jcudit]

@sivapalan that is a solid suggestion. Happy to help support an implementation if anyone wants to pick this up!

From a code organization perspective, we've got a few different types of secrets that would benefit from such a flag. However, we've yet to refactor shared code for each secret type which may complicate or cause drift depending on how this gets implemented.

[yordis]

I gave it a try with #1351, but I am bending my brain regarding how to think about the unit-testing environment. I put some breakpoints to see how I could get to the resourceGithubActionsOrganizationSecretRead function, but no test ended up in resourceGithubActionsOrganizationSecretRead, so I can't learn from some examples.

Some support is appreciated

[kfcampbell]

@yordis how are you executing your unit tests? Would you mind sharing (redacted) environment variables and the command you're using to execute them?

Side note: that drift-detection snippet is sprinkled all over our code. I wonder if there's a nice way to refactor that so the logic can be shared rather than duplicated.