integrations / terraform-provider-github

Terraform GitHub provider
https://www.terraform.io/docs/providers/github/
MIT License
912 stars 752 forks source link

[BUG]: `github_actions_organization_secret` secret recreated after manual update #1383

Open johankees opened 2 years ago

johankees commented 2 years ago

Terraform Version

Run terraform -v to show the version. If you are not running the latest version of Terraform, please upgrade because your issue may have already been fixed.

terraform -v
Terraform v1.3.5
on linux_amd64
+ provider registry.terraform.io/integrations/github v5.9.1

Affected Resource(s)

Terraform Configuration Files

resource "github_actions_organization_secret" "secret" {
  secret_name = "TEST_SECRET"
  visibility  = "private"

  lifecycle {
    ignore_changes = [
      plaintext_value,
      encrypted_value
    ]
  }
}

Debug Output

State file

{
  "version": 4,
  "terraform_version": "1.3.5",
  "serial": 20,
  "lineage": "f6210cb1-a789-4343-822c-60148393fdc3",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "github_actions_organization_secret",
      "name": "admin-token",
      "provider": "provider[\"registry.terraform.io/integrations/github\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "created_at": "2022-11-23 10:47:28 +0000 UTC",
            "encrypted_value": "",
            "id": "TEST_SECRET",
            "plaintext_value": "",
            "secret_name": "TEST_SECRET",
            "selected_repository_ids": null,
            "updated_at": "2022-11-23 10:47:53 +0000 UTC",
            "visibility": "private"
          },
          "sensitive_attributes": [],
          "private": "bnVsbA=="
        }
      ]
    }
  ],
  "check_results": null
}

Panic Output

N/A

Expected Behavior

The secret should not be recreated nor updated. I.e. terraform runs should be idempotent.

Actual Behavior

The secret gets recreated resetting the value of the secret to an empty string.

It looks like the id gets changed when the value was manually set in GitHub, hence the provider lost track of the resource. The state file does have the correct information. (see Debug output)

The linked issue (#974) mentions the use of ignore_changes lifecycle. This doesn't resolve the issue. Tested by adding updated_at, but this field is ignored.

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform apply
  2. make manual update in GitHub to set the value
  3. terraform apply

Important Factoids

N/A

References

wadells commented 1 year ago

I've also encountered this same behavior with github_actions_environment_secret.

resource "github_actions_environment_secret" "placeholder" {
  repository      = "test"
  environment     = "test"
  secret_name     = "TEST"
  plaintext_value = "" # placeholder value, secrets mgmt not implemented yet

  lifecycle {
    ignore_changes = [ 
      plaintext_value
    ]   
  }
}

After the secret is created, I edit it by hand to put a value in place. However, upon the next terraform run the secret is recreated with an empty value.

aandac commented 1 year ago

+1

kwantopia commented 1 year ago

Yes please help fix!

casonlassomd commented 1 year ago

Please help to fix. Looking forward.

GabrielFerrarini commented 1 year ago

Even selecting all won't work (using the same example from @wadells

resource "github_actions_environment_secret" "placeholder" {
  repository      = "test"
  environment     = "test"
  secret_name     = "TEST"
  plaintext_value = "" # placeholder value, secrets mgmt not implemented yet

  lifecycle {
    ignore_changes = all   
  }
kfcampbell commented 1 year ago

It looks like in cases of both creating and updating, we call CreateOrUpdateOrgSecret which routes to this API reference.

Is the suggestion that the provider avoids making this call in some scenarios on updating?

GabrielFerrarini commented 1 year ago

@kfcampbell In the particular scenario where the below is set, because we're explicitly asking terraform to not update these secrets.

  lifecycle {
    ignore_changes = all   
  }

or

  lifecycle {
    ignore_changes = [ 
      plaintext_value
    ]   
  }

or

  lifecycle {
    ignore_changes = [ 
      encrypted_value
    ]   
  }

or similar

GabrielFerrarini commented 1 year ago

@kfcampbell @nickfloyd Any ideas if this is going to be worked on soon?

kfcampbell commented 1 year ago

@GabrielFerrarini unfortunately GitHub's SDK team generally doesn't have the bandwidth to work on this type of issue directly. Do you have the interest or inclination to open up a PR for this behavior?

vparmeland commented 7 months ago

Any news ??

nbali commented 1 week ago

So this is the logic that is responsible for this behaviour:

    if updatedAt, ok := d.GetOk("updated_at"); ok && updatedAt != secret.UpdatedAt.String() {
        log.Printf("[INFO] The secret %s has been externally updated in GitHub", d.Id())
        d.SetId("")
    } else if !ok {
        if err = d.Set("updated_at", secret.UpdatedAt.String()); err != nil {
            return err
        }
    }

Wouldn't this solve our issue, but keep the original functionality as well?

    if updatedAt, ok := d.GetOk("updated_at"); ok && updatedAt != secret.UpdatedAt.String() {
        log.Printf("[INFO] The secret %s has been externally updated in GitHub", d.Id())
        d.Set("encrypted_value", "")
        d.Set("plaintext_value", "")
    } else if !ok {
        if err = d.Set("updated_at", secret.UpdatedAt.String()); err != nil {
            return err
        }
    }