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

Allow to not show changes on terraform plan or apply for any object #26625

Closed
maonat opened this issue Oct 19, 2020 · 28 comments
Closed

Allow to not show changes on terraform plan or apply for any object #26625

maonat opened this issue Oct 19, 2020 · 28 comments
Labels
bug waiting for reproduction unable to reproduce issue without further information

Comments

@maonat
Copy link

maonat commented Oct 19, 2020

Current Terraform Version

Terraform v0.14.0-beta1

Use-cases

I'm trying to keep secret the site_config values that are being added to my function app. This is not only related to function app but also for any other object created by terraform.
I want to have this feature enabled because I need to not show what is the string that is being added to the site config which could be a secret that shouldn't be allowed to be seen from the release pipeline I'm executing the terraform apply.

Proposal

It would be enough to have a parameter that would not show the values changed for a specific block or property of a resource

i.e.:

lifecycle {
    ignore_changes = [
      app_settings["_BuildID"]
    ]
    ....
    ....
    ....
    hide_output= [
      app_settings["APPLICATIONINSIGHTS_CONNECTION_STRING"],
      site_credential,
      storage_account_name
    ]
  }

References

@maonat maonat added enhancement new new issue not yet triaged labels Oct 19, 2020
@pkolyvas
Copy link
Contributor

@romeomorcia Thanks for sharing your needs here.

I believe that this PR gets us much closer to the feature you're looking for: #26590

I don't believe, personally, that the lifecycle block is the place for controlling anything other than the lifecycle of a resource - though my opinion should carry the last amount of weight around here.

That said, referring to the PR I've linked to, our goal is to empower provider developers to continue to use the ability to mark attributes returned by the provider as sensitive. While they can already do this today we're expanding that ability such that these values remain sensitive throughout the plan. To that end, if one of the returned attributes is one you'd like a provider to mark as sensitive, I'd encourage you to file an enhancement request with the providers you're using.

I realise that's not the entirety of what you're asking. We don't have any plans to expand sensitivity to entire blocks or nested blocks within resources at this time. We are, as always, open to being convinced there's enough demand around use cases like this to re-prioritize.

@pkolyvas pkolyvas removed the new new issue not yet triaged label Oct 19, 2020
@maonat
Copy link
Author

maonat commented Oct 19, 2020

@pkolyvas This would be an implementation that would allow everyone to hide whatever we needs with the same outcome that #26183 is providing in the output.

I am not sure who is responsible for creating the output that I do see in the console, but I do not think it's responsibility of the provider (in this case azurerm).

Aren't you sure terraform wouldn't be the better to be updated with this feature as it would improve the security for every other providers?

Thanks!

@pkolyvas
Copy link
Contributor

Apologies for being less than clear. I'm not trying to indicate your expressed need wouldn't be valuable. I'm indicating that at this point in time, ie. right now, there are other areas of focus for the terraform team.

I'm also trying to separate the concern/problem from the suggested solution using the lifecycle block, which doesn't feel right to me.

The way I see the problem you're expressing is that you'd like control over which attributes, blocks and values are printed to the console for any specific resource. You'd like to be able to hide anything you deem as sensitive and you'd like a way to do that per-resource.

@maonat
Copy link
Author

maonat commented Oct 19, 2020

@pkolyvas I do agree that it could be declared in another block other than the lifecycle one.

And yes, I would like to have control on every line that a resource could show.
If I want to hide the site config block hidden even though there is a change, I would like to know that in the block there is the configuration password that has been changed in a value but because there is the hide config enabled for that block, i'm not seeing any change in the console.

How can this be moved in a higher priority? I have a workaround for this but it would require me to remove read access to the release pipeline (which I don't want to because it should be visible to everyone in the project).

@alisdair
Copy link
Member

@romeomorcia Unless I misunderstand, this is the sort of use case that we would hope would be covered by sensitive variables (i.e. the PR you linked to). Can you provide a sample configuration and the CLI output that shows what the problem you're encountering is?

Have you tried marking the variables used to define site_config as sensitive? If so, when are they shown in the CLI output?

@maonat
Copy link
Author

maonat commented Oct 19, 2020

@alisdair I would love to use variables but I want to assign my function app settings a value of a secret, therefore I cannot assign a value of a property of a resource to a variable (am I right?).

In either cases, if I try to add a variable that has the sensitive = true, terraform crash. ---> "panic: value is marked, so must be unmarked first"

Code below:

variable "sensitiveString" {
  type = string
  default = "thisIsNothing"
  sensitive = true
}

resource "azurerm_function_app" "afapp" {
  name                      = "someName"
  location                  = azurerm_resource_group.rg.location
  resource_group_name       = azurerm_resource_group.rg.name
  app_service_plan_id       = azurerm_app_service_plan.appsp.id
  storage_connection_string = azurerm_storage_account.stacc.primary_connection_string
  version                   = "~3"

  identity {
    type = "SystemAssigned"
  }

  app_settings = {
    "_ReleaseName" = ""
    "_BuildName"   = ""
    "_BuildID"     = ""
	
	"stringFromKeyvault" = azurerm_key_vault_secret.keyvault_var.value
	
    "newStringSecret" = var.sensitiveString # ---> If this is here, it crashes the execution
  }

  lifecycle {
    ignore_changes = [
      app_settings["_BuildID"],
      app_settings["_BuildName"],
      app_settings["_ReleaseName"]
    ]
  }
}

@alisdair
Copy link
Member

I would love to use variables but I want to assign my function app settings a value of a secret, therefore I cannot assign a value of a property of a resource to a variable (am I right?).

Yes, that's correct. For that case, as Petros pointed out, our upcoming change in #26590 would ensure that any resource attribute which is sensitive would keep its sensitivity when assigned to another resource. The azurerm_key_vault_secret value attribute is marked sensitive so this should work in the above use case.

In either cases, if I try to add a variable that has the sensitive = true, terraform crash. ---> "panic: value is marked, so must be unmarked first"

Aha, this must be a bug! Could you please fill out this template with more details so that we can investigate further? Thank you!

@maonat
Copy link
Author

maonat commented Oct 19, 2020

@alisdair Will fill the form for the bug, but for instance, a value that is not flagged as a "sensitive data" for instance a azurerm_log_analytics_workspace.logs.primary_shared_key, are we getting the same thing happening or not? Because i think that is not treated as a sensitive data.

@alisdair
Copy link
Member

primary_shared_key is also marked sensitive. Hopefully this will be true for all attributes that you are concerned about—providers are generally very good about this.

@maonat
Copy link
Author

maonat commented Oct 20, 2020

Then this means that the improvement has been already merged into the current release or? When we should expect the new beta version to come out?

Thanks!

@pkolyvas
Copy link
Contributor

The PR regarding sensitive attributes will be included in 0.14.0-beta2 which should ship next week. :)

@pkolyvas pkolyvas self-assigned this Oct 20, 2020
@maonat
Copy link
Author

maonat commented Oct 20, 2020

@pkolyvas Lovely! Thanks!

@maonat maonat closed this as completed Oct 20, 2020
@pselle
Copy link
Contributor

pselle commented Oct 26, 2020

@romeomorcia Wanted to give you an update here that due to concerns that implementing the provider-defined sensitive value tracking could unintentionally break community modules (while 0.14 is advertised as no breaking changes) you'll need to use an experimental flag to turn on this behavior once it comes out in beta2:

terraform {
  experiments = [provider_sensitive_attrs]
}

(associated PR #26706)

The reasoning behind this is that when a sensitive input variable is used in a module output (or a different module's sensitive output is used in a further module output) if that output is not defined as sensitive, an error will display:

Error: Output refers to sensitive values

  on mod/main.tf line 6:
   6: output "token" {

Expressions used in outputs can only refer to sensitive values if the
sensitive attribute is true.

Our concern was that since provider-defined sensitive attributes are somewhat "invisible" to configuration authors, this could lead to surprising errors in the 0.14 upgrade process that we'd like to avoid. So we've kept the same behavior, but it will be behind the experiment shown above.

Hope this helps on your journey to adopt Terraform 0.14 as it comes out!

@maonat
Copy link
Author

maonat commented Oct 26, 2020

@pselle hello! Thanks for the info.

Does this mean that I will have to enable this also when terraform v0.14 will be released or i have to add this only to the betas?

Do we know already the day the beta2 will be released? Thanks!

@pselle
Copy link
Contributor

pselle commented Oct 26, 2020

@romeomorcia beta2 is out later this week, and yes, you will need to have this enabled for v0.14.x. Unfortunately, v0.14 was promised without breaking changes, so we'll be shipping this under the experimental flag -- this also allows us to adjust behavior if say, the provider ecosystem has interesting input to share as well during the 0.14 cycle about how we can make this suite of features more useful.

@maonat
Copy link
Author

maonat commented Oct 26, 2020

It's OK as far as you don't make, as soon as it will be out, version 15 crash if i leave the experiment value set to the one you provided me 👍

@maonat
Copy link
Author

maonat commented Oct 29, 2020

@pselle unfortunately the sensitive flag is not working for sensitive values like keyvaults resources values.
image

@maonat
Copy link
Author

maonat commented Oct 29, 2020

Hello, please take a look in the latest comment of hashicorp/terraform-provider-azurerm#9071

@alisdair
Copy link
Member

I can't tell from the screenshot you provided what exactly is going wrong. Can you provide a small sample configuration which demonstrates the issue here?

Can you also confirm that you've enabled the provider_sensitive_attrs experiment?

@maonat
Copy link
Author

maonat commented Oct 29, 2020

@alisdair hello, you can find the config here hashicorp/terraform-provider-azurerm#9071

@alisdair
Copy link
Member

Sorry, but I don't see the config there. The problem you're reporting as I understand it is that an attribute of an azurerm_function_app is incorrectly not marked sensitive. The linked issue has no configuration listed which includes a resource of that type.

Can you share it here, please? Otherwise I cannot reproduce the bug in order to investigate. Thanks!

@maonat
Copy link
Author

maonat commented Oct 29, 2020

@alisdair sorry I just noticed i did not paste the function app code.

here below you can find what i've set in the function app.

Also i wanted to confirm that i used the provider_sensitive_attrs experiment 👍

variable "sensitiveString" {
  type = string
  default = "thisIsNothing"
  sensitive = true
}

resource "azurerm_function_app" "afapp" {
  name                      = "some-function-name"
  location                  = azurerm_resource_group.rg.location
  resource_group_name       = azurerm_resource_group.rg.name
  app_service_plan_id       = azurerm_app_service_plan.appsp.id
  storage_connection_string = azurerm_storage_account.stacc.primary_connection_string
  version                   = "~3"

  identity {
    type = "SystemAssigned"
  }

  app_settings = {
    "APPINSIGHTS_INSTRUMENTATIONKEY"        = azurerm_key_vault_secret.appins_key.value
    "APPLICATIONINSIGHTS_CONNECTION_STRING" = azurerm_key_vault_secret.appins_connstring.value
    "FUNCTION_APP_EDIT_MODE"                = "readOnly"
    "FUNCTIONS_WORKER_RUNTIME"              = "dotnet"

    "_ReleaseName" = ""
    "_BuildName"   = ""
    "_BuildID"     = ""

     "newStringSecret" = var.sensitiveString 
     "additionalNewString" = azurerm_key_vault_secret.test.value # some random keyvault resource value
  }

  lifecycle {
    ignore_changes = [
      app_settings["_BuildID"],
      app_settings["_BuildName"],
      app_settings["_ReleaseName"]
    ]
  }
}

@alisdair
Copy link
Member

I can't reproduce the issue you're describing. I spent some time just now creating a full configuration for the same resource you're reporting the problems with. Here is the initial configuration:

terraform {
  experiments = [provider_sensitive_attrs]
  required_providers {
    azurerm = {
      source  = "hashicorp/azurerm"
      version = "2.34.0"
    }
    random = {
      source  = "hashicorp/random"
      version = "3.0.0"
    }
  }
}

provider "azurerm" {
  features {}
}

variable "foo" {
  type      = string
  default   = "secret"
  sensitive = true
}

variable "location" {
  type    = string
  default = "eastus"
}

data "azurerm_client_config" "current" {}

resource "random_string" "storage_name" {
  length  = 24
  upper   = false
  lower   = true
  number  = true
  special = false
}

resource "azurerm_resource_group" "rg" {
  name     = "rg"
  location = var.location
}

resource "azurerm_app_service_plan" "asp" {
  name                = "plan"
  resource_group_name = azurerm_resource_group.rg.name
  location            = azurerm_resource_group.rg.location
  kind                = "FunctionApp"
  sku {
    tier = "Dynamic"
    size = "Y1"
  }
}

resource "azurerm_storage_account" "sa" {
  name                     = random_string.storage_name.result
  resource_group_name      = azurerm_resource_group.rg.name
  location                 = azurerm_resource_group.rg.location
  account_tier             = "Standard"
  account_replication_type = "LRS"
}

resource "azurerm_key_vault" "kv" {
  name                = "vault-26625"
  resource_group_name = azurerm_resource_group.rg.name
  location            = azurerm_resource_group.rg.location
  sku_name            = "standard"
  tenant_id           = data.azurerm_client_config.current.tenant_id

  access_policy {
    tenant_id          = data.azurerm_client_config.current.tenant_id
    object_id          = data.azurerm_client_config.current.object_id
    secret_permissions = ["delete", "get", "set"]
  }
}

resource "azurerm_key_vault_secret" "bar" {
  name         = "bar"
  value        = "password123"
  key_vault_id = azurerm_key_vault.kv.id
}

resource "azurerm_function_app" "afapp" {
  name                       = "function-26625"
  resource_group_name        = azurerm_resource_group.rg.name
  location                   = azurerm_resource_group.rg.location
  app_service_plan_id        = azurerm_app_service_plan.asp.id
  storage_account_name       = azurerm_storage_account.sa.name
  storage_account_access_key = azurerm_storage_account.sa.primary_access_key
  version                    = "~3"

  app_settings = {
#    "foo" = var.foo
#    "bar" = azurerm_key_vault_secret.bar.value
  }
}

Note that the app settings are commented for now. I applied this configuration with terraform-0.14.0-beta2 apply, which worked as I expected.

I then uncommented the app settings, and ran terraform-0.14.0-beta2 plan. Each value is correctly marked sensitive in the diff output:

sensitive-attributes

If you can still reproduce the issue using Terraform 0.14.0-beta2 or later and the same version of the Azure provider, please let us know how to do so by changing the above configuration. Thanks!

@alisdair alisdair added waiting for reproduction unable to reproduce issue without further information waiting-response An issue/pull request is waiting for a response from the community labels Oct 30, 2020
@maonat
Copy link
Author

maonat commented Oct 30, 2020

Found the bug! @alisdair
Looks like that the issue appears only when you use modules!

In fact I've copied your solution and had the same result as you did but when i created a new solution using a module named "test", the keyvault value was not hidden!

Here the code:

variable "foo" {
  type      = string
  default   = "secret"
  sensitive = true
}

variable "location" {
  type    = string
  default = "westeurope"
}

data "azurerm_client_config" "current" {}

resource "random_string" "storage_name" {
  length  = 24
  upper   = false
  lower   = true
  number  = true
  special = false
}

resource "azurerm_resource_group" "rg" {
  name     = "new-sandbox"
  location = var.location
}

resource "azurerm_app_service_plan" "asp" {
  name                = "plan"
  resource_group_name = azurerm_resource_group.rg.name
  location            = azurerm_resource_group.rg.location
  kind                = "FunctionApp"
  sku {
    tier = "Dynamic"
    size = "Y1"
  }
}

resource "azurerm_storage_account" "sa" {
  name                     = random_string.storage_name.result
  resource_group_name      = azurerm_resource_group.rg.name
  location                 = azurerm_resource_group.rg.location
  account_tier             = "Standard"
  account_replication_type = "LRS"
}

resource "azurerm_key_vault" "kv" {
  name                = "vault-26625-somethingnew"
  resource_group_name = azurerm_resource_group.rg.name
  location            = azurerm_resource_group.rg.location
  sku_name            = "standard"
  tenant_id           = data.azurerm_client_config.current.tenant_id

  access_policy {
    tenant_id          = data.azurerm_client_config.current.tenant_id
    object_id          = data.azurerm_client_config.current.object_id
    secret_permissions = ["delete", "get", "set"]
  }
}

resource "azurerm_key_vault_secret" "bar" {
  name         = "bar"
  value        = "password123"
  key_vault_id = azurerm_key_vault.kv.id
}

resource "azurerm_function_app" "afapp" {
  name                       = "function-26625"
  resource_group_name        = azurerm_resource_group.rg.name
  location                   = azurerm_resource_group.rg.location
  app_service_plan_id        = azurerm_app_service_plan.asp.id
  storage_account_name       = azurerm_storage_account.sa.name
  storage_account_access_key = azurerm_storage_account.sa.primary_access_key
  version                    = "~3"

  app_settings = {
   "foo" = var.foo
   "bar" = azurerm_key_vault_secret.bar.value
  }
}
terraform {
  experiments = [provider_sensitive_attrs]
  required_providers {
    azurerm = {
      source  = "hashicorp/azurerm"
      version = "2.34.0"
    }
    random = {
      source  = "hashicorp/random"
      version = "3.0.0"
    }
  }
}

provider "azurerm" {
  features {}
  subscription_id = var.subscription_id
}

module "test"{
    source    = "../modules/test" 
}

Output

image

@ghost ghost removed the waiting-response An issue/pull request is waiting for a response from the community label Oct 30, 2020
@alisdair
Copy link
Member

alisdair commented Oct 30, 2020

Ah! I believe the issue here is that the experiments setting is only applied on a per-module basis. Can you copy the terraform block to your module? That should fix it, I hope! 🤞

@alisdair alisdair added the waiting-response An issue/pull request is waiting for a response from the community label Oct 30, 2020
@maonat
Copy link
Author

maonat commented Oct 31, 2020

@alisdair indeed, it worked 👍
I was not able to find the information that every time we use modules, we have to re-declare every configuration we pass to the terraform block or to the azurerm block.
Is this written somewhere? If not, shouldn't be this added to the guide?

Thanks!

@ghost ghost removed the waiting-response An issue/pull request is waiting for a response from the community label Oct 31, 2020
@alisdair
Copy link
Member

alisdair commented Nov 2, 2020

Sorry that wasn't super clear! It's noted in the Terraform settings documentation:

In releases where experimental features are available, you can enable them on a per-module basis by setting the experiments argument inside a terraform block

We'll be sure to emphasize the need to specify the use of the experiment on each module in the release announcement.

I'm going to close this now as I believe the original issue is resolved. If you find any other issues while you're testing, please open a new issue. Thanks for your feedback and help with reproducing the problem!

@alisdair alisdair closed this as completed Nov 2, 2020
@ghost
Copy link

ghost commented Dec 3, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug waiting for reproduction unable to reproduce issue without further information
Projects
None yet
Development

No branches or pull requests

4 participants