From aa05ba6105a4d0d3c563a977d66f3ae4cd9c1e0b Mon Sep 17 00:00:00 2001 From: John-Michael Faircloth Date: Mon, 28 Aug 2023 15:14:38 -0500 Subject: [PATCH] adv ttl mgmt: define schedule interface (#22590) --- builtin/logical/database/backend.go | 24 ++--------- builtin/logical/database/path_roles.go | 7 ++-- builtin/logical/database/path_roles_test.go | 7 ++-- builtin/logical/database/rotation.go | 3 -- builtin/logical/database/rotation_test.go | 38 ++++++++++++++--- builtin/logical/database/schedule/schedule.go | 42 +++++++++++++++++++ 6 files changed, 84 insertions(+), 37 deletions(-) create mode 100644 builtin/logical/database/schedule/schedule.go diff --git a/builtin/logical/database/backend.go b/builtin/logical/database/backend.go index 4c97935ce5..0fc6f615a5 100644 --- a/builtin/logical/database/backend.go +++ b/builtin/logical/database/backend.go @@ -15,6 +15,7 @@ import ( log "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/go-uuid" + "github.com/hashicorp/vault/builtin/logical/database/schedule" "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/syncmap" "github.com/hashicorp/vault/internalshared/configutil" @@ -25,7 +26,6 @@ import ( "github.com/hashicorp/vault/sdk/helper/locksutil" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/queue" - "github.com/robfig/cron/v3" ) const ( @@ -34,7 +34,6 @@ const ( databaseRolePath = "role/" databaseStaticRolePath = "static-role/" minRootCredRollbackAge = 1 * time.Minute - scheduleOptionsDefault = cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow ) type dbPluginInstance struct { @@ -129,6 +128,7 @@ func Backend(conf *logical.BackendConfig) *databaseBackend { b.connections = syncmap.NewSyncMap[string, *dbPluginInstance]() b.queueCtx, b.cancelQueueCtx = context.WithCancel(context.Background()) b.roleLocks = locksutil.CreateLocks() + b.schedule = &schedule.DefaultSchedule{} return &b } @@ -180,25 +180,7 @@ type databaseBackend struct { gaugeCollectionProcess *metricsutil.GaugeCollectionProcess gaugeCollectionProcessStop sync.Once - // scheduleOptionsOverride is used by tests to set a custom ParseOption with seconds enabled - scheduleOptionsOverride cron.ParseOption -} - -func (b *databaseBackend) ParseSchedule(rotationSchedule string) (*cron.SpecSchedule, error) { - scheduleOptions := scheduleOptionsDefault - if b.scheduleOptionsOverride != 0 { - scheduleOptions = b.scheduleOptionsOverride - } - parser := cron.NewParser(scheduleOptions) - schedule, err := parser.Parse(rotationSchedule) - if err != nil { - return nil, err - } - sched, ok := schedule.(*cron.SpecSchedule) - if !ok { - return nil, fmt.Errorf("invalid rotation schedule") - } - return sched, nil + schedule schedule.Scheduler } func (b *databaseBackend) DatabaseConfig(ctx context.Context, s logical.Storage, name string) (*DatabaseConfig, error) { diff --git a/builtin/logical/database/path_roles.go b/builtin/logical/database/path_roles.go index 43417f000a..f68611af42 100644 --- a/builtin/logical/database/path_roles.go +++ b/builtin/logical/database/path_roles.go @@ -591,7 +591,7 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l } if rotationScheduleOk { - parsedSchedule, err := b.ParseSchedule(rotationSchedule) + parsedSchedule, err := b.schedule.Parse(rotationSchedule) if err != nil { return logical.ErrorResponse("could not parse rotation_schedule", "error", err), nil } @@ -600,8 +600,9 @@ func (b *databaseBackend) pathStaticRoleCreateUpdate(ctx context.Context, req *l if rotationWindowOk { rotationWindowSeconds := rotationWindowSecondsRaw.(int) - if rotationWindowSeconds < minRotationWindowSeconds { - return logical.ErrorResponse(fmt.Sprintf("rotation_window must be %d seconds or more", minRotationWindowSeconds)), nil + err := b.schedule.ValidateRotationWindow(rotationWindowSeconds) + if err != nil { + return logical.ErrorResponse("rotation_window is invalid", "error", err), nil } role.StaticAccount.RotationWindow = time.Duration(rotationWindowSeconds) * time.Second } diff --git a/builtin/logical/database/path_roles_test.go b/builtin/logical/database/path_roles_test.go index bb7b5b93a9..87dcc0cdb3 100644 --- a/builtin/logical/database/path_roles_test.go +++ b/builtin/logical/database/path_roles_test.go @@ -7,7 +7,6 @@ import ( "context" "encoding/json" "errors" - "fmt" "strings" "testing" "time" @@ -332,8 +331,8 @@ func TestBackend_StaticRole_Config(t *testing.T) { "rotation_schedule": "* * * * *", "rotation_window": "59s", }, - path: "plugin-role-test", - err: errors.New(fmt.Sprintf("rotation_window must be %d seconds or more", minRotationWindowSeconds)), + path: "plugin-role-test", + errContains: "rotation_window is invalid", }, "disallowed role config": { account: map[string]interface{}{ @@ -1152,7 +1151,7 @@ func TestIsInsideRotationWindow(t *testing.T) { testTime := tc.now if tc.data["rotation_schedule"] != nil && tc.timeModifier != nil { rotationSchedule := tc.data["rotation_schedule"].(string) - schedule, err := b.ParseSchedule(rotationSchedule) + schedule, err := b.schedule.Parse(rotationSchedule) if err != nil { t.Fatalf("could not parse rotation_schedule: %s", err) } diff --git a/builtin/logical/database/rotation.go b/builtin/logical/database/rotation.go index 090678bcba..98778da91c 100644 --- a/builtin/logical/database/rotation.go +++ b/builtin/logical/database/rotation.go @@ -22,9 +22,6 @@ const ( // Default interval to check the queue for items needing rotation defaultQueueTickSeconds = 5 - // Minimum allowed value for rotation_window - minRotationWindowSeconds = 3600 - // Config key to set an alternate interval queueTickIntervalKey = "rotation_queue_tick_interval" diff --git a/builtin/logical/database/rotation_test.go b/builtin/logical/database/rotation_test.go index 4ee3c6d775..6670feb78d 100644 --- a/builtin/logical/database/rotation_test.go +++ b/builtin/logical/database/rotation_test.go @@ -15,6 +15,7 @@ import ( "time" "github.com/Sectorbob/mlab-ns2/gae/ns/digest" + "github.com/hashicorp/vault/builtin/logical/database/schedule" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/testhelpers/mongodb" postgreshelper "github.com/hashicorp/vault/helper/testhelpers/postgresql" @@ -33,9 +34,10 @@ import ( ) const ( - dbUser = "vaultstatictest" - dbUserDefaultPassword = "password" - testScheduleOptionsSeconds = cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow + dbUser = "vaultstatictest" + dbUserDefaultPassword = "password" + testMinRotationWindowSeconds = 10 + testScheduleParseOptions = cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow ) func TestBackend_StaticRole_Rotation_basic(t *testing.T) { @@ -56,7 +58,7 @@ func TestBackend_StaticRole_Rotation_basic(t *testing.T) { } defer b.Cleanup(context.Background()) - b.scheduleOptionsOverride = testScheduleOptionsSeconds + b.schedule = &TestSchedule{} cleanup, connURL := postgreshelper.PrepareTestContainer(t, "") defer cleanup() @@ -1297,7 +1299,7 @@ func TestStoredWALsCorrectlyProcessed(t *testing.T) { t.Fatal(err) } b.credRotationQueue = queue.New() - b.scheduleOptionsOverride = testScheduleOptionsSeconds + b.schedule = &TestSchedule{} configureDBMount(t, config.StorageView) createRoleWithData(t, b, config.StorageView, mockDB, tc.wal.RoleName, tc.data) role, err := b.StaticRole(ctx, config.StorageView, "hashicorp") @@ -1458,7 +1460,7 @@ func getBackend(t *testing.T) (*databaseBackend, logical.Storage, *mockNewDataba if err := b.Setup(context.Background(), config); err != nil { t.Fatal(err) } - b.scheduleOptionsOverride = testScheduleOptionsSeconds + b.schedule = &TestSchedule{} b.credRotationQueue = queue.New() b.populateQueue(context.Background(), config.StorageView) @@ -1548,3 +1550,27 @@ func assertPriorityUnchanged(t *testing.T, priority int64, nextRotationTime time t.Fatalf("expected next rotation at %s, but got %s", nextRotationTime, time.Unix(priority, 0).String()) } } + +var _ schedule.Scheduler = &TestSchedule{} + +type TestSchedule struct{} + +func (d *TestSchedule) Parse(rotationSchedule string) (*cron.SpecSchedule, error) { + parser := cron.NewParser(testScheduleParseOptions) + schedule, err := parser.Parse(rotationSchedule) + if err != nil { + return nil, err + } + sched, ok := schedule.(*cron.SpecSchedule) + if !ok { + return nil, fmt.Errorf("invalid rotation schedule") + } + return sched, nil +} + +func (d *TestSchedule) ValidateRotationWindow(s int) error { + if s < testMinRotationWindowSeconds { + return fmt.Errorf("rotation_window must be %d seconds or more", testMinRotationWindowSeconds) + } + return nil +} diff --git a/builtin/logical/database/schedule/schedule.go b/builtin/logical/database/schedule/schedule.go new file mode 100644 index 0000000000..7306c5c1a1 --- /dev/null +++ b/builtin/logical/database/schedule/schedule.go @@ -0,0 +1,42 @@ +package schedule + +import ( + "fmt" + + "github.com/robfig/cron/v3" +) + +const ( + // Minimum allowed value for rotation_window + minRotationWindowSeconds = 3600 + parseOptions = cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow +) + +type Scheduler interface { + Parse(string) (*cron.SpecSchedule, error) + ValidateRotationWindow(int) error +} + +var _ Scheduler = &DefaultSchedule{} + +type DefaultSchedule struct{} + +func (d *DefaultSchedule) Parse(rotationSchedule string) (*cron.SpecSchedule, error) { + parser := cron.NewParser(parseOptions) + schedule, err := parser.Parse(rotationSchedule) + if err != nil { + return nil, err + } + sched, ok := schedule.(*cron.SpecSchedule) + if !ok { + return nil, fmt.Errorf("invalid rotation schedule") + } + return sched, nil +} + +func (d *DefaultSchedule) ValidateRotationWindow(s int) error { + if s < minRotationWindowSeconds { + return fmt.Errorf("rotation_window must be %d seconds or more", minRotationWindowSeconds) + } + return nil +}