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

sensitive tfe_variable values are wiped upon description change #839

Closed
philbal611 opened this issue Mar 31, 2023 · 4 comments · Fixed by #873
Closed

sensitive tfe_variable values are wiped upon description change #839

philbal611 opened this issue Mar 31, 2023 · 4 comments · Fixed by #873
Labels

Comments

@philbal611
Copy link

Terraform Cloud/Enterprise version

Terraform Enterprise v202209-2

Terraform version

Terraform v1.4.2
+ provider registry.terraform.io/hashicorp/tfe v0.43.0

Terraform Configuration Files

Parent Workspace
// TFE_TOKEN, TFE_HOSTNAME set on workspace
provider "tfe" {}

data "tfe_workspace" "poc-child" {
  name         = "poc-child"
  organization = "Sandbox"
}

resource "tfe_variable" "aws_access_key_id" {
  key          = "AWS_ACCESS_KEY_ID"
  value        = ""
  sensitive    = false
  category     = "env"
  workspace_id = data.tfe_workspace.poc-child.id

  lifecycle {
    ignore_changes = [value]
  }
}

resource "tfe_variable" "aws_secret_access_key" {
  key          = "AWS_SECRET_ACCESS_KEY"
  value        = ""
  sensitive    = true
  category     = "env"
  workspace_id = data.tfe_workspace.poc-child.id
  //description  = "description change"

  lifecycle {
    ignore_changes = [value]
  }
}

resource "tfe_variable" "aws_region" {
  key          = "AWS_REGION"
  value        = "us-east-1"
  sensitive    = false
  category     = "env"
  workspace_id = data.tfe_workspace.poc-child.id
}
Child Workspace
resource "aws_s3_bucket" "b" {
  bucket = "my-tf-test-bucket"

  tags = {
    Name        = "My bucket"
    Environment = "Dev"
  }
}

Debug Output

...

Expected Behavior

  1. Create parent & child workspace
  2. Parent - Apply workspace as coded above (should succeed)
    a. Populate the values for AWS_ACCESS_KEY_ID & AWS_SECRET_KEY in the TFE UI
  3. Child - Apply workspace as coded above (should succeed)
  4. Parent - Remove the comment for tfe_variable.aws_secret_access_key to add the description. Run terraform apply & confirm (should succeed)
  5. Child - Modify the aws_s3_bucket.b resource's bucket attribute with an extra character. Run terraform plan.

The plan run in step 5 should present the bucket change.

Actual Behavior

The parent run adding the description attribute on the tfe_variable.aws_secret_access_key resource only shows the description change, given the ignore_changes includes the value attribute. However, the child workspace fails due to a provider authentication error below.

The change to the description on the environment variable AWS_SECRET_ACCESS_KEY has inadvertently and silently wiped the value that was previously added manually.

Terraform v1.4.2
on linux_amd64
Configuring remote state backend...
Initializing Terraform configuration...

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: Invalid provider configuration
│ 
│ Provider "registry.terraform.io/hashicorp/aws" requires explicit
│ configuration. Add a provider block to the root module and configure the
│ provider's required arguments as described in the provider documentation.
│ 
╵
╷
│ Error: configuring Terraform AWS Provider: no valid credential sources for Terraform AWS Provider found.
│ 
│ Please see https://registry.terraform.io/providers/hashicorp/aws
│ for more information about providing credentials.
│ 
│ AWS Error: failed to refresh cached credentials, no EC2 IMDS role found, operation error ec2imds: GetMetadata, failed to get API token, operation error ec2imds: getToken, http response error StatusCode: 400, request to EC2 IMDS failed
│ 
│ 
│   with provider["registry.terraform.io/hashicorp/aws"],
│   on <empty> line 0:
│   (source code not available)
│ 
╵

Additional Context

  • The issue seems to only apply to sensitive variables, as non-sensitive variables seem unchanged in this scenario.
  • Both env and terraform variable types are affected.
  • I cannot replicate the issue when making the same description change via the TFE UI or API. Only through the tfe provider.
@philbal611 philbal611 added the bug label Mar 31, 2023
@Uk1288
Copy link
Contributor

Uk1288 commented Mar 31, 2023

Thank you for submitting this issue! We'll take a look and get back to you about this. 

@nfagerlund
Copy link
Member

I investigated this, and reproduced the problem pretty quickly. Unfortunately, this is one of a family of related bugs where the terraform-plugin-sdk/v2 offers no way to distinguish between "absent" and "zero," and bad things consequently happen.

Given that we've hit a few of these by now, we've decided to do a spike on adopting the new terraform-plugin-framework, in the hope that its improved capabilities for telling apart different kinds of nothing will actually allow us to solve problems like this. Stay tuned!

@ohmer
Copy link

ohmer commented Apr 24, 2023

I had the same issue but triggered differently, via import. Not a big issue for me if this does not get fixed rapidly
Maybe a note in the resource documentation page would help users who do not always think about checking GitHub issues.

nfagerlund added a commit that referenced this issue May 4, 2023
This commit adds a complete re-implementation of tfe_variable. It keeps most
existing behavior, but fixes a notable bug (#839) where we were unable to
respect `ignore_changes` for sensitive variable values. This was impossible to
address in the v2 SDK, but is incredibly straightforward with the new framework.

Although the effort required for this rewrite wouldn't necessarily be justified
for a single bug fix, it:

- Helps validate our hypothesis that the framework can indeed address the
recurring scourge of "bugs not fixable due to inability to distinguish zero from
absent".

- Gets the framework muxed in and ready to rock and roll, so we can immediately
start implementing new resource types in it.

- Establishes some initial patterns, practices, and examples for using the new
framework within this particular provider.

Anyway, here's some interesting sidenotes about this rewrite:

- The migration docs for the new framework were invaluable:
https://developer.hashicorp.com/terraform/plugin/framework/migrating

- Likewise the working code for the "HashiCups" teaching provider:
https://github.com/hashicorp/terraform-provider-hashicups-pf

- Note that I omitted some logic in most of the existing CRUD methods that
fetched the workspace prior to trying to manage the variable. This logic dated
from the time when the workspace_id argument used the `org/ws_name` format
instead of an external ID; it's no longer necessary now that we have a usable ID
right off the bat.

- Attributes that have a `Default` must also be `Computed`, apparently. I guess
I can see it.

- Most of the go-tfe struct types (update options, etc.) are only constructed by
one CRUD method, but most the methods need to convert a go-tfe value to a model
that we can use to update the state; thus, that's the logic I pulled out into
helper funcs. Conveniently, that's also the point where we need to take some
extra care to make sure we're preserving our last-known information about the
value of a sensitive variable, so we're able to keep that logic centralized too.
@nfagerlund
Copy link
Member

Fix for this is in, and slated for what'll probably be v0.45.0.

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