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

implement function writeToFile in template stanza #12095

Closed
nahsi opened this issue Feb 19, 2022 · 12 comments · Fixed by #12312
Closed

implement function writeToFile in template stanza #12095

nahsi opened this issue Feb 19, 2022 · 12 comments · Fixed by #12312

Comments

@nahsi
Copy link

nahsi commented Feb 19, 2022

Proposal

Add consul-template writeToFile function.

Use-cases

Managing directories with config files

I have a separate repository with dashboards and alerts (let's look at dashboards only):

> tree
.
├── dashboards
│   └── HashiStack
│       ├── consul.json
│       └── nomad.json
└── main.tf

2 directories, 3 files

>cat main.tf

resource "consul_keys" "dashboards" {
  dynamic "key" {
    for_each = fileset("${path.module}/dashboards", "**/**.json")
    content {
      path   = "observability/dashboards/${trimsuffix(key.value, ".json")}"
      value  = file("${path.module}/dashboards/${key.value}")
      delete = true
    }
  }
}

When I run terraform apply dashboards appear in Consul:
image

In Grafana I have a following provisioning config:

apiVersion: 1

providers:
- name: "dashboards"
  type: "file"
  updateIntervalSeconds: 10
  allowUiUpdates: true
  options:
    path: "/local/dashboards"
    foldersFromFilesStructure: true

And finally a Nomad template stanza:

      template {
        data = <<-EOH
        {{ range safeTree "observability/dashboards" }}{{ $file := .Key }}
        {{ .Value | writeToFile (printf "/local/dashboards/%s.json" $file) }}
        {{ end }}
        EOH
        destination = "local/dashboards-template"
        change_mode = "noop"
      }

Currently task is unable to start with error: Template failed: (dynamic): parse: template: :2: function "writeToFile" not defined

Dynamic credentials as separate files

Another usecase would be with database credentials. Instead of restarting the task as in:

      template {
        data        = <<-EOH
        {{ with secret "postgres/creds/grafana" }}
        GF_DATABASE_USER='{{ .Data.username }}'
        GF_DATABASE_PASSWORD='{{ .Data.password }}'
        {{- end }}
        EOH
        destination = "secrets/db.env"
        env         = true
        splay       = "3m"
      }

Credentials can be written to a file:

      template {
        data        = <<-EOH
        {{ with secret "postgres/creds/grafana" }}
        {{ .Data.username | writeToFile "/secrets/pg_username" }}
        {{ .Data.password | writeToFile "/secrets/pg_password" }}
        {{- end }}
        EOH
        destination = "secrets/template"
        change_mode = "noop"
      }

And then used in Grafana config:

[database]
type = postgres
host = master.postgres.service.consul:5432
name = grafana
user = $__file{/secrets/pg_username}
password = $__file{/secrets/pg_password}

This way no restart is required as Grafana can reread credentials on signal, if I'm not mistaken.

File owner/permissions

And it will help with files created with wrong user/permissions.
For example now it is not possible to generate tls key pair for postgres when running postgres as non root.
Keys will be owned by root user and postgres will not be able to read them OR using perms = 777 postgres will refuse to start with not secure permissions on TLS keys.
This can be remedied with {{ key "my/key/path" | writeToFile "/my/file/path.txt" "my-user" "my-group" "0644" "append,newline" }}

I hope that make sense.

@nahsi nahsi changed the title function writeToFile is not defined implement function writeToFile in template stanza Feb 19, 2022
@apollo13
Copy link
Contributor

How would that work? templating is generally done on the client side before the resulting json (?) is submitted to the server.

@tgross
Copy link
Member

tgross commented Feb 22, 2022

Well the template block is executed on the Nomad client, not in the CLI. So if I'm reading this correctly, what you're trying to do is to read multiple Consul keys in a single template block but have them write to separate files in the task? In my mind that makes the destination field semantically weird but hey if consul-template supports it there's no reason not to I suppose.

Fortunately this is simply a matter of updating our consul-template dependency. We're currently on 0.25.2 and the writeToFile function was added in 0.27.0. We already have #11472 open with this, so I'll make a note that the PR will close this issue.

@apollo13
Copy link
Contributor

Oh my mistake, I mixed that up with the HCL function stuff.

@apollo13
Copy link
Contributor

@tgross I doubt it is simply fixed by updating consul-template as that would introduce quite some security issues. Nomad often runs as root and having writeToFile "/etc/passwd" would be rather funny :D

@tgross
Copy link
Member

tgross commented Feb 22, 2022

Right, we'd need to sandbox it the same as we do for destination. (Which likely means we'll need to denylist the function as part of the module upgrade until that's done. Will note that in the PR.)

@Xerkus
Copy link

Xerkus commented Feb 22, 2022

I use template to provide ssl certs to ingress traefik. Something like this:

      dynamic "template" {
        for_each = local.vault_certs
        content {
          destination = "${NOMAD_TASK_DIR}/conf/dynamic/cert-${template.value}.toml"
          env = false
          change_mode = "restart"
          splay = "1m"
          data = <<-EOH
          [[tls.certificates]]
            certFile = "/secrets/certs/${template.value}.crt"
            keyFile =  "/secrets/certs/${template.value}.key"
          EOH
        }
      }

      dynamic "template" {
        for_each = local.vault_certs
        content {
          destination = "${NOMAD_SECRETS_DIR}/certs/${template.value}.crt"
          env = false
          change_mode = "restart"
          splay = "1m"
          data = <<-EOH
          {{- with secret "secrets/traefik/certs/${template.value}" -}}
          {{.Data.data.fullchain}}
          {{- end -}}
          EOH
        }
      }

      dynamic "template" {
        for_each = local.vault_certs
        content {
          destination = "${NOMAD_SECRETS_DIR}/certs/${template.value}.key"
          env = false
          change_mode = "restart"
          splay = "1m"
          data = <<-EOH
          {{- with secret "secrets/traefik/certs/${template.value}" -}}
          {{.Data.data.privkey}}
          {{- end -}}
          EOH
        }
      }

writeToFile would make it so easy to provide a dynamic list of certs from a single template.

@chrisvanmeer
Copy link
Contributor

@Xerkus where do you define the local.vault_certs ?
I am trying to implement the same thing within Nomad for Traefik and found this thread because I found the consul-template function writeToFile in an attempt to make the destination of the template more dynamic.

I now have

      template {
        data = <<EOF
{{ range secrets "secret/metadata/ssl-certificates/" }}
{{ with secret (printf "secret/ssl-certificates/%s/certificate" .) }}{{ .Data.data.value }}{{ end }}
{{ end }}
EOF
        destination = "local/ssl/certificates"
      }

But am stuck at the destination part, because I would want it to be "local/ssl/%s.crt".

@Xerkus
Copy link

Xerkus commented Mar 2, 2022

@chrisvanmeer it is still static setup that is using HCL2 locals rendered by nomad cli on job submission.

I created repo with sample setup: https://github.com/Xerkus/traefik-nomad-vault-certbot
It is off-topic for the issue and should be continued at https://discuss.hashicorp.com/

@chrisvanmeer
Copy link
Contributor

Thanks

@mikenomitch
Copy link
Contributor

Hey @schmichael, I assigned you on this one since it overlaps with the SD/CT work you're on. If that's not the case, let me know and unassign yourself!

schmichael added a commit that referenced this issue Mar 16, 2022
Resolves #12095 by WONTFIXing it.

This approach disables `writeToFile` as it allows arbitrary host
filesystem writes and is only a small quality of life improvement over
multiple `template` stanzas.
schmichael added a commit that referenced this issue Mar 16, 2022
Resolves #12095 by WONTFIXing it.

This approach disables `writeToFile` as it allows arbitrary host
filesystem writes and is only a small quality of life improvement over
multiple `template` stanzas.

This approach has the significant downside of leaving people who have
altered their `template.function_denylist` *still vulnerable!* I added
an upgrade note, but we should have implemented the denylist as a
`map[string]bool` so that new funcs could be denied without overriding
custom configurations.
schmichael added a commit that referenced this issue Mar 18, 2022
Resolves #12095 by WONTFIXing it.

This approach disables `writeToFile` as it allows arbitrary host
filesystem writes and is only a small quality of life improvement over
multiple `template` stanzas.

This approach has the significant downside of leaving people who have
altered their `template.function_denylist` *still vulnerable!* I added
an upgrade note, but we should have implemented the denylist as a
`map[string]bool` so that new funcs could be denied without overriding
custom configurations.

This PR also includes a bug fix that broke enabling all consul-template
funcs. We repeatedly failed to differentiate between a nil (unset)
denylist and an empty (allow all) one.
schmichael added a commit that referenced this issue Mar 18, 2022
Resolves #12095 by WONTFIXing it.

This approach disables `writeToFile` as it allows arbitrary host
filesystem writes and is only a small quality of life improvement over
multiple `template` stanzas.

This approach has the significant downside of leaving people who have
altered their `template.function_denylist` *still vulnerable!* I added
an upgrade note, but we should have implemented the denylist as a
`map[string]bool` so that new funcs could be denied without overriding
custom configurations.

This PR also includes a bug fix that broke enabling all consul-template
funcs. We repeatedly failed to differentiate between a nil (unset)
denylist and an empty (allow all) one.
@schmichael
Copy link
Member

schmichael commented Mar 28, 2022

I plan on merging #12312 which will disable writeToFile in Nomad 1.3. If we containerize template (see #12301 for that discussion) we can allow writeToFile, plugin, and never have to worry about template safety again!

For users who trust their jobspec authors and template sources completely, you can enable writeToFile by setting function_denylist in your Nomad agent configs:

client {
  template {
    function_denylist = []
  }
}

I know this isn't ideal, but securing template is very difficult (see #9129 for one of our past failures securing template).

schmichael added a commit that referenced this issue Mar 28, 2022
Resolves #12095 by WONTFIXing it.

This approach disables `writeToFile` as it allows arbitrary host
filesystem writes and is only a small quality of life improvement over
multiple `template` stanzas.

This approach has the significant downside of leaving people who have
altered their `template.function_denylist` *still vulnerable!* I added
an upgrade note, but we should have implemented the denylist as a
`map[string]bool` so that new funcs could be denied without overriding
custom configurations.

This PR also includes a bug fix that broke enabling all consul-template
funcs. We repeatedly failed to differentiate between a nil (unset)
denylist and an empty (allow all) one.
schmichael added a commit that referenced this issue Mar 29, 2022
Resolves #12095 by WONTFIXing it.

This approach disables `writeToFile` as it allows arbitrary host
filesystem writes and is only a small quality of life improvement over
multiple `template` stanzas.

This approach has the significant downside of leaving people who have
altered their `template.function_denylist` *still vulnerable!* I added
an upgrade note, but we should have implemented the denylist as a
`map[string]bool` so that new funcs could be denied without overriding
custom configurations.

This PR also includes a bug fix that broke enabling all consul-template
funcs. We repeatedly failed to differentiate between a nil (unset)
denylist and an empty (allow all) one.
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.