From b1cf1e7f5172684223e3e0c07b10ba9641ce1f6f Mon Sep 17 00:00:00 2001 From: Ben Ash <32777270+benashz@users.noreply.github.com> Date: Fri, 7 Jun 2024 09:28:05 -0400 Subject: [PATCH] Work around Vault DB static creds TTL rollover bug (#730) * Work around Vault DB TTL rollover bug When syncing database static credentials role configured with scheduled rotation, the TTL is incorrectly rolled over within the same rotation period. Since, VSO relies on the TTL for its sync scheduling, an invalid TTL results in syncing stale credentials. This fix, attempts to detect the TTL rollover bug, and ensure that current rotated creds are properly synced. * Skip scheduled static role tests on vault 1.14 --- controllers/vaultdynamicsecret_controller.go | 247 ++++++++++++--- .../vaultdynamicsecret_controller_test.go | 297 +++++++++++++++++- go.mod | 2 +- internal/vault/client.go | 27 +- .../vaultdynamicsecret/terraform/locals.tf | 21 +- .../vaultdynamicsecret/terraform/main.tf | 8 +- .../vaultdynamicsecret/terraform/outputs.tf | 6 + .../vaultdynamicsecret/terraform/postgres.tf | 55 +++- .../vaultdynamicsecret/terraform/variables.tf | 5 + .../vaultdynamicsecret_integration_test.go | 137 ++++++-- 10 files changed, 712 insertions(+), 93 deletions(-) diff --git a/controllers/vaultdynamicsecret_controller.go b/controllers/vaultdynamicsecret_controller.go index f8504f8d..93b358fc 100644 --- a/controllers/vaultdynamicsecret_controller.go +++ b/controllers/vaultdynamicsecret_controller.go @@ -14,6 +14,7 @@ import ( "os" "time" + "github.com/cenkalti/backoff/v4" "github.com/hashicorp/vault/api" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -341,9 +342,8 @@ func (r *VaultDynamicSecretReconciler) isStaticCreds(meta *secretsv1beta1.VaultS return meta.LastVaultRotation > 0 && (meta.RotationPeriod >= 1 || meta.RotationSchedule != "") } -func (r *VaultDynamicSecretReconciler) syncSecret(ctx context.Context, c vault.ClientBase, - o *secretsv1beta1.VaultDynamicSecret, opt *helpers.SecretTransformationOption, -) (*secretsv1beta1.VaultSecretLease, bool, error) { +// doVault performs a Vault request based on the VaultDynamicSecret's spec. +func (r *VaultDynamicSecretReconciler) doVault(ctx context.Context, c vault.ClientBase, o *secretsv1beta1.VaultDynamicSecret) (vault.Response, error) { path := vault.JoinPath(o.Spec.Mount, o.Spec.Path) var err error var resp vault.Response @@ -357,7 +357,7 @@ func (r *VaultDynamicSecretReconciler) syncSecret(ctx context.Context, c vault.C } method := o.Spec.RequestHTTPMethod - logger := log.FromContext(ctx).WithName("syncSecret") + logger := log.FromContext(ctx).WithName("doVault") if params != nil { if !(method == http.MethodPost || method == http.MethodPut) { logger.V(consts.LogLevelWarning).Info( @@ -377,71 +377,72 @@ func (r *VaultDynamicSecretReconciler) syncSecret(ctx context.Context, c vault.C case http.MethodGet: resp, err = c.Read(ctx, vault.NewReadRequest(path, nil)) default: - return nil, false, fmt.Errorf("unsupported HTTP method %q for sync", method) + return nil, fmt.Errorf("unsupported HTTP method %q for sync", method) } if err != nil { logger.Error(err, "Vault request failed") - return nil, false, err + return nil, err } if resp == nil { - return nil, false, fmt.Errorf("nil response from vault for path %s", path) + return nil, fmt.Errorf("nil response from vault for path %s", path) } - data, err := resp.SecretK8sData(opt) + return resp, nil +} + +func (r *VaultDynamicSecretReconciler) syncSecret(ctx context.Context, c vault.ClientBase, + o *secretsv1beta1.VaultDynamicSecret, opt *helpers.SecretTransformationOption, +) (*secretsv1beta1.VaultSecretLease, bool, error) { + logger := log.FromContext(ctx).WithName("syncSecret") + + resp, err := r.doVault(ctx, c, o) if err != nil { return nil, false, err } + if resp == nil { + return nil, false, errors.New("nil response") + } + + var data map[string][]byte secretLease := r.getVaultSecretLease(resp.Secret()) if !r.isRenewableLease(secretLease, o, true) && o.Spec.AllowStaticCreds { - respData := resp.Data() - if v, ok := respData["last_vault_rotation"]; ok && v != nil { - ts, err := time.Parse(time.RFC3339Nano, v.(string)) - if err == nil { - o.Status.StaticCredsMetaData.LastVaultRotation = ts.Unix() - } + staticCredsMeta, rotatedResponse, err := r.awaitVaultSecretRotation(ctx, o, c, resp) + if err != nil { + return nil, false, err } - if v, ok := respData["rotation_period"]; ok && v != nil { - switch t := v.(type) { - case json.Number: - period, err := t.Int64() - if err == nil { - o.Status.StaticCredsMetaData.RotationPeriod = period - } - } + + resp = rotatedResponse + data, err = resp.SecretK8sData(opt) + if err != nil { + return nil, false, err } - if v, ok := respData["rotation_schedule"]; ok && v != nil { - if schedule, ok := v.(string); ok && v != nil { - o.Status.StaticCredsMetaData.RotationSchedule = schedule - } + + dataToMAC := maps.Clone(data) + for _, k := range []string{"ttl", "rotation_schedule", "rotation_period", "last_vault_rotation", "_raw"} { + delete(dataToMAC, k) } - if v, ok := respData["ttl"]; ok && v != nil { - switch t := v.(type) { - case json.Number: - ttl, err := t.Int64() - if err == nil { - o.Status.StaticCredsMetaData.TTL = ttl - } - } + + macsEqual, messageMAC, err := helpers.HandleSecretHMAC(ctx, r.Client, r.HMACValidator, o, dataToMAC) + if err != nil { + return nil, false, err } - if r.isStaticCreds(&o.Status.StaticCredsMetaData) { - dataToMAC := maps.Clone(data) - for _, k := range []string{"ttl", "rotation_schedule", "rotation_period", "last_vault_rotation", "_raw"} { - delete(dataToMAC, k) - } + logger.V(consts.LogLevelTrace).Info("Secret HMAC", "macsEqual", macsEqual) - macsEqual, messageMAC, err := helpers.HandleSecretHMAC(ctx, r.Client, r.HMACValidator, o, dataToMAC) - if err != nil { - return nil, false, err - } + o.Status.SecretMAC = base64.StdEncoding.EncodeToString(messageMAC) + if macsEqual { + return secretLease, false, nil + } - o.Status.SecretMAC = base64.StdEncoding.EncodeToString(messageMAC) - if macsEqual { - return secretLease, false, nil - } + o.Status.StaticCredsMetaData = *staticCredsMeta + logger.V(consts.LogLevelDebug).Info("Static creds", "status", o.Status) + } else { + data, err = resp.SecretK8sData(opt) + if err != nil { + return nil, false, err } } @@ -453,6 +454,100 @@ func (r *VaultDynamicSecretReconciler) syncSecret(ctx context.Context, c vault.C return secretLease, true, nil } +// awaitVaultSecretRotation waits for the Vault secret to be rotated. This is +// necessary for the case where the Vault secret is a static-creds secret and includes +// a rotation schedule. +func (r *VaultDynamicSecretReconciler) awaitVaultSecretRotation(ctx context.Context, o *secretsv1beta1.VaultDynamicSecret, + c vault.ClientBase, lastResponse vault.Response) (*secretsv1beta1.VaultStaticCredsMetaData, + vault.Response, + error, +) { + logger := log.FromContext(ctx).WithName("awaitVaultSecretRotation") + + resp := lastResponse + respData := lastResponse.Data() + staticCredsMeta, err := vaultStaticCredsMetaDataFromData(respData) + if err != nil { + return nil, nil, err + } + + // if we are not handling static creds or the rotation schedule is not set, then + // we can return early. + if !r.isStaticCreds(staticCredsMeta) || staticCredsMeta.RotationSchedule == "" { + return staticCredsMeta, resp, nil + } + + lastSyncStaticCredsMeta := o.Status.StaticCredsMetaData.DeepCopy() + inLastSyncRotation := lastSyncStaticCredsMeta.LastVaultRotation == staticCredsMeta.LastVaultRotation + switch { + case !inLastSyncRotation: + // return early, not in the last rotation + return staticCredsMeta, resp, nil + case lastSyncStaticCredsMeta.RotationSchedule == "": + // return early, rotation schedule was not set in the last sync + return staticCredsMeta, resp, nil + case lastSyncStaticCredsMeta.RotationSchedule != staticCredsMeta.RotationSchedule: + // return early, rotation schedule has changed + return staticCredsMeta, resp, nil + } + + logger = logger.WithValues( + "staticCredsMeta", staticCredsMeta, + "lastSyncStaticCredsMeta", lastSyncStaticCredsMeta, + "ttl", staticCredsMeta.TTL, + "inLastSyncRotation", inLastSyncRotation, + ) + + bo := backoff.NewExponentialBackOff( + // the minimum rotation period is 5s, so it should be safe to double that. + // Ideally we could use the rotation's TTL value here, but that value is not + // considered to be reliable to the TTL roll-over bug that might exist in the database + // secrets engine. + backoff.WithMaxElapsedTime(time.Second*10), + backoff.WithMaxInterval(time.Second*2)) + if err := backoff.Retry( + func() error { + resp, err = r.doVault(ctx, c, o) + if err != nil { + return err + } + + newStaticCredsMeta, err := vaultStaticCredsMetaDataFromData(resp.Data()) + if err != nil { + return err + } + + var retryError error + if newStaticCredsMeta.LastVaultRotation == staticCredsMeta.LastVaultRotation { + // in the case where we are in the rotation period, we need to wait for the next + // rotation if it is less than 2s away or if the ttl has increased wrt. to the + // last rotation. An increase in ttl indicates that secrets engine has the TTL + // rollover bug, so we need to wait for the next rotation in order to get the + // correct/true TTL value. + if newStaticCredsMeta.TTL <= 2 { + retryError = errors.New("near rotation, ttl<=2") + } else if newStaticCredsMeta.TTL >= lastSyncStaticCredsMeta.TTL { + retryError = errors.New("not rotated, handling ttl rollover bug") + } + } + + logger.V(consts.LogLevelDebug).Info("Stale static creds backoff", + "newStaticCredsMeta", newStaticCredsMeta, + "retryError", retryError, + ) + if retryError != nil { + return retryError + } + + staticCredsMeta = newStaticCredsMeta + return nil + }, bo); err != nil { + return nil, nil, err + } + + return staticCredsMeta, resp, nil +} + func (r *VaultDynamicSecretReconciler) updateStatus(ctx context.Context, o *secretsv1beta1.VaultDynamicSecret) error { if r.runtimePodUID != "" { o.Status.LastRuntimePodUID = r.runtimePodUID @@ -635,8 +730,12 @@ func (r *VaultDynamicSecretReconciler) computePostSyncHorizon(ctx context.Contex ) } else { if d > 0 { - // give Vault an extra .5 seconds to perform the rotation - horizon = d + 500*time.Millisecond + horizon = d + if staticCredsMeta.RotationPeriod > 0 { + // give Vault an extra .5 seconds to perform the rotation if the case of a + // non-scheduled rotation. + horizon = d + 500*time.Millisecond + } } else { horizon = time.Second * 1 } @@ -772,3 +871,53 @@ func computeRelativeHorizonWithJitter(o *secretsv1beta1.VaultDynamicSecret, minH } return horizon, inWindow } + +func vaultStaticCredsMetaDataFromData(data map[string]any) (*secretsv1beta1.VaultStaticCredsMetaData, error) { + var ret secretsv1beta1.VaultStaticCredsMetaData + if v, ok := data["last_vault_rotation"]; ok && v != nil { + ts, err := time.Parse(time.RFC3339Nano, v.(string)) + if err != nil { + return nil, fmt.Errorf("invalid last_vault_rotation %w", err) + } + + ret.LastVaultRotation = ts.Unix() + } + + if v, ok := data["rotation_period"]; ok && v != nil { + switch t := v.(type) { + case json.Number: + period, err := t.Int64() + if err != nil { + return nil, err + } + ret.RotationPeriod = period + case int: + ret.RotationPeriod = int64(t) + default: + return nil, errors.New("invalid rotation_period") + } + } + + if v, ok := data["rotation_schedule"]; ok && v != nil { + if schedule, ok := v.(string); ok { + ret.RotationSchedule = schedule + } + } + + if v, ok := data["ttl"]; ok && v != nil { + switch t := v.(type) { + case json.Number: + ttl, err := t.Int64() + if err != nil { + return nil, err + } + ret.TTL = ttl + case int: + ret.TTL = int64(t) + default: + return nil, errors.New("invalid ttl") + } + } + + return &ret, nil +} diff --git a/controllers/vaultdynamicsecret_controller_test.go b/controllers/vaultdynamicsecret_controller_test.go index b6ececc9..e0ca9fc3 100644 --- a/controllers/vaultdynamicsecret_controller_test.go +++ b/controllers/vaultdynamicsecret_controller_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/hashicorp/vault/api" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -25,6 +26,7 @@ import ( "github.com/hashicorp/vault-secrets-operator/api/v1beta1" secretsv1beta1 "github.com/hashicorp/vault-secrets-operator/api/v1beta1" "github.com/hashicorp/vault-secrets-operator/internal/credentials/provider" + "github.com/hashicorp/vault-secrets-operator/internal/helpers" "github.com/hashicorp/vault-secrets-operator/internal/vault" ) @@ -887,7 +889,7 @@ func TestVaultDynamicSecretReconciler_computePostSyncHorizon(t *testing.T) { }, }, }, - wantMinHorizon: time.Duration(30.5 * float64(time.Second)), + wantMinHorizon: time.Duration(30 * float64(time.Second)), // max jitter 150000000 wantMaxHorizon: time.Duration(30.65 * float64(time.Second)), }, @@ -1137,3 +1139,296 @@ func TestVaultDynamicSecretReconciler_vaultClientCallback(t *testing.T) { }) } } + +func Test_vaultStaticCredsMetaDataFromData(t *testing.T) { + tests := []struct { + name string + data map[string]any + want *secretsv1beta1.VaultStaticCredsMetaData + wantErr assert.ErrorAssertionFunc + }{ + { + name: "with-rotation-schedule", + data: map[string]any{ + "last_vault_rotation": "2024-05-01T23:18:01.330875393Z", + "rotation_schedule": "1 0 * * *", + "ttl": 30, + }, + want: &secretsv1beta1.VaultStaticCredsMetaData{ + LastVaultRotation: 1714605481, + RotationSchedule: "1 0 * * *", + TTL: 30, + }, + wantErr: assert.NoError, + }, + { + name: "with-rotation-period", + data: map[string]any{ + "last_vault_rotation": "2024-05-01T23:18:01.330875393Z", + "rotation_period": 600, + "ttl": 30, + }, + want: &secretsv1beta1.VaultStaticCredsMetaData{ + LastVaultRotation: 1714605481, + RotationPeriod: 600, + TTL: 30, + }, + wantErr: assert.NoError, + }, + { + name: "invalid-last_vault_rotation", + data: map[string]any{ + "last_vault_rotation": "2-024-05-01T23:18:01.330875393Z", + "rotation_schedule": "1 0 * * *", + "ttl": 30, + }, + want: nil, + wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { + return assert.ErrorContains(t, err, "invalid last_vault_rotation", i...) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := vaultStaticCredsMetaDataFromData(tt.data) + if !tt.wantErr(t, err, fmt.Sprintf("vaultStaticCredsMetaDataFromData(%v)", tt.data)) { + return + } + assert.Equalf(t, tt.want, got, "vaultStaticCredsMetaDataFromData(%v)", tt.data) + }) + } +} + +type vaultResponse struct { + data map[string]any +} + +func (s *vaultResponse) Secret() *api.Secret { + return nil +} + +func (s *vaultResponse) Data() map[string]any { + return s.data +} + +func (s *vaultResponse) SecretK8sData(_ *helpers.SecretTransformationOption) (map[string][]byte, error) { + return nil, nil +} + +func TestVaultDynamicSecretReconciler_awaitRotation(t *testing.T) { + ts, err := time.Parse(time.RFC3339Nano, "2024-05-02T19:48:01.328261545Z") + if err != nil { + require.NoError(t, err) + } + + ts1, err := time.Parse(time.RFC3339Nano, "2024-05-02T19:49:01.325799425Z") + if err != nil { + require.NoError(t, err) + } + + ctx := context.Background() + tests := []struct { + name string + o *secretsv1beta1.VaultDynamicSecret + c *vault.MockRecordingVaultClient + initialResponse vault.Response + wantStaticCredsMetaData *secretsv1beta1.VaultStaticCredsMetaData + wantResponse vault.Response + wantRequestCount int + wantErr assert.ErrorAssertionFunc + }{ + { + name: "invalid-static-creds-meta-data", + c: &vault.MockRecordingVaultClient{}, + initialResponse: &vaultResponse{ + data: map[string]any{ + "last_vault_rotation": "2-024-05-02T19:48:01.328261545Z", + "password": "Y3pro72-fl1ndHTFOg9h", + "rotation_schedule": "*/1 * * * *", + "rotation_window": 3600, + "ttl": 59, + "username": "dev-postgres-static-user-scheduled", + }, + }, + wantErr: func(t assert.TestingT, err error, i ...interface{}) bool { + return assert.ErrorContains(t, err, "invalid last_vault_rotation", i...) + }, + }, + { + name: "not-static-creds", + c: &vault.MockRecordingVaultClient{}, + initialResponse: &vaultResponse{ + data: map[string]any{ + "username": "foo", + "password": "bar", + }, + }, + wantErr: assert.NoError, + wantResponse: &vaultResponse{ + data: map[string]any{ + "username": "foo", + "password": "bar", + }, + }, + wantStaticCredsMetaData: &secretsv1beta1.VaultStaticCredsMetaData{}, + wantRequestCount: 0, + }, + { + name: "empty-last-rotation-schedule", + c: &vault.MockRecordingVaultClient{}, + initialResponse: &vaultResponse{ + data: map[string]any{ + "last_vault_rotation": "2024-05-02T19:48:01.328261545Z", + "password": "Y3pro72-fl1ndHTFOg9h", + "rotation_schedule": "*/1 * * * *", + "rotation_window": 3600, + "ttl": 59, + "username": "dev-postgres-static-user-scheduled", + }, + }, + wantErr: assert.NoError, + o: &secretsv1beta1.VaultDynamicSecret{ + Status: secretsv1beta1.VaultDynamicSecretStatus{ + StaticCredsMetaData: secretsv1beta1.VaultStaticCredsMetaData{ + LastVaultRotation: ts.Unix(), + TTL: 55, + }, + }, + }, + wantResponse: &vaultResponse{ + data: map[string]any{ + "last_vault_rotation": "2024-05-02T19:48:01.328261545Z", + "password": "Y3pro72-fl1ndHTFOg9h", + "rotation_schedule": "*/1 * * * *", + "rotation_window": 3600, + "ttl": 59, + "username": "dev-postgres-static-user-scheduled", + }, + }, + wantStaticCredsMetaData: &secretsv1beta1.VaultStaticCredsMetaData{ + LastVaultRotation: ts.Unix(), + RotationSchedule: "*/1 * * * *", + TTL: 59, + }, + wantRequestCount: 0, + }, + { + name: "static-creds-periodic-rotation", + c: &vault.MockRecordingVaultClient{}, + initialResponse: &vaultResponse{ + data: map[string]any{ + "last_vault_rotation": "2024-05-02T19:48:01.328261545Z", + "password": "Y3pro72-fl1ndHTFOg9h", + "rotation_period": 3600, + "ttl": 59, + "username": "dev-postgres-static-user-xxx", + }, + }, + wantErr: assert.NoError, + o: &secretsv1beta1.VaultDynamicSecret{ + Status: secretsv1beta1.VaultDynamicSecretStatus{ + StaticCredsMetaData: secretsv1beta1.VaultStaticCredsMetaData{ + LastVaultRotation: ts.Unix(), + TTL: 55, + }, + }, + }, + wantResponse: &vaultResponse{ + data: map[string]any{ + "last_vault_rotation": "2024-05-02T19:48:01.328261545Z", + "password": "Y3pro72-fl1ndHTFOg9h", + "rotation_period": 3600, + "ttl": 59, + "username": "dev-postgres-static-user-xxx", + }, + }, + wantStaticCredsMetaData: &secretsv1beta1.VaultStaticCredsMetaData{ + LastVaultRotation: ts.Unix(), + RotationPeriod: 3600, + TTL: 59, + }, + wantRequestCount: 0, + }, + { + name: "static-creds-scheduled-initial-ttl-zero", + o: &secretsv1beta1.VaultDynamicSecret{ + Spec: secretsv1beta1.VaultDynamicSecretSpec{ + Mount: "mount", + Path: "static-creds/scheduled", + }, + Status: secretsv1beta1.VaultDynamicSecretStatus{ + StaticCredsMetaData: secretsv1beta1.VaultStaticCredsMetaData{ + LastVaultRotation: ts.Unix(), + RotationSchedule: "*/1 * * * *", + TTL: 55, + }, + }, + }, + initialResponse: &vaultResponse{ + data: map[string]any{ + "last_vault_rotation": "2024-05-02T19:48:01.328261545Z", + "password": "Y3pro72-fl1ndHTFOg9h", + "rotation_schedule": "*/1 * * * *", + "rotation_window": 3600, + "ttl": 0, + "username": "dev-postgres-static-user-scheduled", + }, + }, + wantErr: assert.NoError, + wantStaticCredsMetaData: &secretsv1beta1.VaultStaticCredsMetaData{ + LastVaultRotation: ts1.Unix(), + RotationSchedule: "*/1 * * * *", + TTL: 58, + }, + wantResponse: &vaultResponse{ + data: map[string]any{ + "last_vault_rotation": "2024-05-02T19:49:01.325799425Z", + "password": "qSGA-u8f1-H6WYkII4Yn", + "rotation_schedule": "*/1 * * * *", + "rotation_window": 3600, + "ttl": 58, + "username": "dev-postgres-static-user-scheduled", + }, + }, + c: &vault.MockRecordingVaultClient{ + ReadResponses: map[string][]vault.Response{ + "mount/static-creds/scheduled": { + &vaultResponse{ + data: map[string]any{ + "last_vault_rotation": "2024-05-02T19:48:01.328261545Z", + "password": "Y3pro72-fl1ndHTFOg9h", + "rotation_schedule": "*/1 * * * *", + "rotation_window": 3600, + "ttl": 59, + "username": "dev-postgres-static-user-scheduled", + }, + }, + &vaultResponse{ + data: map[string]any{ + "last_vault_rotation": "2024-05-02T19:49:01.325799425Z", + "password": "qSGA-u8f1-H6WYkII4Yn", + "rotation_schedule": "*/1 * * * *", + "rotation_window": 3600, + "ttl": 58, + "username": "dev-postgres-static-user-scheduled", + }, + }, + }, + }, + }, + wantRequestCount: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &VaultDynamicSecretReconciler{} + got, got1, err := r.awaitVaultSecretRotation(ctx, tt.o, tt.c, tt.initialResponse) + if !tt.wantErr(t, err, fmt.Sprintf("awaitVaultSecretRotation(%v, %v, %v, %v)", ctx, tt.o, tt.c, tt.initialResponse)) { + return + } + assert.Equalf(t, tt.wantStaticCredsMetaData, got, "awaitVaultSecretRotation(%v, %v, %v, %v)", ctx, tt.o, tt.c, tt.initialResponse) + assert.Equalf(t, tt.wantResponse, got1, "awaitVaultSecretRotation(%v, %v, %v, %v)", ctx, tt.o, tt.c, tt.initialResponse) + assert.Equalf(t, tt.wantRequestCount, len(tt.c.Requests), "awaitVaultSecretRotation(%v, %v, %v, %v)", ctx, tt.o, tt.c, tt.initialResponse) + }) + } +} diff --git a/go.mod b/go.mod index 79550154..de48ea42 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/hashicorp/go-hclog v1.6.3 github.com/hashicorp/go-rootcerts v1.0.2 github.com/hashicorp/go-secure-stdlib/awsutil v0.3.0 + github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/hashicorp/hcp-sdk-go v0.65.0 github.com/hashicorp/vault/api v1.13.0 @@ -97,7 +98,6 @@ require ( github.com/hashicorp/go-secure-stdlib/parseutil v0.1.8 // indirect github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect github.com/hashicorp/go-sockaddr v1.0.6 // indirect - github.com/hashicorp/go-version v1.6.0 // indirect github.com/hashicorp/hcl v1.0.1-vault-5 // indirect github.com/hashicorp/hcl/v2 v2.9.1 // indirect github.com/hashicorp/terraform-json v0.13.0 // indirect diff --git a/internal/vault/client.go b/internal/vault/client.go index 95af5f13..cba04578 100644 --- a/internal/vault/client.go +++ b/internal/vault/client.go @@ -808,8 +808,10 @@ type MockRequest struct { var _ ClientBase = (*MockRecordingVaultClient)(nil) type MockRecordingVaultClient struct { - Requests []*MockRequest - Id string + ReadResponses map[string][]Response + WriteResponses map[string][]Response + Requests []*MockRequest + Id string } func (m *MockRecordingVaultClient) ID() string { @@ -826,6 +828,16 @@ func (m *MockRecordingVaultClient) Read(_ context.Context, s ReadRequest) (Respo Params: nil, }) + resps, ok := m.ReadResponses[s.Path()] + if ok { + if len(resps) == 0 { + return nil, fmt.Errorf("no more responses for %s", s.Path()) + } + resp := resps[0] + resps = append(resps[:0], resps[1:]...) + return resp, nil + } + return &defaultResponse{ secret: &api.Secret{ Data: make(map[string]interface{}), @@ -839,6 +851,17 @@ func (m *MockRecordingVaultClient) Write(_ context.Context, s WriteRequest) (Res Path: s.Path(), Params: s.Params(), }) + + resps, ok := m.WriteResponses[s.Path()] + if ok { + if len(resps) == 0 { + return nil, fmt.Errorf("no more responses for %s", s.Path()) + } + resp := resps[0] + resps = append(resps[:0], resps[1:]...) + return resp, nil + } + return &defaultResponse{ secret: &api.Secret{ Data: make(map[string]interface{}), diff --git a/test/integration/vaultdynamicsecret/terraform/locals.tf b/test/integration/vaultdynamicsecret/terraform/locals.tf index ac4752f2..49225803 100644 --- a/test/integration/vaultdynamicsecret/terraform/locals.tf +++ b/test/integration/vaultdynamicsecret/terraform/locals.tf @@ -16,9 +16,20 @@ locals { auth_role_operator = "auth-role-operator" # db locals - postgres_host = "${data.kubernetes_service.postgres.metadata[0].name}.${helm_release.postgres.namespace}.svc.cluster.local:${data.kubernetes_service.postgres.spec[0].port[0].port}" - db_role = "dev-postgres" - db_role_static = "${local.db_role}-static" - db_role_static_user = "${local.db_role_static}-user" - k8s_secret_role = "k8s-secret" + postgres_host = "${data.kubernetes_service.postgres.metadata[0].name}.${helm_release.postgres.namespace}.svc.cluster.local:${data.kubernetes_service.postgres.spec[0].port[0].port}" + db_role = "dev-postgres" + db_role_static = "${local.db_role}-static" + db_role_static_user = "${local.db_role_static}-user" + db_role_static_scheduled = "${local.db_role_static}-scheduled" + db_role_static_user_scheduled = "${local.db_role_static}-user-scheduled" + k8s_secret_role = "k8s-secret" + + dev_token_policies = concat( + [ + vault_policy.revocation.name, + vault_policy.db.name, + vault_policy.k8s_secrets.name, + ], + vault_policy.db-scheduled[*].name, + ) } diff --git a/test/integration/vaultdynamicsecret/terraform/main.tf b/test/integration/vaultdynamicsecret/terraform/main.tf index 42416f3c..c780fccf 100644 --- a/test/integration/vaultdynamicsecret/terraform/main.tf +++ b/test/integration/vaultdynamicsecret/terraform/main.tf @@ -82,12 +82,8 @@ resource "vault_kubernetes_auth_backend_role" "dev" { ] bound_service_account_namespaces = [kubernetes_namespace.dev.metadata[0].name] token_period = var.vault_token_period - token_policies = [ - vault_policy.revocation.name, - vault_policy.db.name, - vault_policy.k8s_secrets.name, - ] - audience = "vault" + token_policies = local.dev_token_policies + audience = "vault" } resource "vault_policy" "revocation" { diff --git a/test/integration/vaultdynamicsecret/terraform/outputs.tf b/test/integration/vaultdynamicsecret/terraform/outputs.tf index 079348eb..e1a59109 100644 --- a/test/integration/vaultdynamicsecret/terraform/outputs.tf +++ b/test/integration/vaultdynamicsecret/terraform/outputs.tf @@ -28,6 +28,12 @@ output "db_role_static" { output "db_role_static_user" { value = local.db_role_static_user } +output "db_role_static_scheduled" { + value = local.db_role_static_scheduled +} +output "db_role_static_user_scheduled" { + value = local.db_role_static_user_scheduled +} output "k8s_secret_path" { value = vault_kubernetes_secret_backend.k8s_secrets.path } diff --git a/test/integration/vaultdynamicsecret/terraform/postgres.tf b/test/integration/vaultdynamicsecret/terraform/postgres.tf index 23cb7e47..61749c63 100644 --- a/test/integration/vaultdynamicsecret/terraform/postgres.tf +++ b/test/integration/vaultdynamicsecret/terraform/postgres.tf @@ -45,6 +45,7 @@ resource "null_resource" "create-pg-user" { namespace = data.kubernetes_pod.postgres.metadata[0].namespace pod = data.kubernetes_pod.postgres.metadata[0].name password = data.kubernetes_secret.postgres.data["postgres-password"] + role = local.db_role_static_user } provisioner "local-exec" { command = < 0 { + expected = tt.expectedStatic + } else if len(tt.expectedStaticScheduled) > 0 { + expected = tt.expectedStaticScheduled + } expectedPresentOnly = append(expectedPresentOnly, helpers.SecretDataKeyRaw, "last_vault_rotation", @@ -606,6 +674,7 @@ func TestVaultDynamicSecret_vaultClientCallback(t *testing.T) { "vault_token_period": 15, // set a high default lease ttl to avoid renewals during the test "vault_db_default_lease_ttl": 600, + "with_static_role_scheduled": withStaticRoleScheduled, }, } if entTests { @@ -985,13 +1054,25 @@ func assertDynamicSecretRotation(t *testing.T, ctx context.Context, client ctrlc if !vdsObj.Spec.AllowStaticCreds { maxTries = uint64(vdsObj.Status.SecretLease.LeaseDuration * 10) } else { - if !assert.Greater(t, vdsObj.Status.StaticCredsMetaData.RotationPeriod, int64(0)) { - return nil + if vdsObj.Status.StaticCredsMetaData.RotationSchedule == "" { + if !assert.Greater(t, vdsObj.Status.StaticCredsMetaData.RotationPeriod, int64(0), + "expected Status.StaticCredsMetaData.RotationPeriod to be set") { + return nil + } + maxTries = uint64(vdsObj.Status.StaticCredsMetaData.RotationPeriod * 4) + } else { + ttl := vdsObj.Status.StaticCredsMetaData.TTL + if ttl <= 2 { + // inflate the ttl since we are near the rotation period and may hit the TTL + // rollover bug + ttl = 10 + } + maxTries = uint64(ttl * 5) } - if !assert.NotEmpty(t, vdsObj.Status.SecretMAC) { + if !assert.NotEmpty(t, vdsObj.Status.SecretMAC, + "expected Status.SecretMAC to be set") { return nil } - maxTries = uint64(vdsObj.Status.StaticCredsMetaData.RotationPeriod * 4) } }