hashicorp / terraform-provider-vault

Terraform Vault provider
https://www.terraform.io/docs/providers/vault/
Mozilla Public License 2.0
457 stars 538 forks source link

plural version of vault_generic_secret? #789

Open dmead opened 4 years ago

dmead commented 4 years ago

Hi there,

I've got a number of secrets that all share a common path in my vault instance

is there a way to ask vault for multiple secrets with one data source or resource? or a way in terraform to dynamically construct a set of secrets?

is there a consensus on this issue? Having 30+ secrets for my service is making things super verbose.

Terraform Version

> terraform -v Terraform v0.12.25

Affected Resource(s)

pdecat commented 4 years ago

Hi @dmead, I believe there are several parts around this issue. Let me try to break this down.

is there a way to ask vault for multiple secrets with one data source

Do you mean something like this?

# NOT IMPLEMENTED (plural generic secrets datasource)
data "vault_generic_secrets" "secrets" {
  path = "mypath/"
}

Note: I came to this issue for this specific use case.

or resource?

Do you mean something like this?

# NOT IMPLEMENTED (plural version of vault_generic_secret)
resource "vault_generic_secrets" secrets {
  path = "mypath/"

  secret {
    name      = "name1"
    data_json = { "key1" = "secret1" }
  }

  secret {
    name      = "name2"
    data_json = { "key2" = "secret2" }
  }
} 

If so not right now. And I don't think it's actually needed as you should be able to use the construct from the answer below.

or a way in terraform to dynamically construct a set of secrets?

Would for_each work for you? E.g.:

resource "vault_generic_secret" "example" {
  for_each = {
    name1 = jsonencode({"key1" = "secret1"})
    name2 = jsonencode({"key2" = "secret2"})
  }

  path = "secret/${each.key}"

  data_json = each.value
}

Corresponding plan:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # vault_generic_secret.example["name1"] will be created
  + resource "vault_generic_secret" "example" {
      + data         = (sensitive value)
      + data_json    = (sensitive value)
      + disable_read = false
      + id           = (known after apply)
      + path         = "secret/name1"
    }

  # vault_generic_secret.example["name2"] will be created
  + resource "vault_generic_secret" "example" {
      + data         = (sensitive value)
      + data_json    = (sensitive value)
      + disable_read = false
      + id           = (known after apply)
      + path         = "secret/name2"
    }

Plan: 2 to add, 0 to change, 0 to destroy.
dmead commented 4 years ago

interesting. thank you. this is very helpful. it's not super clear from the docs that this would work.

pdecat commented 4 years ago

Great! What do you think about renaming this issue to focus on the first use case?

Currently, there's no solution to retrieve multiple secrets from a path without knowing the precise secret name.

Expanded use case:

# NOT IMPLEMENTED (plural generic secrets datasource)
data "vault_generic_secrets" "mypath" {
  path = "mypath/"
}

resource "kubernetes_secret" "test" {
  for_each = data.vault_generic_secrets.mypath.secrets

  metadata {
    name = each.value.name
  }

  data = each.value.data
}
dmead commented 4 years ago

yea cool, done.

I'm not personally a fan of making things plural by adding an s on the end.

it's grammatically correct (for English) but is easy to miss and in my opinion can be a source of errors.

what about introducing a different noun to deal with this?

call it a vault_generic_subtree or something like that, so we can express what we mean and make the difference with vault_generic_secret pretty obvious.

# NOT IMPLEMENTED (plural generic secrets datasource)
data "vault_generic_subtree" "mypath" {
  prefix_path = "mypath/"
}

resource "kubernetes_secret" "test" {
  for_each = data.vault_generic_secrets.mypath.secrets

  metadata {
    name = each.value.name
  }

  data = each.value.data
}

it would need to crawl all the secrets under that path (recursively?) and flatted out the list so it's usable with for_each

does that seem cleaner? I'm pretty new to terraform so i don't know what other expectations this might break.

I'm of two minds about it though. adding this kind of dynamic behavior makes the code more writeable but less readable. It requires prior knowledge about where secrets live vault to debug problems with written out secrets, but would seriously reduce the LOC needed for me to manage the service that takes up so many secrets.

pdecat commented 4 years ago

Using plural on datasources is quite standard on terraform land:

Data Source Names Similar to resource names, data source names should be nouns. The main difference is that in some cases data sources are used to return a list and can in those cases be plural. For example the data source aws_availability_zones in the AWS provider returns a list of availability zones.

see https://www.terraform.io/docs/extend/best-practices/naming.html#data-source-names

As there exists both vault_generic_secret datasource and resource, may Isuggest renaming the issue to:

Plural version of vault_generic_secret datasource

Note: I wouldn't expect that datasource to read secrets recursively, at least not by default.

KrisShannon commented 11 months ago

The vault_generic_secret datasource is basically a generic GET against a provided path.

What is missing would be the same but using the HTTP method LIST

Running a GET on each name returned from the LIST is often not needed and for example in the case of KV2 the LIST endpoint only works on the metadata path so the GETs would return the metadata instead of the secrets.

When a GET on each name is needed it could be done with a for_each on avault_generic_secret data source against the result of the list.

E.g. if the new data source was called vault_generic_list and the result property was called values:

data "vault_generic_list" "secrets" {
  path = "kv2/metadata/path/"
}

data "vault_generic_secret" "secrets" {
  for_each = data.vault_generic_list.secrets.values

  path = "kv2/data/path/${each.key}"
}