diff --git a/docs.md b/docs.md index a5bb74174d..074d7d1e6d 100644 --- a/docs.md +++ b/docs.md @@ -632,11 +632,19 @@ When settings are created or updated, the following common checks take place: - If set, `user-retention-cron` must be a valid standard cron expression (e.g. `0 0 * * 0`). - The `auth-user-session-ttl-minutes` must be a positive integer and can't be greater than `disable-inactive-user-after` or `delete-inactive-user-after` if those values are set. - The `auth-user-session-idle-ttl-minutes` must be a positive integer and can't be greater than `auth-user-session-ttl-minutes`. +- The `auth-user-info-max-age-seconds` must be a valid duration value. +- The `auth-user-info-resync-cron` must be a valid cron expression. #### Update When settings are updated, the following additional checks take place: +- Settings with a source of `env` may not be updated + +- Read only settings like `cacerts` must not be updated. + +- The new value for the setting must not be "" + - If `agent-tls-mode` has `default` or `value` updated from `system-store` to `strict`, then all non-local clusters must have a status condition `AgentTlsStrictCheck` set to `True`, unless the new setting has an overriding annotation `cattle.io/force=true`. diff --git a/pkg/resources/management.cattle.io/v3/setting/Setting.md b/pkg/resources/management.cattle.io/v3/setting/Setting.md index 5021c6ff36..bceea37b99 100644 --- a/pkg/resources/management.cattle.io/v3/setting/Setting.md +++ b/pkg/resources/management.cattle.io/v3/setting/Setting.md @@ -10,11 +10,19 @@ When settings are created or updated, the following common checks take place: - If set, `user-retention-cron` must be a valid standard cron expression (e.g. `0 0 * * 0`). - The `auth-user-session-ttl-minutes` must be a positive integer and can't be greater than `disable-inactive-user-after` or `delete-inactive-user-after` if those values are set. - The `auth-user-session-idle-ttl-minutes` must be a positive integer and can't be greater than `auth-user-session-ttl-minutes`. +- The `auth-user-info-max-age-seconds` must be a valid duration value. +- The `auth-user-info-resync-cron` must be a valid cron expression. ### Update When settings are updated, the following additional checks take place: +- Settings with a source of `env` may not be updated + +- Read only settings like `cacerts` must not be updated. + +- The new value for the setting must not be "" + - If `agent-tls-mode` has `default` or `value` updated from `system-store` to `strict`, then all non-local clusters must have a status condition `AgentTlsStrictCheck` set to `True`, unless the new setting has an overriding annotation `cattle.io/force=true`. diff --git a/pkg/resources/management.cattle.io/v3/setting/validator.go b/pkg/resources/management.cattle.io/v3/setting/validator.go index 2d565d52ed..464c78165d 100644 --- a/pkg/resources/management.cattle.io/v3/setting/validator.go +++ b/pkg/resources/management.cattle.io/v3/setting/validator.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "slices" "strconv" "time" @@ -25,17 +26,19 @@ import ( ) const ( + AgentTLSMode = "agent-tls-mode" + AuthUserInfoResyncCron = "auth-user-info-resync-cron" + AuthUserInfoMaxAgeSeconds = "auth-user-info-max-age-seconds" + AuthUserSessionIdleTTLMinutes = "auth-user-session-idle-ttl-minutes" + AuthUserSessionTTLMinutes = "auth-user-session-ttl-minutes" + CattleClusterAgentPodDisruptionBudget = "cluster-agent-default-pod-disruption-budget" + CattleClusterAgentPriorityClass = "cluster-agent-default-priority-class" DeleteInactiveUserAfter = "delete-inactive-user-after" DisableInactiveUserAfter = "disable-inactive-user-after" - AuthUserSessionTTLMinutes = "auth-user-session-ttl-minutes" - AuthUserSessionIdleTTLMinutes = "auth-user-session-idle-ttl-minutes" + FleetAgentPodDisruptionBudget = "fleet-agent-default-pod-disruption-budget" + FleetAgentPriorityClass = "fleet-agent-default-priority-class" UserLastLoginDefault = "user-last-login-default" UserRetentionCron = "user-retention-cron" - AgentTLSMode = "agent-tls-mode" - CattleClusterAgentPriorityClass = "cluster-agent-default-priority-class" - CattleClusterAgentPodDisruptionBudget = "cluster-agent-default-pod-disruption-budget" - FleetAgentPriorityClass = "fleet-agent-default-priority-class" - FleetAgentPodDisruptionBudget = "fleet-agent-default-pod-disruption-budget" ) // MinDeleteInactiveUserAfter is the minimum duration for delete-inactive-user-after setting. @@ -117,7 +120,19 @@ func (a *admitter) admitCreate(newSetting *v3.Setting) (*admissionv1.AdmissionRe return a.admitCommonCreateUpdate(nil, newSetting) } +var ReadOnlySettings = []string{ + "cacerts", +} + func (a *admitter) admitUpdate(oldSetting, newSetting *v3.Setting) (*admissionv1.AdmissionResponse, error) { + if oldSetting.Source == "env" { + return admission.ResponseBadRequest("setting cannot be updated since its value is sourced from an environment variable"), nil + } + + if slices.Contains(ReadOnlySettings, oldSetting.Name) { + return admission.ResponseBadRequest("setting is read only"), nil + } + var err error switch newSetting.Name { @@ -153,6 +168,10 @@ func (a *admitter) admitCommonCreateUpdate(_, newSetting *v3.Setting) (*admissio err = a.validateAuthUserSessionTTLMinutes(newSetting) case AuthUserSessionIdleTTLMinutes: err = a.validateAuthUserSessionIdleTTLMinutes(newSetting) + case AuthUserInfoMaxAgeSeconds: + err = a.validateAuthUserInfoMaxAgeSeconds(newSetting) + case AuthUserInfoResyncCron: + err = a.validateAuthUserInfoResyncCron(newSetting) default: } @@ -341,6 +360,32 @@ func (a *admitter) validateUserRetentionCron(s *v3.Setting) error { return nil } +// validateAuthUserInfoMaxAgeSeconds validates the auth-user-info-max-age-seconds setting +// to make sure it's a valid duration in seconds. +func (a *admitter) validateAuthUserInfoMaxAgeSeconds(s *v3.Setting) error { + // We cannot use the validateDuration func since it does not allow for negative durations + // which are valid for the auth-user-info-max-age-seconds setting. + if _, err := time.ParseDuration(s.Value + "s"); err != nil { + return field.TypeInvalid(valuePath, s.Value, err.Error()) + } + + return nil +} + +// validateAuthUserInfoResyncCron validates the auth-user-info-resync-cron setting +// to make sure it's a valid cron string as defined in https://en.wikipedia.org/wiki/Cron. +func (a *admitter) validateAuthUserInfoResyncCron(s *v3.Setting) error { + if s.Value == "" { + return nil + } + + if _, err := cron.ParseStandard(s.Value); err != nil { + return field.TypeInvalid(valuePath, s.Value, err.Error()) + } + + return nil +} + // validateUserLastLoginDefault validates the user-last-login-default setting // to make sure it's a valid RFC3339 formatted date time. func (a *admitter) validateUserLastLoginDefault(s *v3.Setting) error { diff --git a/pkg/resources/management.cattle.io/v3/setting/validator_test.go b/pkg/resources/management.cattle.io/v3/setting/validator_test.go index 42905b65f6..4d952f4921 100644 --- a/pkg/resources/management.cattle.io/v3/setting/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/setting/validator_test.go @@ -37,6 +37,58 @@ var ( gvr = metav1.GroupVersionResource{Group: "management.cattle.io", Version: "v3", Resource: "settings"} ) +func (s *SettingSuite) TestAdmitUpdateGuards() { + tests := []struct { + name string + oldSetting *v3.Setting + newSetting *v3.Setting + allowed bool + }{ + { + name: "reject update when sourced from env", + oldSetting: &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{Name: setting.UserRetentionCron}, + Source: "env", + }, + newSetting: &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{Name: setting.UserRetentionCron}, + Value: "0 0 * * *", + }, + }, + { + name: "reject read-only setting", + oldSetting: &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{Name: "cacerts"}, + }, + newSetting: &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{Name: "cacerts"}, + Value: "new-certs", + }, + }, + { + name: "allow normal valid update", + oldSetting: &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{Name: setting.UserRetentionCron}, + }, + newSetting: &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{Name: setting.UserRetentionCron}, + Value: "0 0 * * *", + }, + allowed: true, + }, + } + + for _, test := range tests { + test := test + s.T().Run(test.name, func(t *testing.T) { + t.Parallel() + + validator := setting.NewValidator(nil, nil) + s.testAdmit(t, validator, test.oldSetting, test.newSetting, v1.Update, test.allowed) + }) + } +} + func (s *SettingSuite) TestValidateDisableInactiveUserAfterOnUpdate() { s.validateDisableInactiveUserAfter(v1.Update) } @@ -303,6 +355,105 @@ func (s *SettingSuite) TestValidateUserLastLoginDefaultOnCreate() { s.validateUserLastLoginDefault(v1.Create) } +func (s *SettingSuite) TestValidateAuthUserInfoMaxAgeSecondsOnUpdate() { + s.validateAuthUserInfoMaxAgeSeconds(v1.Update) +} + +func (s *SettingSuite) TestValidateAuthUserInfoMaxAgeSecondsOnCreate() { + s.validateAuthUserInfoMaxAgeSeconds(v1.Create) +} + +func (s *SettingSuite) validateAuthUserInfoMaxAgeSeconds(op v1.Operation) { + tests := []struct { + desc string + value string + allowed bool + }{ + { + desc: "valid max age", + value: "3600", + allowed: true, + }, + { + desc: "valid negative max age", + value: "-1", + allowed: true, + }, + { + desc: "invalid max age", + value: "foo", + }, + } + + for _, test := range tests { + test := test + s.T().Run(test.desc, func(t *testing.T) { + t.Parallel() + + validator := setting.NewValidator(nil, nil) + s.testAdmit(t, validator, &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: setting.AuthUserInfoMaxAgeSeconds, + }, + }, &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: setting.AuthUserInfoMaxAgeSeconds, + }, + Value: test.value, + }, op, test.allowed) + }) + } +} + +func (s *SettingSuite) TestValidateAuthUserInfoResyncCronOnUpdate() { + s.validateAuthUserInfoResyncCron(v1.Update) +} + +func (s *SettingSuite) TestValidateAuthUserInfoResyncCronOnCreate() { + s.validateAuthUserInfoResyncCron(v1.Create) +} + +func (s *SettingSuite) validateAuthUserInfoResyncCron(op v1.Operation) { + tests := []struct { + desc string + value string + allowed bool + }{ + { + desc: "valid cron expression", + value: "0 0 * * *", + allowed: true, + }, + { + desc: "non-standard cron expression", + value: "* * * * * *", + }, + { + desc: "nonsensical value", + value: "foo", + }, + } + + for _, test := range tests { + test := test + s.T().Run(test.desc, func(t *testing.T) { + t.Parallel() + + validator := setting.NewValidator(nil, nil) + s.testAdmit(t, validator, &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: setting.AuthUserInfoResyncCron, + }, + }, &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: setting.AuthUserInfoResyncCron, + }, + Value: test.value, + }, op, test.allowed) + }) + } +} + func (s *SettingSuite) validateUserLastLoginDefault(op v1.Operation) { tests := []struct { desc string