terraform-provider-gitlab icon indicating copy to clipboard operation
terraform-provider-gitlab copied to clipboard

allow for override for variable in gitlab_project_variable to be non-sensitive

Open antonym opened this issue 3 years ago • 5 comments

Currently the variables in gitlab_project_variable are sensitive by default and cannot be overridden. There are some variables that I'd like to view the output for when doing a plan.

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place
Terraform will perform the following actions:
  # gitlab_project_variable.terraform_version-project will be updated in-place
  ~ resource "gitlab_project_variable" "terraform_version-project" {
        id                = "12345:TERRAFORM_VERSION:*"
      ~ value             = (sensitive value)
        # (6 unchanged attributes hidden)
    }
Plan: 0 to add, 1 to change, 0 to destroy.

In this case, we're setting a default TERRAFORM_VERSION and would like to be able to review the variable that is going to be rolled out when doing the plan. Would it be possible to allow a sensitive override to false for these type of scenarios? I would imagine there are a lot other valid scenarios for this feature as well.

We are running Terraform 1.0.11 with version 3.7.0 of terraform-provider-gitlab.

antonym avatar Nov 30 '21 21:11 antonym

In theory this could be supported with a new attribute value_plaintext which would be mutually exclusive (ExactlyOneOf) with value, and would not have the sensitive attribute. However I have not found precedence for a Terraform plugin having this functionality, and it would add considerable tech debt managing the field, which is not a part of the upstream GitLab API, for all the variable resources (group, project, instance -level).

armsnyder avatar Feb 04 '22 01:02 armsnyder

IMHO I wouldn't expect the provider to set the value field as sensitive, unless the value supplied itself is a sensitive value within terraform itself. For example from an input/output declared as sensitive, or another resource returns the value as sensitive, or declared sensitive with the sensitive() function.

As an CI/CD variable is very generic it shouldn't make any assumptions about the sensitivity of the value, as Terraform is already good in determining that. I understand the thoughts about doing so, although.

Even if masked = true is set, it may sound logical to set the value as sensitive too. On first I would agree, but on second thought this should still be left at the discretion of terraform (referring to a sensitive value) and the developer using the resource (using sensitive())

There is also the nonsensitive() function, which is gladly ignored as the state is always treated as sensitive. IIRC internals correctly you can't differ between value = nonsensitive(var.whatever) and value = var.whatever where var.whatever isn't a sensitive value, because in both cases the passed value doesn't have the sensitive flag set.

QuingKhaos avatar Feb 17 '22 10:02 QuingKhaos

@timofurrer @QuingKhaos

I agree that it would be better to allow the sensitivity to be determined by the value being passed to the value attribute; however, the sensitive() function and the behavior of propagating sensitivity to provider attributes is only enabled since Terraform v0.15 - https://www.terraform.io/language/upgrade-guides/0-15#sensitive-output-values.

As an CI/CD variable is very generic it shouldn't make any assumptions about the sensitivity of the value

CI/CD variables are most often used for sensitive variables, since non-sensitive variables can be defined in either the .gitlab-ci.yml or in a template.

If we go forward with this it effectively makes our minimum terraform version v0.15, which is less than a year old at this point. 😬 Due to the potential security risk I'd like to see more community support 👍 on this issue before continuing further.

Also there is an explanation and workaround in the Terraform docs here:

https://www.terraform.io/language/upgrade-guides/0-14#sensitive-values-in-plan-output

For this feature we've taken the approach that it's better to be conservative and obscure potentially-sensitive values at the expense of potentially also obscuring some values that aren't sensitive. Unfortunately this means that if you've written a module in a generic or dynamic way then Terraform may over-generalize which values are sensitive, leading to less helpful plan output.

Due to the security implications of this feature, Terraform offers no direct way to opt out of this change. However, the obscuring of these values is done at the UI layer only and so you can still access the raw values, if needed, by saving your plan to an plan file and then asking Terraform to present it in machine-readable JSON format:

terraform plan -out=tfplan terraform show -json tfplan

armsnyder avatar Feb 21 '22 21:02 armsnyder

@armsnyder thanks for highlighting this!

If we go forward with this it effectively makes our minimum terraform version v0.15, which is less than a year old at this point. 😬 Due to the potential security risk I'd like to see more community support 👍 on this issue before continuing further.

I agree that even though it's not a "breaking" change in a classical sense I still think that given the potential security implications we should effectively plan this for v4 and adjust the terraform requirement.

timofurrer avatar Feb 22 '22 08:02 timofurrer

Hi @armsnyder @timofurrer, thanks for the feedback. I already thought so, that this will be major release change, even if not BC breaking in a classical sense :)

QuingKhaos avatar Feb 22 '22 13:02 QuingKhaos