From 2e5998670c0dde278b05ae334c25eb3a27086a98 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 15 Mar 2018 15:32:08 -0700 Subject: [PATCH 1/2] Allow and recommend Orphaned Vault tokens This PR removes enforcement that the Vault token role disallows orphaned tokens and recommends orphaned tokens to simplify the bootstrapping/upgrading of Nomad clusters. The requirement that Nomad's Vault token never expire and be shared by all instances of Nomad servers is not operationally friendly. --- nomad/vault.go | 11 +++------- nomad/vault_test.go | 7 ++---- .../source/data/vault/nomad-cluster-role.json | 2 +- .../docs/vault-integration/index.html.md | 22 +++++++++++++++---- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/nomad/vault.go b/nomad/vault.go index 9a07a6365c4..2a853eefcc2 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -625,10 +625,9 @@ func (v *vaultClient) parseSelfToken() error { // a) Must have read capability for "auth/token/roles/" // c) Role must: - // 1) Not allow orphans - // 2) Must allow tokens to be renewed - // 3) Must not have an explicit max TTL - // 4) Must have non-zero period + // 1) Must allow tokens to be renewed + // 2) Must not have an explicit max TTL + // 3) Must have non-zero period // 5) If not configured against a role, the token must be root var mErr multierror.Error @@ -793,10 +792,6 @@ func (v *vaultClient) validateRole(role string) error { // Validate the role is acceptable var mErr multierror.Error - if data.Orphan { - multierror.Append(&mErr, fmt.Errorf("Role must not allow orphans")) - } - if !data.Renewable { multierror.Append(&mErr, fmt.Errorf("Role must allow tokens to be renewed")) } diff --git a/nomad/vault_test.go b/nomad/vault_test.go index 58f5867f8ae..687395a0dc2 100644 --- a/nomad/vault_test.go +++ b/nomad/vault_test.go @@ -274,9 +274,6 @@ func TestVaultClient_ValidateRole(t *testing.T) { }) errStr := connErr.Error() - if !strings.Contains(errStr, "not allow orphans") { - t.Fatalf("Expect orphan error") - } if !strings.Contains(errStr, "explicit max ttl") { t.Fatalf("Expect explicit max ttl error") } @@ -318,7 +315,7 @@ func TestVaultClient_ValidateRole_NonExistant(t *testing.T) { errStr := connErr.Error() if !strings.Contains(errStr, "does not exist") { - t.Fatalf("Expect orphan error") + t.Fatalf("Expect does not exist error") } } @@ -366,7 +363,7 @@ func TestVaultClient_ValidateToken(t *testing.T) { errStr := connErr.Error() if !strings.Contains(errStr, vaultTokenRevokePath) { - t.Fatalf("Expect orphan error") + t.Fatalf("Expect revoke error") } if !strings.Contains(errStr, fmt.Sprintf(vaultRoleLookupPath, "test")) { t.Fatalf("Expect explicit max ttl error") diff --git a/website/source/data/vault/nomad-cluster-role.json b/website/source/data/vault/nomad-cluster-role.json index 37728363f21..dc2621524c4 100644 --- a/website/source/data/vault/nomad-cluster-role.json +++ b/website/source/data/vault/nomad-cluster-role.json @@ -2,7 +2,7 @@ "disallowed_policies": "nomad-server", "explicit_max_ttl": 0, "name": "nomad-cluster", - "orphan": false, + "orphan": true, "period": 259200, "renewable": true } diff --git a/website/source/docs/vault-integration/index.html.md b/website/source/docs/vault-integration/index.html.md index bdf64900c48..a8e5d9ad875 100644 --- a/website/source/docs/vault-integration/index.html.md +++ b/website/source/docs/vault-integration/index.html.md @@ -138,7 +138,7 @@ An example token role definition is given below: "disallowed_policies": "nomad-server", "explicit_max_ttl": 0, "name": "nomad-cluster", - "orphan": false, + "orphan": true, "period": 259200, "renewable": true } @@ -176,9 +176,23 @@ documentation for all possible fields and more complete documentation. `nomad-cluster`. If a different name is chosen, replace the token role in the above policy. -* `orphan` - Specifies whether tokens created against this token role will be - orphaned and have no parents. **Must be set to `false`**. This ensures that the - token can be revoked when the task is no longer needed or a node dies. +* `orphan` - Specifies whether tokens created against this token role will be + orphaned and have no parents. Nomad does not enforce the value of this field + but understanding the implications of each value is important. + + If set to false, all tokens will be revoked when the Vault token given to + Nomad expires. This makes it easy to revoke all tokens generated by Nomad but + forces all Nomad servers to use the same Vault token, even through upgrades of + Nomad servers. If the Vault token that was given to Nomad and used to generate + a tasks token expires, the token used by the task will also be revoked which + is not ideal. + + When set to true, the tokens generated for tasks will not be revoked when + Nomad's token is revoked. However Nomad will still revoke tokens when the + allocation is no longer running, minimizing the lifetime of any task's token. + With orphaned enabled, each Nomad server may also use a unique Vault token, + making bootstrapping and upgrading simpler. As such, **setting `orphan = true` + is the recommended setting**. * `period` - Specifies the length the TTL is extended by each renewal in seconds. It is suggested to set this value on the order of magnitude of 3 days From d101469e3ecc6b6d17dcbcbe4d52372f8d591639 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Fri, 16 Mar 2018 10:59:36 -0700 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1227aec920..31a57ab9322 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ IMPROVEMENTS: * driver/docker: Support mounting root filesystem as read-only [[GH-3802](https://github.com/hashicorp/nomad/issues/3802)] * driver/lxc: Add volumes config to LXC driver [[GH-3687](https://github.com/hashicorp/nomad/issues/3687)] * telemetry: Support DataDog tags [[GH-3839](https://github.com/hashicorp/nomad/issues/3839)] + * vault: Allow Nomad to create orphaned tokens for allocations [[GH-3922](https://github.com/hashicorp/nomad/issues/3922)] BUG FIXES: * core: Fix search endpoint forwarding for multi-region clusters [[GH-3680](https://github.com/hashicorp/nomad/issues/3680)]