diff --git a/docs.md b/docs.md index db2692818..264ff7b59 100644 --- a/docs.md +++ b/docs.md @@ -309,6 +309,12 @@ When a Setting is updated, the following checks take place: - If set, `user-last-login-default` must be a date time according to RFC3339 (e.g. `2023-11-29T00:00:00Z`). - If set, `user-retention-cron` must be a valid standard cron expression (e.g. `0 0 * * 1`). +#### Forbidden - Update + +- 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`. + ## UserAttribute ### Validation Checks diff --git a/pkg/resources/management.cattle.io/v3/setting/Setting.md b/pkg/resources/management.cattle.io/v3/setting/Setting.md index f735424a9..4df21041a 100644 --- a/pkg/resources/management.cattle.io/v3/setting/Setting.md +++ b/pkg/resources/management.cattle.io/v3/setting/Setting.md @@ -17,3 +17,9 @@ When a Setting is updated, the following checks take place: - If set, `delete-inactive-user-after` must be zero or a positive duration (e.g. `240h`). - If set, `user-last-login-default` must be a date time according to RFC3339 (e.g. `2023-11-29T00:00:00Z`). - If set, `user-retention-cron` must be a valid standard cron expression (e.g. `0 0 * * 1`). + +### Forbidden - Update + +- 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 da2d436ab..6b336fd09 100644 --- a/pkg/resources/management.cattle.io/v3/setting/validator.go +++ b/pkg/resources/management.cattle.io/v3/setting/validator.go @@ -6,14 +6,18 @@ import ( "time" v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" - "github.com/rancher/webhook/pkg/admission" - objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3" "github.com/robfig/cron" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/utils/trace" + + "github.com/rancher/webhook/pkg/admission" + controllerv3 "github.com/rancher/webhook/pkg/generated/controllers/management.cattle.io/v3" + objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3" ) // MinDeleteInactiveUserAfter is the minimum duration for delete-inactive-user-after setting. @@ -33,9 +37,11 @@ type Validator struct { } // NewValidator returns a new Validator instance. -func NewValidator() *Validator { +func NewValidator(clusterCache controllerv3.ClusterCache) *Validator { return &Validator{ - admitter: admitter{}, + admitter: admitter{ + clusterCache: clusterCache, + }, } } @@ -61,29 +67,36 @@ func (v *Validator) Admitters() []admission.Admitter { return []admission.Admitter{&v.admitter} } -type admitter struct{} +type admitter struct { + clusterCache controllerv3.ClusterCache +} // Admit handles the webhook admission requests. func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) { listTrace := trace.New("userAttributeValidator Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) defer listTrace.LogIfLong(admission.SlowTraceDuration) - if request.Operation == admissionv1.Create || request.Operation == admissionv1.Update { - setting, err := objectsv3.SettingFromRequest(&request.AdmissionRequest) - if err != nil { - return nil, fmt.Errorf("failed to get Setting from request: %w", err) + oldSetting, newSetting, err := objectsv3.SettingOldAndNewFromRequest(&request.AdmissionRequest) + if err != nil { + return nil, fmt.Errorf("failed to get Setting from request: %w", err) + } + switch request.Operation { + case admissionv1.Create: + if err := a.validateUserRetentionSettings(newSetting); err != nil { + return admission.ResponseBadRequest(err.Error()), nil } - - err = a.validateSetting(setting) - if err != nil { + case admissionv1.Update: + if err := a.validateUserRetentionSettings(newSetting); err != nil { + return admission.ResponseBadRequest(err.Error()), nil + } + if err := a.validateAgentTLSMode(*oldSetting, *newSetting); err != nil { return admission.ResponseBadRequest(err.Error()), nil } } - return admission.ResponseAllowed(), nil } -func (a *admitter) validateSetting(s *v3.Setting) error { +func (a *admitter) validateUserRetentionSettings(s *v3.Setting) error { var err error switch s.Name { @@ -117,6 +130,31 @@ func (a *admitter) validateSetting(s *v3.Setting) error { return nil } +func (a *admitter) validateAgentTLSMode(oldSetting, newSetting v3.Setting) error { + if oldSetting.Name != "agent-tls-mode" || newSetting.Name != "agent-tls-mode" { + return nil + } + if effectiveValue(oldSetting) == "system-store" && effectiveValue(newSetting) == "strict" { + if _, force := newSetting.Annotations["cattle.io/force"]; force { + return nil + } + clusters, err := a.clusterCache.List(labels.NewSelector()) + if err != nil { + return fmt.Errorf("failed to list clusters: %w", err) + } + for _, cluster := range clusters { + if cluster.Name == "local" { + continue + } + if !clusterConditionMatches(cluster, "AgentTlsStrictCheck", "True") { + return field.Forbidden(field.NewPath("value", "default"), + fmt.Sprintf("AgentTlsStrictCheck condition of cluster %s isn't 'True'", cluster.Name)) + } + } + } + return nil +} + func validateDuration(value string) (time.Duration, error) { dur, err := time.ParseDuration(value) if err != nil { @@ -129,3 +167,21 @@ func validateDuration(value string) (time.Duration, error) { return dur, err } + +func clusterConditionMatches(cluster *v3.Cluster, t v3.ClusterConditionType, status v1.ConditionStatus) bool { + for _, cond := range cluster.Status.Conditions { + if cond.Type == t && cond.Status == status { + return true + } + } + return false +} + +func effectiveValue(s v3.Setting) string { + if s.Value != "" { + return s.Value + } else if s.Default != "" { + return s.Default + } + return "" +} 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 b13884562..6b59a2a6a 100644 --- a/pkg/resources/management.cattle.io/v3/setting/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/setting/validator_test.go @@ -3,18 +3,23 @@ package setting_test import ( "context" "encoding/json" + "errors" "testing" "time" + "github.com/golang/mock/gomock" v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" - "github.com/rancher/webhook/pkg/admission" - "github.com/rancher/webhook/pkg/resources/management.cattle.io/v3/setting" + "github.com/rancher/wrangler/v2/pkg/generic/fake" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" v1 "k8s.io/api/admission/v1" authenticationv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + + "github.com/rancher/webhook/pkg/admission" + "github.com/rancher/webhook/pkg/resources/management.cattle.io/v3/setting" ) type SettingSuite struct { @@ -160,7 +165,7 @@ func (s *SettingSuite) validate(op v1.Operation) { } func (s *SettingSuite) setup() admission.Admitter { - validator := setting.NewValidator() + validator := setting.NewValidator(nil) s.Len(validator.Admitters(), 1, "expected 1 admitter") return validator.Admitters()[0] @@ -182,3 +187,419 @@ func newRequest(op v1.Operation, obj, oldObj []byte) *admission.Request { Context: context.Background(), } } + +func TestValidateAgentTLSMode(t *testing.T) { + t.Parallel() + tests := map[string]struct { + oldSetting v3.Setting + newSetting v3.Setting + operation v1.Operation + clusters []*v3.Cluster + clusterListerFails bool + allowed bool + }{ + "create allowed for system store": { + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "system-store", + }, + operation: v1.Create, + allowed: true, + }, + "create allowed for strict": { + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "strict", + }, + operation: v1.Create, + allowed: true, + }, + "update forbidden due to missing status": { + oldSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Value: "system-store", + }, + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Value: "strict", + }, + clusters: []*v3.Cluster{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-2", + }, + }, + }, + operation: v1.Update, + allowed: false, + }, + "update allowed without cluster status but with force annotation": { + oldSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "system-store", + }, + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + Annotations: map[string]string{ + "cattle.io/force": "true", + }, + }, + Default: "strict", + }, + clusters: []*v3.Cluster{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-2", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "Foo", + Status: "True", + }, + }, + }, + }, + }, + operation: v1.Update, + allowed: true, + }, + "update allowed with cluster status and force annotation": { + oldSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Value: "system-store", + }, + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + Annotations: map[string]string{ + "cattle.io/force": "true", + }, + }, + Value: "strict", + }, + clusters: []*v3.Cluster{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-2", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + }, + operation: v1.Update, + allowed: true, + }, + "update allowed from strict to system store": { + oldSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "strict", + }, + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "system-store", + }, + operation: v1.Update, + allowed: true, + }, + "update allowed from system store to strict": { + oldSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "system-store", + Value: "system-store", + }, + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "strict", + Value: "strict", + }, + clusters: []*v3.Cluster{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "local", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "False", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-2", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + }, + operation: v1.Update, + allowed: true, + }, + "update allowed with value changing from system store to strict": { + oldSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "system-store", + Value: "", + }, + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "system-store", + Value: "strict", + }, + clusters: []*v3.Cluster{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "local", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-2", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + }, + operation: v1.Update, + allowed: true, + }, + "update forbidden from system store to strict due to incorrect value on target status": { + oldSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "system-store", + Value: "system-store", + }, + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "strict", + Value: "strict", + }, + clusters: []*v3.Cluster{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "local", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "True", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-2", + }, + Status: v3.ClusterStatus{ + Conditions: []v3.ClusterCondition{ + { + Type: "AgentTlsStrictCheck", + Status: "False", + }, + }, + }, + }, + }, + operation: v1.Update, + allowed: false, + }, + "update forbidden on error to list clusters": { + oldSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "system-store", + }, + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + Default: "strict", + }, + operation: v1.Update, + clusterListerFails: true, + allowed: false, + }, + "ineffectual update allowed": { + oldSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + }, + newSetting: v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: "agent-tls-mode", + }, + }, + operation: v1.Update, + allowed: true, + }, + } + for name, tc := range tests { + name, tc := name, tc + t.Run(name, func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + clusterCache := fake.NewMockNonNamespacedCacheInterface[*v3.Cluster](ctrl) + _, force := tc.newSetting.Annotations["cattle.io/force"] + if tc.operation == v1.Update && !force && len(tc.clusters) > 0 { + clusterCache.EXPECT().List(gomock.Any()).Return(tc.clusters, nil) + } + if tc.clusterListerFails { + clusterCache.EXPECT().List(gomock.Any()).Return(tc.clusters, errors.New("some error")) + } + v := setting.NewValidator(clusterCache) + admitters := v.Admitters() + require.Len(t, admitters, 1) + + oldSetting, err := json.Marshal(tc.oldSetting) + require.NoError(t, err) + newSetting, err := json.Marshal(tc.newSetting) + require.NoError(t, err) + + res, err := admitters[0].Admit(&admission.Request{ + AdmissionRequest: v1.AdmissionRequest{ + Object: runtime.RawExtension{ + Raw: newSetting, + }, + OldObject: runtime.RawExtension{ + Raw: oldSetting, + }, + Operation: tc.operation, + }, + }) + require.NoError(t, err) + assert.Equal(t, tc.allowed, res.Allowed) + }) + } +} diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index bde40ab88..86f0546ff 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -47,7 +47,7 @@ func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandle nodeDriver := nodedriver.NewValidator(clients.Management.Node().Cache(), clients.Dynamic) projects := project.NewValidator(clients.Management.Cluster().Cache()) userAttribute := userattribute.NewValidator() - setting := setting.NewValidator() + setting := setting.NewValidator(clients.Management.Cluster().Cache()) handlers = append(handlers, psact, globalRoles, globalRoleBindings, prtbs, crtbs, roleTemplates, secrets, nodeDriver, projects, userAttribute, setting) } return handlers, nil