Merge pull request #6574 from hashicorp/b-gh-6570-vault-role-validation

vault: honor new `token_period` in vault token role

.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 >>

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."))
 	}
 

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) {