secrets/database: advanced TTL management for static roles #22484

fairclothjm · 2023-08-21T21:45:38Z

This PR enables schedule-based auto-rotation of static roles for the Database Secrets engine. This rotation mechanism will be “embedded” in the Database Secrets engine and will not be reusable by other engines. In particular, this approach allows for exclusion windows (e.g. market hours) in which automatic rotations will not occur.

We add a new field rotation_schedule to the create static role endpoint. This field can be set with a cron-style string that will define the schedule on which rotations should occur:

// set rotations to occur on Saturday at 00:00
vault write database/static-roles/test-role \
    db_name=postgresql \
    username="vault" \
    rotation_schedule="0 0 * * SAT"

Additionally, we add a new rotation_window field on the create static role endpoint. It will be optional when rotation_schedule is set and disallowed when rotation_period is set. This field can be set with a time duration string with a minimum of 1 hour. The rotation_window will define the window of time in which rotations are allowed to occur starting from a given rotation_schedule:

// set rotations to occur on Saturday at 00:00, but allow up to Sunday at 23:59
vault write database/static-roles/test-role \
    db_name=postgresql \
    username="vault" \
    rotation_schedule="0 0 * * SAT" \
    rotation_window="24h"

github-actions · 2023-08-21T22:07:27Z

Build Results:
All builds succeeded! ✅

github-actions · 2023-08-21T22:13:23Z

CI Results:
All Go tests succeeded! ✅

fairclothjm · 2023-08-22T19:17:43Z

builtin/logical/database/backend.go

+	// TODO(JM): don't allow seconds in production, this is helpful for
+	// development/testing though
+	parser := cron.NewParser(
+		cron.Second | cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.DowOptional,


* handle mutual exclusion of rotation fields * fix import

* adv ttl mgmt: add rotation_window field * do some rotation_window validation and add unit tests

#22341) * general cleanup and refactor rotation type checks * make NextRotationTime account for the rotation type * add comments

* add unit tests to handle mutual exclusion * revert rotation_test.go and add missing test case to path_roles_test.go

* support manual rotation for schedule based roles * update description and naming

* consider rotation window ensure rotations only occur within a rotation window for schedule-based rotations * use helper method to set priority in rotateCredential * fix bug with priority check * remove test for now * add and remove comments

* update read static-creds endpoint to include correct resp data * return rotation_window if set * update

austingebauer

Looks good to me! Nice job on this @fairclothjm and @MilenaHC! 🚀

austingebauer · 2023-08-23T17:07:25Z

builtin/logical/database/path_roles.go

+	} else if rotationScheduleChanged {
+		next := role.StaticAccount.Schedule.Next(lvr)
+		b.logger.Debug("init priority for Schedule", "lvr", lvr, "next", next)
+		item.Priority = role.StaticAccount.Schedule.Next(lvr).Unix()


nit: any good reason to not use next.Unix()?

austingebauer · 2023-08-23T18:28:40Z

builtin/logical/database/path_roles.go

+	_, rotationPeriodChanged := data.Raw["rotation_period"]
+	_, rotationScheduleChanged := data.Raw["rotation_schedule"]


Curious if there is a difference between these values from data.Raw and rotationPeriodOk / rotationScheduleOk from above?

Nope, good catch!

austingebauer · 2023-08-23T18:30:27Z

builtin/logical/database/path_roles.go

+	rotationSchedule := data.Get("rotation_schedule").(string)
+	rotationScheduleOk := rotationSchedule != ""


nit: I might be missing something but curious if there is a reason GetOk didn't work for rotation_schedule?

I think this line was added a bit later when we realized we needed the boolean. Will update, thanks!

vinay-gopalan

LGTM! Had a couple questions :)

builtin/logical/database/path_roles.go

vinay-gopalan · 2023-08-23T18:17:38Z

builtin/logical/database/backend.go

@@ -127,6 +128,13 @@ func Backend(conf *logical.BackendConfig) *databaseBackend {
 	b.connections = syncmap.NewSyncMap[string, *dbPluginInstance]()
 	b.queueCtx, b.cancelQueueCtx = context.WithCancel(context.Background())
 	b.roleLocks = locksutil.CreateLocks()
+
+	// TODO(JM): don't allow seconds in production, this is helpful for


Are we going to remove supporting seconds altogether or are we going to add some safeguards so they can still be run in our CI tests but users can't add them in production? From the tests set up below, I can definitely see us keeping seconds around just for our testing

Ah, also just saw Milena's PR to only allow seconds in testing, so this comment should be resolved when that PR is merged 😅

fairclothjm · 2023-08-24T21:43:40Z

@austingebauer @vinay-gopalan Thanks for the reviews! I am going to address your comments in a follow up PR.

fairclothjm requested a review from a team as a code owner August 21, 2023 21:45

fairclothjm requested review from a team and removed request for a team August 21, 2023 21:45

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Aug 21, 2023

fairclothjm added this to the 1.15 milestone Aug 21, 2023

fairclothjm commented Aug 22, 2023

View reviewed changes

fairclothjm and others added 18 commits August 22, 2023 12:36

add rotation_schedule field to db backend

2d934c9

add cron schedule field

5511ed4

use priority queue with scheduled rotation types

6b95acf

allow marshalling of cron schedule type

417466c

return warning on use of mutually exclusive fields

4779495

handle mutual exclusion of rotation fields (#22306)

acb5ebb

* handle mutual exclusion of rotation fields * fix import

adv ttl mgmt: add rotation_window field (#22303)

7ceac55

* adv ttl mgmt: add rotation_window field * do some rotation_window validation and add unit tests

adv ttl mgmt: Ensure initialization sets appropriate rotation schedule (

6aedaaf

#22341) * general cleanup and refactor rotation type checks * make NextRotationTime account for the rotation type * add comments

add unit tests to handle mutual exclusion (#22352)

7a542fc

* add unit tests to handle mutual exclusion * revert rotation_test.go and add missing test case to path_roles_test.go

adv ttl mgmt: add tests for init queue (#22376)

c270552

Vault 18908/handle manual rotation (#22389)

f23ff91

* support manual rotation for schedule based roles * update description and naming

add unit tests for manual rotation (#22453)

519eacf

adv ttl mgmt: add tests for rotation_window

38c3d76

adv ttl mgmt: refactor window tests (#22472)

d725e4a

Handle GET static-creds endpoint (#22476)

fdeb705

* update read static-creds endpoint to include correct resp data * return rotation_window if set * update

add changelog

552e804

add unit test for static-creds read endpoint (#22505)

4fa0f54

Zlaticanin force-pushed the VAULT-17232/adv-ttl-mgmt branch from 25968ca to 4fa0f54 Compare August 22, 2023 19:38

austingebauer approved these changes Aug 23, 2023

View reviewed changes

vinay-gopalan approved these changes Aug 23, 2023

View reviewed changes

fairclothjm merged commit 83f3e39 into main Aug 24, 2023

fairclothjm deleted the VAULT-17232/adv-ttl-mgmt branch August 24, 2023 21:45

fairclothjm mentioned this pull request Sep 7, 2023

docs: db schedule-based static role rotations #22863

Merged

hc-github-team-secure-vault-core mentioned this pull request Sep 7, 2023

Backport of docs: db schedule-based static role rotations into release/1.15.x #22878

Merged

This was referenced Sep 18, 2023

secrets/db: add tests for static role config updates #23153

Merged

support for db schedule-based static role rotations hashicorp/vault-secrets-operator#367

Closed

support for db schedule-based static role rotations hashicorp/vault-secrets-operator#369

Merged

hc-github-team-secure-vault-core mentioned this pull request Sep 22, 2023

Backport of secrets/db: add tests for static role config updates into release/1.15.x #23247

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

secrets/database: advanced TTL management for static roles #22484

secrets/database: advanced TTL management for static roles #22484

fairclothjm commented Aug 21, 2023

github-actions bot commented Aug 21, 2023

github-actions bot commented Aug 21, 2023 •

edited

Loading

fairclothjm Aug 22, 2023

austingebauer left a comment •

edited

Loading

austingebauer Aug 23, 2023

austingebauer Aug 23, 2023

fairclothjm Aug 24, 2023

austingebauer Aug 23, 2023

fairclothjm Aug 24, 2023

vinay-gopalan left a comment

vinay-gopalan Aug 23, 2023

vinay-gopalan Aug 23, 2023

fairclothjm commented Aug 24, 2023

	cron.Second \| cron.Minute \| cron.Hour \| cron.Dom \| cron.Month \| cron.DowOptional,
	cron.Second \| cron.Minute \| cron.Hour \| cron.Dom \| cron.Month \| cron.Dow,

		_, rotationPeriodChanged := data.Raw["rotation_period"]
		_, rotationScheduleChanged := data.Raw["rotation_schedule"]

		rotationSchedule := data.Get("rotation_schedule").(string)
		rotationScheduleOk := rotationSchedule != ""

secrets/database: advanced TTL management for static roles #22484

secrets/database: advanced TTL management for static roles #22484

Conversation

fairclothjm commented Aug 21, 2023

github-actions bot commented Aug 21, 2023

github-actions bot commented Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

austingebauer left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinay-gopalan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fairclothjm commented Aug 24, 2023

github-actions bot commented Aug 21, 2023 •

edited

Loading

austingebauer left a comment •

edited

Loading