From a4780807e8aceb5cec2ed89fe6a37b2b2e79cf4c Mon Sep 17 00:00:00 2001 From: Vault Automation Date: Mon, 11 May 2026 11:24:42 -0600 Subject: [PATCH] Allow WIF and rotation parameters to be set independently (#14414) (#14713) * allowing WIF and rotation parameters to be set independently * adding CL entry * VAULT-42211 allowing independently setting of parameter for client/config endpoint * introducing logic for identity token and rotation parameter detection * moving the detectection change logic to corresponding packages * sdk: add rotation and wif helpers * changelog * changelog updates --------- Co-authored-by: John-Michael Faircloth Co-authored-by: Martin Hristov --- builtin/credential/aws/path_config_client.go | 12 + .../credential/aws/path_config_client_test.go | 36 +++ changelog/_14414.txt | 7 + sdk/helper/automatedrotationutil/fields.go | 6 + .../automatedrotationutil/fields_test.go | 219 ++++++++++++++++++ sdk/helper/pluginidentityutil/fields.go | 6 + sdk/helper/pluginidentityutil/fields_test.go | 141 +++++++++++ 7 files changed, 427 insertions(+) create mode 100644 changelog/_14414.txt diff --git a/builtin/credential/aws/path_config_client.go b/builtin/credential/aws/path_config_client.go index 315381d0a3..3f34443767 100644 --- a/builtin/credential/aws/path_config_client.go +++ b/builtin/credential/aws/path_config_client.go @@ -368,14 +368,26 @@ func (b *backend) pathConfigClientCreateUpdate(ctx context.Context, req *logical configEntry.RoleARN = data.Get("role_arn").(string) } + // Checking if identity_token_ttl is actually changed, no need to flush the cache if it is not + previousIdentityParams := configEntry.PluginIdentityTokenParams if err := configEntry.ParsePluginIdentityTokenFields(data); err != nil { return logical.ErrorResponse(err.Error()), nil } + if !previousIdentityParams.Equals(configEntry.PluginIdentityTokenParams) { + changedCreds = true + } + + // Checking if any for the rotation parameters has been modified, if yes, we set "changedOtherConfig" to true + previousRotationParams := configEntry.AutomatedRotationParams if err := configEntry.ParseAutomatedRotationFields(data); err != nil { return logical.ErrorResponse(err.Error()), nil } + if !previousRotationParams.Equals(configEntry.AutomatedRotationParams) { + changedOtherConfig = true + } + // handle mutual exclusivity if configEntry.IdentityTokenAudience != "" && configEntry.AccessKey != "" { return logical.ErrorResponse("only one of 'access_key' or 'identity_token_audience' can be set"), nil diff --git a/builtin/credential/aws/path_config_client_test.go b/builtin/credential/aws/path_config_client_test.go index e901705c2b..1b24036024 100644 --- a/builtin/credential/aws/path_config_client_test.go +++ b/builtin/credential/aws/path_config_client_test.go @@ -187,3 +187,39 @@ func (d testSystemView) RegisterRotationJob(_ context.Context, _ *rotation.Rotat func (d testSystemView) DeregisterRotationJob(_ context.Context, _ *rotation.RotationJobDeregisterRequest) error { return nil } + +// TestBackend_PathConfigClient_RotationParameters tests that configuration +// of root creds rotation returns an immediate error. +func TestBackend_PathConfigClient_RotationParameters(t *testing.T) { + config := logical.TestBackendConfig() + config.StorageView = &logical.InmemStorage{} + config.System = &testSystemView{} + + b, err := Backend(config) + if err != nil { + t.Fatal(err) + } + + err = b.Setup(context.Background(), config) + if err != nil { + t.Fatal(err) + } + + configData := map[string]interface{}{ + "disable_automated_rotation": "false", + "rotation_schedule": "0 2 1-7 * TUE", + "rotation_window": "1h", + } + + configReq := &logical.Request{ + Operation: logical.UpdateOperation, + Storage: config.StorageView, + Path: "config/client", + Data: configData, + } + + resp, err := b.HandleRequest(context.Background(), configReq) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.ErrorContains(t, resp.Error(), automatedrotationutil.ErrRotationManagerUnsupported.Error()) +} diff --git a/changelog/_14414.txt b/changelog/_14414.txt new file mode 100644 index 0000000000..afbdfa453d --- /dev/null +++ b/changelog/_14414.txt @@ -0,0 +1,7 @@ +```release-note:bug +auth/aws: fix bug where rotation and wif config updates were not persisted to storage +``` +```release-note:improvement +sdk: add WIF and rotation helpers for checking if params were updated to allow +the consumer to know when changes need to be persisted to storage +``` diff --git a/sdk/helper/automatedrotationutil/fields.go b/sdk/helper/automatedrotationutil/fields.go index 267482be92..7b291fc175 100644 --- a/sdk/helper/automatedrotationutil/fields.go +++ b/sdk/helper/automatedrotationutil/fields.go @@ -250,3 +250,9 @@ func AddAutomatedRotationFieldsWithGroup(m map[string]*framework.FieldSchema, gr func AddAutomatedRotationFields(m map[string]*framework.FieldSchema) { AddAutomatedRotationFieldsWithGroup(m, "default") } + +// Equals returns true if the automated rotation parameters match the other instance. +// Useful for detecting configuration changes after parsing new field data. +func (p *AutomatedRotationParams) Equals(other AutomatedRotationParams) bool { + return *p == other +} diff --git a/sdk/helper/automatedrotationutil/fields_test.go b/sdk/helper/automatedrotationutil/fields_test.go index ae2fec8cbf..335761792e 100644 --- a/sdk/helper/automatedrotationutil/fields_test.go +++ b/sdk/helper/automatedrotationutil/fields_test.go @@ -525,3 +525,222 @@ func TestAddAutomatedRotationFields(t *testing.T) { }) } } + +func TestAutomatedRotationParams_Equals(t *testing.T) { + testcases := []struct { + name string + p1 AutomatedRotationParams + p2 AutomatedRotationParams + expected bool + }{ + { + name: "equal-all-fields", + p1: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + p2: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + expected: true, + }, + { + name: "equal-zero-values", + p1: AutomatedRotationParams{ + RotationSchedule: "", + RotationWindow: 0, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "", + }, + p2: AutomatedRotationParams{ + RotationSchedule: "", + RotationWindow: 0, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "", + }, + expected: true, + }, + { + name: "different-schedule", + p1: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + p2: AutomatedRotationParams{ + RotationSchedule: "*/30 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + expected: false, + }, + { + name: "different-window", + p1: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + p2: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 120 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + expected: false, + }, + { + name: "different-period", + p1: AutomatedRotationParams{ + RotationSchedule: "", + RotationWindow: 0, + RotationPeriod: 10 * time.Second, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + p2: AutomatedRotationParams{ + RotationSchedule: "", + RotationWindow: 0, + RotationPeriod: 20 * time.Second, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + expected: false, + }, + { + name: "different-disable-flag", + p1: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + p2: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: true, + RotationPolicy: "policy1", + }, + expected: false, + }, + { + name: "different-policy", + p1: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + p2: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy2", + }, + expected: false, + }, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + result := tt.p1.Equals(tt.p2) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestAutomatedRotationParams_Equals_Embedded(t *testing.T) { + // Test with embedded struct + type ConfigWithRotationParams struct { + Name string + AutomatedRotationParams + } + + testcases := []struct { + name string + c1 ConfigWithRotationParams + c2 ConfigWithRotationParams + expected bool + }{ + { + name: "embedded-equal", + c1: ConfigWithRotationParams{ + Name: "config1", + AutomatedRotationParams: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + }, + c2: ConfigWithRotationParams{ + Name: "config2", // Different name, but we only compare AutomatedRotationParams + AutomatedRotationParams: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + }, + expected: true, + }, + { + name: "embedded-different", + c1: ConfigWithRotationParams{ + Name: "config1", + AutomatedRotationParams: AutomatedRotationParams{ + RotationSchedule: "*/15 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + }, + c2: ConfigWithRotationParams{ + Name: "config1", + AutomatedRotationParams: AutomatedRotationParams{ + RotationSchedule: "*/30 * * * *", + RotationWindow: 60 * time.Second, + RotationPeriod: 0, + DisableAutomatedRotation: false, + RotationPolicy: "policy1", + }, + }, + expected: false, + }, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + // Test comparing the embedded fields directly + result := tt.c1.AutomatedRotationParams.Equals(tt.c2.AutomatedRotationParams) + assert.Equal(t, tt.expected, result) + + // Test using method promotion + result2 := tt.c1.Equals(tt.c2.AutomatedRotationParams) + assert.Equal(t, tt.expected, result2) + }) + } +} diff --git a/sdk/helper/pluginidentityutil/fields.go b/sdk/helper/pluginidentityutil/fields.go index 8347806b72..6cd0ee8f95 100644 --- a/sdk/helper/pluginidentityutil/fields.go +++ b/sdk/helper/pluginidentityutil/fields.go @@ -75,3 +75,9 @@ func AddPluginIdentityTokenFieldsWithGroup(m map[string]*framework.FieldSchema, func AddPluginIdentityTokenFields(m map[string]*framework.FieldSchema) { AddPluginIdentityTokenFieldsWithGroup(m, "default") } + +// Equals returns true if the plugin identity token parameters match the other instance. +// Useful for detecting configuration changes after parsing new field data. +func (p *PluginIdentityTokenParams) Equals(other PluginIdentityTokenParams) bool { + return *p == other +} diff --git a/sdk/helper/pluginidentityutil/fields_test.go b/sdk/helper/pluginidentityutil/fields_test.go index f726537ece..a63777d4a6 100644 --- a/sdk/helper/pluginidentityutil/fields_test.go +++ b/sdk/helper/pluginidentityutil/fields_test.go @@ -232,3 +232,144 @@ func TestAddPluginIdentityTokenFields(t *testing.T) { }) } } + +func TestPluginIdentityTokenParams_Equals(t *testing.T) { + testcases := []struct { + name string + p1 PluginIdentityTokenParams + p2 PluginIdentityTokenParams + expected bool + }{ + { + name: "equal-all-fields", + p1: PluginIdentityTokenParams{ + IdentityTokenTTL: 10 * time.Second, + IdentityTokenAudience: "test-aud", + }, + p2: PluginIdentityTokenParams{ + IdentityTokenTTL: 10 * time.Second, + IdentityTokenAudience: "test-aud", + }, + expected: true, + }, + { + name: "equal-zero-values", + p1: PluginIdentityTokenParams{ + IdentityTokenTTL: 0, + IdentityTokenAudience: "", + }, + p2: PluginIdentityTokenParams{ + IdentityTokenTTL: 0, + IdentityTokenAudience: "", + }, + expected: true, + }, + { + name: "different-ttl", + p1: PluginIdentityTokenParams{ + IdentityTokenTTL: 10 * time.Second, + IdentityTokenAudience: "test-aud", + }, + p2: PluginIdentityTokenParams{ + IdentityTokenTTL: 20 * time.Second, + IdentityTokenAudience: "test-aud", + }, + expected: false, + }, + { + name: "different-audience", + p1: PluginIdentityTokenParams{ + IdentityTokenTTL: 10 * time.Second, + IdentityTokenAudience: "test-aud", + }, + p2: PluginIdentityTokenParams{ + IdentityTokenTTL: 10 * time.Second, + IdentityTokenAudience: "different-aud", + }, + expected: false, + }, + { + name: "different-both", + p1: PluginIdentityTokenParams{ + IdentityTokenTTL: 10 * time.Second, + IdentityTokenAudience: "test-aud", + }, + p2: PluginIdentityTokenParams{ + IdentityTokenTTL: 20 * time.Second, + IdentityTokenAudience: "different-aud", + }, + expected: false, + }, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + result := tt.p1.Equals(tt.p2) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestPluginIdentityTokenParams_Equals_Embedded(t *testing.T) { + // Test with embedded struct + type ConfigWithIdentityParams struct { + Name string + PluginIdentityTokenParams + } + + testcases := []struct { + name string + c1 ConfigWithIdentityParams + c2 ConfigWithIdentityParams + expected bool + }{ + { + name: "embedded-equal", + c1: ConfigWithIdentityParams{ + Name: "config1", + PluginIdentityTokenParams: PluginIdentityTokenParams{ + IdentityTokenTTL: 10 * time.Second, + IdentityTokenAudience: "test-aud", + }, + }, + c2: ConfigWithIdentityParams{ + Name: "config2", // Different name, but we only compare PluginIdentityTokenParams + PluginIdentityTokenParams: PluginIdentityTokenParams{ + IdentityTokenTTL: 10 * time.Second, + IdentityTokenAudience: "test-aud", + }, + }, + expected: true, + }, + { + name: "embedded-different", + c1: ConfigWithIdentityParams{ + Name: "config1", + PluginIdentityTokenParams: PluginIdentityTokenParams{ + IdentityTokenTTL: 10 * time.Second, + IdentityTokenAudience: "test-aud", + }, + }, + c2: ConfigWithIdentityParams{ + Name: "config1", + PluginIdentityTokenParams: PluginIdentityTokenParams{ + IdentityTokenTTL: 20 * time.Second, + IdentityTokenAudience: "test-aud", + }, + }, + expected: false, + }, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + // Test comparing the embedded fields directly + result := tt.c1.PluginIdentityTokenParams.Equals(tt.c2.PluginIdentityTokenParams) + assert.Equal(t, tt.expected, result) + + // Test using method promotion + result2 := tt.c1.Equals(tt.c2.PluginIdentityTokenParams) + assert.Equal(t, tt.expected, result2) + }) + } +}