Small fixes on UX of Automated Root Rotation parameters (#29685)

builtin/logical/aws/path_config_root_test.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/vault/helper/namespace"
 	"github.com/hashicorp/vault/sdk/helper/automatedrotationutil"
@@ -43,7 +44,8 @@ func TestBackend_PathConfigRoot(t *testing.T) {
 		"identity_token_audience":    "",
 		"identity_token_ttl":         int64(0),
 		"rotation_schedule":          "",
-		"rotation_window":            0,
+		"rotation_period":            time.Duration(0).Seconds(),
+		"rotation_window":            time.Duration(0).Seconds(),
 		"disable_automated_rotation": false,
 	}
 
@@ -69,8 +71,6 @@ func TestBackend_PathConfigRoot(t *testing.T) {
 	}
 
 	delete(configData, "secret_key")
-	// remove rotation_period from response for comparison with original config
-	delete(resp.Data, "rotation_period")
 	require.Equal(t, configData, resp.Data)
 	if !reflect.DeepEqual(resp.Data, configData) {
 		t.Errorf("bad: expected to read config root as %#v, got %#v instead", configData, resp.Data)
@@ -103,7 +103,7 @@ func TestBackend_PathConfigRoot_STSFallback(t *testing.T) {
 		"identity_token_audience":    "",
 		"identity_token_ttl":         int64(0),
 		"rotation_schedule":          "",
-		"rotation_window":            0,
+		"rotation_window":            time.Duration(0).Seconds(),
 		"disable_automated_rotation": false,
 	}
 
@@ -152,7 +152,7 @@ func TestBackend_PathConfigRoot_STSFallback(t *testing.T) {
 		"identity_token_audience":    "",
 		"identity_token_ttl":         int64(0),
 		"rotation_schedule":          "",
-		"rotation_window":            0,
+		"rotation_window":            time.Duration(0).Seconds(),
 		"disable_automated_rotation": false,
 	}
 

builtin/logical/database/backend_test.go

@@ -214,8 +214,8 @@ func TestBackend_config_connection(t *testing.T) {
 			"verify_connection":                  false,
 			"skip_static_role_import_rotation":   false,
 			"rotation_schedule":                  "",
-			"rotation_period":                    0,
+			"rotation_period":                    time.Duration(0).Seconds(),
-			"rotation_window":                    0,
+			"rotation_window":                    time.Duration(0).Seconds(),
 			"disable_automated_rotation":         false,
 		}
 		configReq.Operation = logical.ReadOperation
@@ -225,7 +225,6 @@ func TestBackend_config_connection(t *testing.T) {
 		}
 
 		delete(resp.Data["connection_details"].(map[string]interface{}), "name")
-		delete(resp.Data, "AutomatedRotationParams")
 		if !reflect.DeepEqual(expected, resp.Data) {
 			t.Fatalf("bad: expected:%#v\nactual:%#v\n", expected, resp.Data)
 		}
@@ -275,8 +274,8 @@ func TestBackend_config_connection(t *testing.T) {
 			"verify_connection":                  false,
 			"skip_static_role_import_rotation":   false,
 			"rotation_schedule":                  "",
-			"rotation_period":                    0,
+			"rotation_period":                    time.Duration(0).Seconds(),
-			"rotation_window":                    0,
+			"rotation_window":                    time.Duration(0).Seconds(),
 			"disable_automated_rotation":         false,
 		}
 		configReq.Operation = logical.ReadOperation
@@ -325,8 +324,8 @@ func TestBackend_config_connection(t *testing.T) {
 			"verify_connection":                  false,
 			"skip_static_role_import_rotation":   false,
 			"rotation_schedule":                  "",
-			"rotation_period":                    0,
+			"rotation_period":                    time.Duration(0).Seconds(),
-			"rotation_window":                    0,
+			"rotation_window":                    time.Duration(0).Seconds(),
 			"disable_automated_rotation":         false,
 		}
 		configReq.Operation = logical.ReadOperation

builtin/logical/database/path_config_connection.go

@@ -416,6 +416,9 @@ func (b *databaseBackend) connectionReadHandler() framework.OperationFunc {
 
 		resp.Data = structs.New(config).Map()
 		config.PopulateAutomatedRotationData(resp.Data)
+		// remove extra nested AutomatedRotationParams key
+		// before returning response
+		delete(resp.Data, "AutomatedRotationParams")
 		return resp, nil
 	}
 }
@@ -523,7 +526,7 @@ func (b *databaseBackend) connectionWriteHandler() framework.OperationFunc {
 		delete(data.Raw, "skip_static_role_import_rotation")
 		delete(data.Raw, "rotation_schedule")
 		delete(data.Raw, "rotation_window")
-		delete(data.Raw, "rotation_ttl")
+		delete(data.Raw, "rotation_period")
 		delete(data.Raw, "disable_automated_rotation")
 
 		id, err := uuid.GenerateUUID()

sdk/helper/automatedrotationutil/fields.go

@@ -6,6 +6,7 @@ package automatedrotationutil
 import (
 	"errors"
 	"fmt"
+	"time"
 
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/rotation"
@@ -23,9 +24,9 @@ type AutomatedRotationParams struct {
 	RotationSchedule string `json:"rotation_schedule"`
 	// RotationWindow specifies the amount of time in which the rotation is allowed to
 	// occur starting from a given rotation_schedule.
-	RotationWindow int `json:"rotation_window"`
+	RotationWindow time.Duration `json:"rotation_window"`
 	// RotationPeriod is an alternate choice for simple time-to-live based rotation timing.
-	RotationPeriod int `json:"rotation_period"`
+	RotationPeriod time.Duration `json:"rotation_period"`
 
 	// If set, will deregister all registered rotation jobs from the RotationManager for plugin.
 	DisableAutomatedRotation bool `json:"disable_automated_rotation"`
@@ -34,11 +35,12 @@ type AutomatedRotationParams struct {
 // ParseAutomatedRotationFields provides common field parsing to embedding structs.
 func (p *AutomatedRotationParams) ParseAutomatedRotationFields(d *framework.FieldData) error {
 	rotationScheduleRaw, scheduleOk := d.GetOk("rotation_schedule")
-	rotationWindowRaw, windowOk := d.GetOk("rotation_window")
+	rotationWindowSecondsRaw, windowOk := d.GetOk("rotation_window")
-	rotationPeriodRaw, periodOk := d.GetOk("rotation_period")
+	rotationPeriodSecondsRaw, periodOk := d.GetOk("rotation_period")
+	disableRotation, disableRotationOk := d.GetOk("disable_automated_rotation")
 
 	if scheduleOk {
-		if periodOk {
+		if periodOk && rotationPeriodSecondsRaw.(int) != 0 && rotationScheduleRaw.(string) != "" {
 			return ErrRotationMutuallyExclusiveFields
 		}
 		p.RotationSchedule = rotationScheduleRaw.(string)
@@ -53,21 +55,25 @@ func (p *AutomatedRotationParams) ParseAutomatedRotationFields(d *framework.Fiel
 	}
 
 	if windowOk {
-		if periodOk {
+		if periodOk && rotationPeriodSecondsRaw.(int) != 0 && rotationWindowSecondsRaw.(int) != 0 {
 			return fmt.Errorf("rotation_window does not apply to period")
 		}
-		p.RotationWindow = rotationWindowRaw.(int)
+		rotationWindowSeconds := rotationWindowSecondsRaw.(int)
+		p.RotationWindow = time.Duration(rotationWindowSeconds) * time.Second
 	}
 
 	if periodOk {
-		p.RotationPeriod = rotationPeriodRaw.(int)
+		rotationPeriodSeconds := rotationPeriodSecondsRaw.(int)
+		p.RotationPeriod = time.Duration(rotationPeriodSeconds) * time.Second
 	}
 
-	if windowOk && !scheduleOk {
+	if (windowOk && rotationWindowSecondsRaw.(int) != 0) && !scheduleOk {
 		return fmt.Errorf("cannot use rotation_window without rotation_schedule")
 	}
 
-	p.DisableAutomatedRotation = d.Get("disable_automated_rotation").(bool)
+	if disableRotationOk {
+		p.DisableAutomatedRotation = disableRotation.(bool)
+	}
 
 	return nil
 }
@@ -75,8 +81,8 @@ func (p *AutomatedRotationParams) ParseAutomatedRotationFields(d *framework.Fiel
 // PopulateAutomatedRotationData adds PluginIdentityTokenParams info into the given map.
 func (p *AutomatedRotationParams) PopulateAutomatedRotationData(m map[string]interface{}) {
 	m["rotation_schedule"] = p.RotationSchedule
-	m["rotation_window"] = p.RotationWindow
+	m["rotation_window"] = p.RotationWindow.Seconds()
-	m["rotation_period"] = p.RotationPeriod
+	m["rotation_period"] = p.RotationPeriod.Seconds()
 	m["disable_automated_rotation"] = p.DisableAutomatedRotation
 }
 
@@ -97,11 +103,11 @@ func AddAutomatedRotationFields(m map[string]*framework.FieldSchema) {
 			Description: "CRON-style string that will define the schedule on which rotations should occur. Mutually exclusive with rotation_period",
 		},
 		"rotation_window": {
-			Type:        framework.TypeInt,
+			Type:        framework.TypeDurationSecond,
 			Description: "Specifies the amount of time in which the rotation is allowed to occur starting from a given rotation_schedule",
 		},
 		"rotation_period": {
-			Type:        framework.TypeInt,
+			Type:        framework.TypeDurationSecond,
 			Description: "TTL for automatic credential rotation of the given username. Mutually exclusive with rotation_schedule",
 		},
 		"disable_automated_rotation": {

sdk/helper/automatedrotationutil/fields_test.go

@@ -7,6 +7,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/vault/sdk/framework"
 )
@@ -17,11 +18,11 @@ var schemaMap = map[string]*framework.FieldSchema{
 		Description: "CRON-style string that will define the schedule on which rotations should occur. Mutually exclusive with rotation_period",
 	},
 	"rotation_window": {
-		Type:        framework.TypeInt,
+		Type:        framework.TypeDurationSecond,
 		Description: "Specifies the amount of time in which the rotation is allowed to occur starting from a given rotation_schedule",
 	},
 	"rotation_period": {
-		Type:        framework.TypeInt,
+		Type:        framework.TypeDurationSecond,
 		Description: "TTL for automatic credential rotation of the given username. Mutually exclusive with rotation_schedule",
 	},
 	"disable_automated_rotation": {
@@ -35,6 +36,7 @@ func TestParseAutomatedRotationFields(t *testing.T) {
 		name           string
 		data           *framework.FieldData
 		expectedParams *AutomatedRotationParams
+		initialParams  *AutomatedRotationParams
 		expectedError  string
 	}{
 		{
@@ -48,7 +50,7 @@ func TestParseAutomatedRotationFields(t *testing.T) {
 			},
 			expectedParams: &AutomatedRotationParams{
 				RotationSchedule:         "*/15 * * * *",
-				RotationWindow:           60,
+				RotationWindow:           60 * time.Second,
 				RotationPeriod:           0,
 				DisableAutomatedRotation: false,
 			},
@@ -96,11 +98,118 @@ func TestParseAutomatedRotationFields(t *testing.T) {
 			},
 			expectedError: "cannot use rotation_window without rotation_schedule",
 		},
+		{
+			name: "rotation-period-duration-seconds",
+			data: &framework.FieldData{
+				Raw: map[string]interface{}{
+					"rotation_period": "2m",
+				},
+				Schema: schemaMap,
+			},
+			expectedParams: &AutomatedRotationParams{
+				RotationSchedule:         "",
+				RotationWindow:           0,
+				RotationPeriod:           120 * time.Second,
+				DisableAutomatedRotation: false,
+			},
+		},
+		{
+			name: "rotation-window-duration-seconds",
+			data: &framework.FieldData{
+				Raw: map[string]interface{}{
+					"rotation_window":   "12h",
+					"rotation_schedule": "* */2 * * *",
+				},
+				Schema: schemaMap,
+			},
+			expectedParams: &AutomatedRotationParams{
+				RotationSchedule:         "* */2 * * *",
+				RotationWindow:           12 * time.Hour,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+			},
+		},
+		{
+			name: "period-and-window-ok",
+			data: &framework.FieldData{
+				Raw: map[string]interface{}{
+					"rotation_window": 0,
+					"rotation_period": 10,
+				},
+				Schema: schemaMap,
+			},
+			expectedParams: &AutomatedRotationParams{
+				RotationSchedule:         "",
+				RotationWindow:           0,
+				RotationPeriod:           10 * time.Second,
+				DisableAutomatedRotation: false,
+			},
+		},
+		{
+			name: "period-and-window-ok-strings",
+			data: &framework.FieldData{
+				Raw: map[string]interface{}{
+					"rotation_schedule": "* */2 * * *",
+					"rotation_window":   "5h",
+					"rotation_period":   "",
+				},
+				Schema: schemaMap,
+			},
+			expectedParams: &AutomatedRotationParams{
+				RotationSchedule:         "* */2 * * *",
+				RotationWindow:           5 * time.Hour,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+			},
+		},
+		{
+			name: "period-and-schedule-ok",
+			data: &framework.FieldData{
+				Raw: map[string]interface{}{
+					"rotation_schedule": "",
+					"rotation_window":   "",
+					"rotation_period":   "2m",
+				},
+				Schema: schemaMap,
+			},
+			expectedParams: &AutomatedRotationParams{
+				RotationSchedule:         "",
+				RotationWindow:           0,
+				RotationPeriod:           2 * time.Minute,
+				DisableAutomatedRotation: false,
+			},
+		},
+		{
+			name: "zero-out-schedule-and-window-set-period",
+			data: &framework.FieldData{
+				Raw: map[string]interface{}{
+					"rotation_schedule": "",
+					"rotation_window":   "",
+					"rotation_period":   "2m",
+				},
+				Schema: schemaMap,
+			},
+			expectedParams: &AutomatedRotationParams{
+				RotationSchedule:         "",
+				RotationWindow:           0,
+				RotationPeriod:           2 * time.Minute,
+				DisableAutomatedRotation: false,
+			},
+			initialParams: &AutomatedRotationParams{
+				RotationSchedule:         "*/1 * * * *",
+				RotationWindow:           30 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+			},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			p := &AutomatedRotationParams{}
+			if tt.initialParams != nil {
+				p = tt.initialParams
+			}
 			err := p.ParseAutomatedRotationFields(tt.data)
 			if err != nil {
 				if tt.expectedError == "" {
@@ -128,8 +237,8 @@ func TestPopulateAutomatedRotationData(t *testing.T) {
 			name: "basic",
 			expected: map[string]interface{}{
 				"rotation_schedule":          "*/15 * * * *",
-				"rotation_window":            60,
+				"rotation_window":            time.Duration(60).Seconds(),
-				"rotation_period":            0,
+				"rotation_period":            time.Duration(0).Seconds(),
 				"disable_automated_rotation": false,
 			},
 			inputParams: &AutomatedRotationParams{

sdk/plugin/grpc_system.go

@@ -472,8 +472,8 @@ func (s *gRPCSystemViewServer) RegisterRotationJob(ctx context.Context, req *pb.
 		MountPoint:       req.Job.MountPoint,
 		ReqPath:          req.Job.Path,
 		RotationSchedule: req.Job.RotationSchedule,
-		RotationWindow:   int(req.Job.RotationWindow),
+		RotationWindow:   time.Duration(req.Job.RotationWindow) * time.Second,
-		RotationPeriod:   int(req.Job.RotationPeriod),
+		RotationPeriod:   time.Duration(req.Job.RotationPeriod) * time.Second,
 	}
 
 	rotationID, err := s.impl.RegisterRotationJob(ctx, cfgReq)

sdk/rotation/rotation_job.go

@@ -40,8 +40,8 @@ type RotationJobConfigureRequest struct {
 	MountPoint       string
 	ReqPath          string
 	RotationSchedule string
-	RotationWindow   int
+	RotationWindow   time.Duration
-	RotationPeriod   int
+	RotationPeriod   time.Duration
 }
 
 type RotationJobDeregisterRequest struct {
@@ -58,7 +58,7 @@ func (s *RotationJob) Validate() error {
 		return fmt.Errorf("ReqPath is required")
 	}
 
-	if s.Schedule.RotationSchedule == "" && s.Schedule.RotationPeriod == 0 {
+	if s.Schedule.RotationSchedule == "" && s.Schedule.RotationPeriod.Seconds() == 0 {
 		return fmt.Errorf("RotationSchedule or RotationPeriod is required to set up rotation job")
 	}
 
@@ -78,8 +78,8 @@ func newRotationJob(configRequest *RotationJobConfigureRequest) (*RotationJob, e
 	rs := &RotationSchedule{
 		Schedule:          cronSc,
 		RotationSchedule:  configRequest.RotationSchedule,
-		RotationWindow:    time.Duration(configRequest.RotationWindow) * time.Second,
+		RotationWindow:    configRequest.RotationWindow,
-		RotationPeriod:    time.Duration(configRequest.RotationPeriod) * time.Second,
+		RotationPeriod:    configRequest.RotationPeriod,
 		NextVaultRotation: time.Time{},
 		LastVaultRotation: time.Time{},
 	}