From 321fd4145d567654d14e5930be87d8fb16bc0b40 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 28 Oct 2019 09:33:26 -0400 Subject: [PATCH 1/2] vault: Support new role field `token_role` Vault 1.2.0 deprecated `period` field in favor of `token_period` in auth role: > * Token store roles use new, common token fields for the values > that overlap with other auth backends. `period`, `explicit_max_ttl`, and > `bound_cidrs` will continue to work, with priority being given to the > `token_` prefixed versions of those parameters. They will also be returned > when doing a read on the role if they were used to provide values initially; > however, in Vault 1.4 if `period` or `explicit_max_ttl` is zero they will no > longer be returned. (`explicit_max_ttl` was already not returned if empty.) https://github.com/hashicorp/vault/blob/master/CHANGELOG.md#120-july-30th-2019 --- nomad/vault.go | 9 ++-- nomad/vault_test.go | 102 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 100 insertions(+), 11 deletions(-) diff --git a/nomad/vault.go b/nomad/vault.go index 44a61973e094..a55525db7ba1 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -10,11 +10,11 @@ import ( "sync/atomic" "time" - "gopkg.in/tomb.v2" + tomb "gopkg.in/tomb.v2" - "github.com/armon/go-metrics" + metrics "github.com/armon/go-metrics" log "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-multierror" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/structs/config" vapi "github.com/hashicorp/vault/api" @@ -883,6 +883,7 @@ func (v *vaultClient) validateRole(role string) error { ExplicitMaxTtl int `mapstructure:"explicit_max_ttl"` Orphan bool Period int + TokenPeriod int `mapstructure:"token_period"` Renewable bool } if err := mapstructure.WeakDecode(rsecret.Data, &data); err != nil { @@ -899,7 +900,7 @@ func (v *vaultClient) validateRole(role string) error { multierror.Append(&mErr, fmt.Errorf("Role can not use an explicit max ttl. Token must be periodic.")) } - if data.Period == 0 { + if data.Period == 0 && data.TokenPeriod == 0 { multierror.Append(&mErr, fmt.Errorf("Role must have a non-zero period to make tokens periodic.")) } diff --git a/nomad/vault_test.go b/nomad/vault_test.go index 0664ac26b8ae..1df1a95a352c 100644 --- a/nomad/vault_test.go +++ b/nomad/vault_test.go @@ -305,9 +305,8 @@ func TestVaultClient_ValidateRole(t *testing.T) { logger := testlog.HCLogger(t) v.Config.ConnectionRetryIntv = 100 * time.Millisecond client, err := NewVaultClient(v.Config, logger, nil) - if err != nil { - t.Fatalf("failed to build vault client: %v", err) - } + require.NoError(t, err) + defer client.Stop() // Wait for an error @@ -325,13 +324,102 @@ func TestVaultClient_ValidateRole(t *testing.T) { return true, nil }, func(err error) { - t.Fatalf("bad: %v", err) + require.NoError(t, err) }) - errStr := connErr.Error() - if !strings.Contains(errStr, "explicit max ttl") { - t.Fatalf("Expect explicit max ttl error") + require.Contains(t, connErr.Error(), "explicit max ttl") +} + +// TestVaultClient_ValidateRole_Success asserts that a valid token role +// gets marked as valid +func TestVaultClient_ValidateRole_Success(t *testing.T) { + t.Parallel() + v := testutil.NewTestVault(t) + defer v.Stop() + + // Set the configs token in a new test role + vaultPolicies := map[string]string{ + "nomad-role-create": nomadRoleCreatePolicy, + "nomad-role-management": nomadRoleManagementPolicy, } + data := map[string]interface{}{ + "allowed_policies": "default,root", + "orphan": true, + "renewable": true, + "token_period": 1000, + } + v.Config.Token = testVaultRoleAndToken(v, t, vaultPolicies, data, nil) + + logger := testlog.HCLogger(t) + v.Config.ConnectionRetryIntv = 100 * time.Millisecond + client, err := NewVaultClient(v.Config, logger, nil) + require.NoError(t, err) + + defer client.Stop() + + // Wait for an error + var conn bool + var connErr error + testutil.WaitForResult(func() (bool, error) { + conn, connErr = client.ConnectionEstablished() + if !conn { + return false, fmt.Errorf("Should connect") + } + + if connErr != nil { + return false, connErr + } + + return true, nil + }, func(err error) { + require.NoError(t, err) + }) +} + +// TestVaultClient_ValidateRole_Deprecated_Success asserts that a valid token +// role gets marked as valid, even if it uses deprecated field, period +func TestVaultClient_ValidateRole_Deprecated_Success(t *testing.T) { + t.Parallel() + v := testutil.NewTestVault(t) + defer v.Stop() + + // Set the configs token in a new test role + vaultPolicies := map[string]string{ + "nomad-role-create": nomadRoleCreatePolicy, + "nomad-role-management": nomadRoleManagementPolicy, + } + data := map[string]interface{}{ + "allowed_policies": "default,root", + "orphan": true, + "renewable": true, + "period": 1000, + } + v.Config.Token = testVaultRoleAndToken(v, t, vaultPolicies, data, nil) + + logger := testlog.HCLogger(t) + v.Config.ConnectionRetryIntv = 100 * time.Millisecond + client, err := NewVaultClient(v.Config, logger, nil) + require.NoError(t, err) + + defer client.Stop() + + // Wait for an error + var conn bool + var connErr error + testutil.WaitForResult(func() (bool, error) { + conn, connErr = client.ConnectionEstablished() + if !conn { + return false, fmt.Errorf("Should connect") + } + + if connErr != nil { + return false, connErr + } + + return true, nil + }, func(err error) { + require.NoError(t, err) + }) } func TestVaultClient_ValidateRole_NonExistant(t *testing.T) { From 008ae5bd7c3b196420d4250f6695923179f2062b Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 28 Oct 2019 09:37:06 -0400 Subject: [PATCH 2/2] Test with Vault latest, 1.2.3 To ensure we test with latest with latest configuration. --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 6abd1292cdc2..63bc14e6fa70 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -298,7 +298,7 @@ commands: parameters: version: type: string - default: 1.0.0 + default: 1.2.3 steps: - run: name: Install Vault << parameters.version >>