From 6b74a9a9006ccb27e74146a91899a02dd03a7d3b Mon Sep 17 00:00:00 2001 From: Peter Matseykanets Date: Thu, 19 Sep 2024 15:40:04 -0400 Subject: [PATCH] Validate creatorId and creator-principal-name annotations for cluster/project (#501) Ref: https://github.com/rancher/rancher/issues/46828 --- docs.md | 18 +- pkg/auth/escalation.go | 2 + pkg/codegen/main.go | 1 + .../management.cattle.io/v3/interface.go | 5 + .../management.cattle.io/v3/user.go | 208 +++++++++++++ pkg/resources/common/validation.go | 55 ++++ pkg/resources/common/validation_test.go | 211 ++++++++++++- .../v3/cluster/Cluster.md | 7 + .../v3/cluster/validator.go | 79 +++-- .../v3/cluster/validator_test.go | 158 +++++++++- .../v3/project/Project.md | 8 +- .../v3/project/validator.go | 19 +- .../v3/project/validator_test.go | 282 +++++++++++++++++- pkg/server/handlers.go | 16 +- 14 files changed, 1012 insertions(+), 57 deletions(-) create mode 100644 pkg/generated/controllers/management.cattle.io/v3/user.go create mode 100644 pkg/resources/management.cattle.io/v3/cluster/Cluster.md diff --git a/docs.md b/docs.md index 9305e635f..7d4fa821d 100644 --- a/docs.md +++ b/docs.md @@ -63,6 +63,16 @@ If yes, the webhook redacts the role, so that it only grants a deletion permissi # management.cattle.io/v3 +## Cluster + +### Validation Checks + +#### Annotations validation + +When a cluster is created and `field.cattle.io/creator-principal-name` annotation is set then `field.cattle.io/creatorId` annotation must be set as well. The value of `field.cattle.io/creator-principal-name` should match the creator's user principal id. + +When a cluster is updated `field.cattle.io/creator-principal-name` and `field.cattle.io/creatorId` annotations must stay the same or removed. + ## ClusterProxyConfig ### Validation Checks @@ -216,7 +226,7 @@ This admission webhook prevents the disabling or deletion of a NodeDriver if the #### ClusterName validation -ClusterName must be equal to the namespace, and must refer to an existing management.cattle.io/v3.Cluster object. In addition, users cannot update the field after creation. +ClusterName must be equal to the namespace, and must refer to an existing `management.cattle.io/v3.Cluster` object. In addition, users cannot update the field after creation. #### Protects system project @@ -233,6 +243,12 @@ The container default resource configuration must have properly formatted quanti Limits for any resource must not be less than requests. +#### Annotations validation + +When a project is created and `field.cattle.io/creator-principal-name` annotation is set then `field.cattle.io/creatorId` annotation must be set as well. The value of `field.cattle.io/creator-principal-name` should match the creator's user principal id. + +When a project is updated `field.cattle.io/creator-principal-name` and `field.cattle.io/creatorId` annotations must stay the same or removed. + ### Mutations #### On create diff --git a/pkg/auth/escalation.go b/pkg/auth/escalation.go index 50cf5cf34..d6d9df370 100644 --- a/pkg/auth/escalation.go +++ b/pkg/auth/escalation.go @@ -22,6 +22,8 @@ import ( const ( // CreatorIDAnn is an annotation key for the id of the creator. CreatorIDAnn = "field.cattle.io/creatorId" + // CreatorPrincipalNameAnn is an annotation key for the principal name of the creator. + CreatorPrincipalNameAnn = "field.cattle.io/creator-principal-name" ) // RequestUserHasVerb checks if the user associated with the context has a given verb on a given gvr for a specified name/namespace diff --git a/pkg/codegen/main.go b/pkg/codegen/main.go index 388840795..b30fe58bc 100644 --- a/pkg/codegen/main.go +++ b/pkg/codegen/main.go @@ -48,6 +48,7 @@ func main() { v3.ClusterProxyConfig{}, v3.Feature{}, v3.Setting{}, + v3.User{}, }, }, "provisioning.cattle.io": { diff --git a/pkg/generated/controllers/management.cattle.io/v3/interface.go b/pkg/generated/controllers/management.cattle.io/v3/interface.go index 87e5238dd..9286506ef 100644 --- a/pkg/generated/controllers/management.cattle.io/v3/interface.go +++ b/pkg/generated/controllers/management.cattle.io/v3/interface.go @@ -43,6 +43,7 @@ type Interface interface { ProjectRoleTemplateBinding() ProjectRoleTemplateBindingController RoleTemplate() RoleTemplateController Setting() SettingController + User() UserController } func New(controllerFactory controller.SharedControllerFactory) Interface { @@ -102,3 +103,7 @@ func (v *version) RoleTemplate() RoleTemplateController { func (v *version) Setting() SettingController { return generic.NewNonNamespacedController[*v3.Setting, *v3.SettingList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "Setting"}, "settings", v.controllerFactory) } + +func (v *version) User() UserController { + return generic.NewNonNamespacedController[*v3.User, *v3.UserList](schema.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "User"}, "users", v.controllerFactory) +} diff --git a/pkg/generated/controllers/management.cattle.io/v3/user.go b/pkg/generated/controllers/management.cattle.io/v3/user.go new file mode 100644 index 000000000..2eac82257 --- /dev/null +++ b/pkg/generated/controllers/management.cattle.io/v3/user.go @@ -0,0 +1,208 @@ +/* +Copyright 2024 Rancher Labs, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by codegen. DO NOT EDIT. + +package v3 + +import ( + "context" + "sync" + "time" + + v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" + "github.com/rancher/wrangler/v3/pkg/apply" + "github.com/rancher/wrangler/v3/pkg/condition" + "github.com/rancher/wrangler/v3/pkg/generic" + "github.com/rancher/wrangler/v3/pkg/kv" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// UserController interface for managing User resources. +type UserController interface { + generic.NonNamespacedControllerInterface[*v3.User, *v3.UserList] +} + +// UserClient interface for managing User resources in Kubernetes. +type UserClient interface { + generic.NonNamespacedClientInterface[*v3.User, *v3.UserList] +} + +// UserCache interface for retrieving User resources in memory. +type UserCache interface { + generic.NonNamespacedCacheInterface[*v3.User] +} + +// UserStatusHandler is executed for every added or modified User. Should return the new status to be updated +type UserStatusHandler func(obj *v3.User, status v3.UserStatus) (v3.UserStatus, error) + +// UserGeneratingHandler is the top-level handler that is executed for every User event. It extends UserStatusHandler by a returning a slice of child objects to be passed to apply.Apply +type UserGeneratingHandler func(obj *v3.User, status v3.UserStatus) ([]runtime.Object, v3.UserStatus, error) + +// RegisterUserStatusHandler configures a UserController to execute a UserStatusHandler for every events observed. +// If a non-empty condition is provided, it will be updated in the status conditions for every handler execution +func RegisterUserStatusHandler(ctx context.Context, controller UserController, condition condition.Cond, name string, handler UserStatusHandler) { + statusHandler := &userStatusHandler{ + client: controller, + condition: condition, + handler: handler, + } + controller.AddGenericHandler(ctx, name, generic.FromObjectHandlerToHandler(statusHandler.sync)) +} + +// RegisterUserGeneratingHandler configures a UserController to execute a UserGeneratingHandler for every events observed, passing the returned objects to the provided apply.Apply. +// If a non-empty condition is provided, it will be updated in the status conditions for every handler execution +func RegisterUserGeneratingHandler(ctx context.Context, controller UserController, apply apply.Apply, + condition condition.Cond, name string, handler UserGeneratingHandler, opts *generic.GeneratingHandlerOptions) { + statusHandler := &userGeneratingHandler{ + UserGeneratingHandler: handler, + apply: apply, + name: name, + gvk: controller.GroupVersionKind(), + } + if opts != nil { + statusHandler.opts = *opts + } + controller.OnChange(ctx, name, statusHandler.Remove) + RegisterUserStatusHandler(ctx, controller, condition, name, statusHandler.Handle) +} + +type userStatusHandler struct { + client UserClient + condition condition.Cond + handler UserStatusHandler +} + +// sync is executed on every resource addition or modification. Executes the configured handlers and sends the updated status to the Kubernetes API +func (a *userStatusHandler) sync(key string, obj *v3.User) (*v3.User, error) { + if obj == nil { + return obj, nil + } + + origStatus := obj.Status.DeepCopy() + obj = obj.DeepCopy() + newStatus, err := a.handler(obj, obj.Status) + if err != nil { + // Revert to old status on error + newStatus = *origStatus.DeepCopy() + } + + if a.condition != "" { + if errors.IsConflict(err) { + a.condition.SetError(&newStatus, "", nil) + } else { + a.condition.SetError(&newStatus, "", err) + } + } + if !equality.Semantic.DeepEqual(origStatus, &newStatus) { + if a.condition != "" { + // Since status has changed, update the lastUpdatedTime + a.condition.LastUpdated(&newStatus, time.Now().UTC().Format(time.RFC3339)) + } + + var newErr error + obj.Status = newStatus + newObj, newErr := a.client.UpdateStatus(obj) + if err == nil { + err = newErr + } + if newErr == nil { + obj = newObj + } + } + return obj, err +} + +type userGeneratingHandler struct { + UserGeneratingHandler + apply apply.Apply + opts generic.GeneratingHandlerOptions + gvk schema.GroupVersionKind + name string + seen sync.Map +} + +// Remove handles the observed deletion of a resource, cascade deleting every associated resource previously applied +func (a *userGeneratingHandler) Remove(key string, obj *v3.User) (*v3.User, error) { + if obj != nil { + return obj, nil + } + + obj = &v3.User{} + obj.Namespace, obj.Name = kv.RSplit(key, "/") + obj.SetGroupVersionKind(a.gvk) + + if a.opts.UniqueApplyForResourceVersion { + a.seen.Delete(key) + } + + return nil, generic.ConfigureApplyForObject(a.apply, obj, &a.opts). + WithOwner(obj). + WithSetID(a.name). + ApplyObjects() +} + +// Handle executes the configured UserGeneratingHandler and pass the resulting objects to apply.Apply, finally returning the new status of the resource +func (a *userGeneratingHandler) Handle(obj *v3.User, status v3.UserStatus) (v3.UserStatus, error) { + if !obj.DeletionTimestamp.IsZero() { + return status, nil + } + + objs, newStatus, err := a.UserGeneratingHandler(obj, status) + if err != nil { + return newStatus, err + } + if !a.isNewResourceVersion(obj) { + return newStatus, nil + } + + err = generic.ConfigureApplyForObject(a.apply, obj, &a.opts). + WithOwner(obj). + WithSetID(a.name). + ApplyObjects(objs...) + if err != nil { + return newStatus, err + } + a.storeResourceVersion(obj) + return newStatus, nil +} + +// isNewResourceVersion detects if a specific resource version was already successfully processed. +// Only used if UniqueApplyForResourceVersion is set in generic.GeneratingHandlerOptions +func (a *userGeneratingHandler) isNewResourceVersion(obj *v3.User) bool { + if !a.opts.UniqueApplyForResourceVersion { + return true + } + + // Apply once per resource version + key := obj.Namespace + "/" + obj.Name + previous, ok := a.seen.Load(key) + return !ok || previous != obj.ResourceVersion +} + +// storeResourceVersion keeps track of the latest resource version of an object for which Apply was executed +// Only used if UniqueApplyForResourceVersion is set in generic.GeneratingHandlerOptions +func (a *userGeneratingHandler) storeResourceVersion(obj *v3.User) { + if !a.opts.UniqueApplyForResourceVersion { + return + } + + key := obj.Namespace + "/" + obj.Name + a.seen.Store(key, obj.ResourceVersion) +} diff --git a/pkg/resources/common/validation.go b/pkg/resources/common/validation.go index 9c3e53df6..bc1fb7cfc 100644 --- a/pkg/resources/common/validation.go +++ b/pkg/resources/common/validation.go @@ -2,12 +2,15 @@ package common import ( "errors" + "fmt" "net/http" "github.com/rancher/webhook/pkg/admission" "github.com/rancher/webhook/pkg/auth" + controllerv3 "github.com/rancher/webhook/pkg/generated/controllers/management.cattle.io/v3" admissionv1 "k8s.io/api/admission/v1" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/apis/rbac" @@ -58,3 +61,55 @@ func ValidateRules(rules []rbacv1.PolicyRule, isNamespaced bool, fldPath *field. } return returnErr } + +var annotationsFieldPath = field.NewPath("metadata").Child("annotations") + +// CheckCreatorPrincipalName checks that if creator-principal-name annotation is set then creatorId annotation must be set as well. +// The value of creator-principal-name annotation should match the creator's user principal id. +func CheckCreatorPrincipalName(userCache controllerv3.UserCache, obj metav1.Object) (*field.Error, error) { + annotations := obj.GetAnnotations() + principalName := annotations[auth.CreatorPrincipalNameAnn] + if principalName == "" { // Nothing to check. + return nil, nil + } + + creatorID := annotations[auth.CreatorIDAnn] + if creatorID == "" { + return field.Invalid(annotationsFieldPath, auth.CreatorPrincipalNameAnn, fmt.Sprintf("annotation %s is required", auth.CreatorIDAnn)), nil + } + + user, err := userCache.Get(creatorID) + if err != nil { + if apierrors.IsNotFound(err) { + return field.Invalid(annotationsFieldPath, auth.CreatorPrincipalNameAnn, fmt.Sprintf("creator user %s doesn't exist", creatorID)), nil + } + return nil, fmt.Errorf("error getting creator user %s: %w", creatorID, err) + } + + for _, principal := range user.PrincipalIDs { + if principal == principalName { + return nil, nil + } + } + + return field.Invalid(annotationsFieldPath, auth.CreatorPrincipalNameAnn, fmt.Sprintf("creator user %s doesn't have principal %s", creatorID, principalName)), nil +} + +// CheckCreatorAnnotationsOnUpdate checks that the creatorId and creator-principal-name annotations are immutable. +// The only allowed update is removing the annotations. +// This function should only be called for the update operation. +func CheckCreatorAnnotationsOnUpdate(oldObj, newObj metav1.Object) *field.Error { + oldAnnotations := oldObj.GetAnnotations() + newAnnotations := newObj.GetAnnotations() + + for _, annotation := range []string{auth.CreatorIDAnn, auth.CreatorPrincipalNameAnn} { + if _, ok := newAnnotations[annotation]; ok { + // If the annotation exists on the new object it must be the same as on the old object. + if oldAnnotations[annotation] != newAnnotations[annotation] { + return field.Invalid(annotationsFieldPath, annotation, "annotation is immutable") + } + } + } + + return nil +} diff --git a/pkg/resources/common/validation_test.go b/pkg/resources/common/validation_test.go index 8b64c4e0d..63ae39ec7 100644 --- a/pkg/resources/common/validation_test.go +++ b/pkg/resources/common/validation_test.go @@ -1,11 +1,18 @@ package common import ( + "fmt" "testing" + "github.com/golang/mock/gomock" + v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" + "github.com/rancher/webhook/pkg/auth" + "github.com/rancher/wrangler/v3/pkg/generic/fake" "github.com/stretchr/testify/require" rbacv1 "k8s.io/api/rbac/v1" - v1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" ) @@ -60,22 +67,27 @@ func TestValidateRules(t *testing.T) { }, } - for _, testcase := range tests { - t.Run("global/"+testcase.name, func(t *testing.T) { - err := ValidateRules([]v1.PolicyRule{ - testcase.data, + for _, test := range tests { + test := test + t.Run("global/"+test.name, func(t *testing.T) { + t.Parallel() + + err := ValidateRules([]rbacv1.PolicyRule{ + test.data, }, false, gField) - if testcase.haserr { + if test.haserr { require.Error(t, err) return } require.NoError(t, err) }) - t.Run("namespaced/"+testcase.name, func(t *testing.T) { - err := ValidateRules([]v1.PolicyRule{ - testcase.data, + t.Run("namespaced/"+test.name, func(t *testing.T) { + t.Parallel() + + err := ValidateRules([]rbacv1.PolicyRule{ + test.data, }, true, nsField) - if testcase.haserr { + if test.haserr { require.Error(t, err) return } @@ -83,3 +95,182 @@ func TestValidateRules(t *testing.T) { }) } } + +func TestCheckCreatorPrincipalName(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + userCache := fake.NewMockNonNamespacedCacheInterface[*v3.User](ctrl) + userCache.EXPECT().Get(gomock.Any()).DoAndReturn(func(name string) (*v3.User, error) { + switch name { + case "u-12345": + return &v3.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "u-12345", + }, + PrincipalIDs: []string{"local://12345", "keycloak_user://12345"}, + }, nil + case "u-error": + return nil, fmt.Errorf("some error") + default: + return nil, apierrors.NewNotFound(schema.GroupResource{}, name) + } + }).AnyTimes() + + tests := []struct { + desc string + creatorID string + principalName string + fieldErr bool + err bool + }{ + { + desc: "no principal name annotation", + }, + { + desc: "creator id and principal name match", + creatorID: "u-12345", + principalName: "keycloak_user://12345", + }, + { + desc: "no creatorId annotation", + principalName: "keycloak_user://12345", + fieldErr: true, + }, + { + desc: "principal doesn't belong to creator", + creatorID: "u-12345", + principalName: "keycloak_user://12346", + fieldErr: true, + }, + { + desc: "creator user doesn't exist", + creatorID: "u-12346", + principalName: "keycloak_user://12345", + fieldErr: true, + }, + { + desc: "error getting creator user", + creatorID: "u-error", + principalName: "keycloak_user://12345", + err: true, + }, + } + + for _, test := range tests { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + annotations := map[string]string{} + if test.creatorID != "" { + annotations[auth.CreatorIDAnn] = test.creatorID + } + if test.principalName != "" { + annotations[auth.CreatorPrincipalNameAnn] = test.principalName + } + + fieldErr, err := CheckCreatorPrincipalName(userCache, &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: annotations, + }, + }) + require.Equal(t, test.fieldErr, fieldErr != nil) + require.Equal(t, test.err, err != nil) + }) + } +} + +func TestCheckCreatorAnnotationsOnUpdate(t *testing.T) { + t.Parallel() + + tests := []struct { + desc string + oldObj metav1.Object + newObj metav1.Object + fieldErr bool + }{ + { + desc: "no annotations", + oldObj: &v3.Project{}, + newObj: &v3.Project{}, + }, + { + desc: "no change", + oldObj: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + newObj: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + }, + { + desc: "annotations removed", + oldObj: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + newObj: &v3.Project{}, + }, + { + desc: "creator id changed", + oldObj: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + }, + }, + }, + newObj: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12346", + }, + }, + }, + fieldErr: true, + }, + { + desc: "principal name changed", + oldObj: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + newObj: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12346", + }, + }, + }, + fieldErr: true, + }, + } + + for _, test := range tests { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + fieldErr := CheckCreatorAnnotationsOnUpdate(test.oldObj, test.newObj) + require.Equal(t, test.fieldErr, fieldErr != nil) + }) + } +} diff --git a/pkg/resources/management.cattle.io/v3/cluster/Cluster.md b/pkg/resources/management.cattle.io/v3/cluster/Cluster.md new file mode 100644 index 000000000..46e830a61 --- /dev/null +++ b/pkg/resources/management.cattle.io/v3/cluster/Cluster.md @@ -0,0 +1,7 @@ +## Validation Checks + +### Annotations validation + +When a cluster is created and `field.cattle.io/creator-principal-name` annotation is set then `field.cattle.io/creatorId` annotation must be set as well. The value of `field.cattle.io/creator-principal-name` should match the creator's user principal id. + +When a cluster is updated `field.cattle.io/creator-principal-name` and `field.cattle.io/creatorId` annotations must stay the same or removed. diff --git a/pkg/resources/management.cattle.io/v3/cluster/validator.go b/pkg/resources/management.cattle.io/v3/cluster/validator.go index 7fac6d7ae..e5bbb959f 100644 --- a/pkg/resources/management.cattle.io/v3/cluster/validator.go +++ b/pkg/resources/management.cattle.io/v3/cluster/validator.go @@ -12,6 +12,7 @@ import ( v3 "github.com/rancher/webhook/pkg/generated/controllers/management.cattle.io/v3" objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3" psa "github.com/rancher/webhook/pkg/podsecurityadmission" + "github.com/rancher/webhook/pkg/resources/common" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" authenticationv1 "k8s.io/api/authentication/v1" @@ -23,15 +24,19 @@ import ( authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1" ) -var parsedRangeLessThan125 = semver.MustParseRange("< 1.25.0-rancher0") var parsedRangeLessThan123 = semver.MustParseRange("< 1.23.0-rancher0") // NewValidator returns a new validator for management clusters. -func NewValidator(sar authorizationv1.SubjectAccessReviewInterface, cache v3.PodSecurityAdmissionConfigurationTemplateCache) *Validator { +func NewValidator( + sar authorizationv1.SubjectAccessReviewInterface, + cache v3.PodSecurityAdmissionConfigurationTemplateCache, + userCache v3.UserCache, +) *Validator { return &Validator{ admitter: admitter{ - sar: sar, - psact: cache, + sar: sar, + psact: cache, + userCache: userCache, // userCache is nil for downstream clusters. }, } } @@ -64,13 +69,19 @@ func (v *Validator) Admitters() []admission.Admitter { } type admitter struct { - sar authorizationv1.SubjectAccessReviewInterface - psact v3.PodSecurityAdmissionConfigurationTemplateCache + sar authorizationv1.SubjectAccessReviewInterface + psact v3.PodSecurityAdmissionConfigurationTemplateCache + userCache v3.UserCache } // Admit handles the webhook admission request sent to this webhook. func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) { - response, err := a.validateFleetPermissions(request) + oldCluster, newCluster, err := objectsv3.ClusterOldAndNewFromRequest(&request.AdmissionRequest) + if err != nil { + return nil, fmt.Errorf("failed get old and new clusters from request: %w", err) + } + + response, err := a.validateFleetPermissions(request, oldCluster, newCluster) if err != nil { return nil, fmt.Errorf("failed to validate fleet permissions: %w", err) } @@ -78,26 +89,38 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp return response, nil } - if request.Operation == admissionv1.Create || request.Operation == admissionv1.Update { - cluster, err := objectsv3.ClusterFromRequest(&request.AdmissionRequest) - if err != nil { - return nil, fmt.Errorf("failed to get cluster from request: %w", err) + if a.userCache != nil { + // The following checks don't make sense for downstream clusters (userCache == nil) + if request.Operation == admissionv1.Create { + fieldErr, err := common.CheckCreatorPrincipalName(a.userCache, newCluster) + if err != nil { + return nil, fmt.Errorf("error checking creator principal: %w", err) + } + if fieldErr != nil { + return admission.ResponseBadRequest(fieldErr.Error()), nil + } + } else if request.Operation == admissionv1.Update { + if fieldErr := common.CheckCreatorAnnotationsOnUpdate(oldCluster, newCluster); fieldErr != nil { + return admission.ResponseBadRequest(fieldErr.Error()), nil + } } + } + + if request.Operation == admissionv1.Create || request.Operation == admissionv1.Update { // no need to validate the local cluster, or imported cluster which represents a KEv2 cluster (GKE/EKS/AKS) or v1 Provisioning Cluster - if cluster.Name == "local" || cluster.Spec.RancherKubernetesEngineConfig == nil { + if newCluster.Name == "local" || newCluster.Spec.RancherKubernetesEngineConfig == nil { return admission.ResponseAllowed(), nil } - response, err = a.validatePSACT(request) + + response, err = a.validatePSACT(oldCluster, newCluster, request.Operation) if err != nil { return nil, fmt.Errorf("failed to validate PodSecurityAdmissionConfigurationTemplate(PSACT): %w", err) } if !response.Allowed { return response, nil } - if !response.Allowed { - return response, nil - } } + return admission.ResponseAllowed(), nil } @@ -110,12 +133,7 @@ func toExtra(extra map[string]authenticationv1.ExtraValue) map[string]v1.ExtraVa } // validateFleetPermissions validates whether the request maker has required permissions around FleetWorkspace. -func (a *admitter) validateFleetPermissions(request *admission.Request) (*admissionv1.AdmissionResponse, error) { - oldCluster, newCluster, err := objectsv3.ClusterOldAndNewFromRequest(&request.AdmissionRequest) - if err != nil { - return nil, fmt.Errorf("failed to get old and new clusters from request: %w", err) - } - +func (a *admitter) validateFleetPermissions(request *admission.Request, oldCluster, newCluster *apisv3.Cluster) (*admissionv1.AdmissionResponse, error) { // Ensure that the FleetWorkspaceName field cannot be unset once it is set, as it would cause (likely unintentional) // cluster deletion. Note that we're only enforcing this rule on UPDATE because Spec.FleetWorkspaceName will be // empty on cluster deletion, which is fine. @@ -169,24 +187,24 @@ func (a *admitter) validateFleetPermissions(request *admission.Request) (*admiss Allowed: false, }, nil } + return admission.ResponseAllowed(), nil } // validatePSACT validates the cluster spec when PodSecurityAdmissionConfigurationTemplate is used. -func (a *admitter) validatePSACT(request *admission.Request) (*admissionv1.AdmissionResponse, error) { - oldCluster, newCluster, err := objectsv3.ClusterOldAndNewFromRequest(&request.AdmissionRequest) - if err != nil { - return nil, fmt.Errorf("failed to get old and new clusters from request: %w", err) - } +func (a *admitter) validatePSACT(oldCluster, newCluster *apisv3.Cluster, op admissionv1.Operation) (*admissionv1.AdmissionResponse, error) { newTemplateName := newCluster.Spec.DefaultPodSecurityAdmissionConfigurationTemplateName oldTemplateName := oldCluster.Spec.DefaultPodSecurityAdmissionConfigurationTemplateName + parsedVersion, err := psa.GetClusterVersion(newCluster.Spec.RancherKubernetesEngineConfig.Version) if err != nil { return nil, fmt.Errorf("failed to parse cluster version: %w", err) } + if parsedRangeLessThan123(parsedVersion) && newTemplateName != "" { return admission.ResponseBadRequest("PodSecurityAdmissionConfigurationTemplate(PSACT) is only supported in k8s version 1.23 and above"), nil } + if newTemplateName != "" { response, err := a.checkPSAConfigOnCluster(newCluster) if err != nil { @@ -196,7 +214,7 @@ func (a *admitter) validatePSACT(request *admission.Request) (*admissionv1.Admis return response, nil } } else { - switch request.Operation { + switch op { case admissionv1.Create: return admission.ResponseAllowed(), nil case admissionv1.Update: @@ -219,6 +237,7 @@ func (a *admitter) validatePSACT(request *admission.Request) (*admissionv1.Admis } } } + return admission.ResponseAllowed(), nil } @@ -239,6 +258,7 @@ func (a *admitter) checkPSAConfigOnCluster(cluster *apisv3.Cluster) (*admissionv } return nil, fmt.Errorf("failed to get PodSecurityAdmissionConfigurationTemplate [%s]: %w", name, err) } + fromTemplate, err := psa.GetPluginConfigFromTemplate(template, cluster.Spec.RancherKubernetesEngineConfig.Version) if err != nil { return nil, fmt.Errorf("failed to get the PluginConfig: %w", err) @@ -247,6 +267,7 @@ func (a *admitter) checkPSAConfigOnCluster(cluster *apisv3.Cluster) (*admissionv if !found { return admission.ResponseBadRequest("PodSecurity Configuration is not found under kube-api.admission_configuration"), nil } + var psaConfig, psaConfig2 any err = json.Unmarshal(fromTemplate.Configuration.Raw, &psaConfig) if err != nil { @@ -256,9 +277,11 @@ func (a *admitter) checkPSAConfigOnCluster(cluster *apisv3.Cluster) (*admissionv if err != nil { return nil, fmt.Errorf("failed to unmarshal PodSecurityConfiguration from admissionConfig: %w", err) } + if !equality.Semantic.DeepEqual(psaConfig, psaConfig2) { return admission.ResponseBadRequest("PodSecurity Configuration under kube-api.admission_configuration " + "does not match the content of the PodSecurityAdmissionConfigurationTemplate"), nil } + return admission.ResponseAllowed(), nil } diff --git a/pkg/resources/management.cattle.io/v3/cluster/validator_test.go b/pkg/resources/management.cattle.io/v3/cluster/validator_test.go index 58896bda7..b819b2865 100644 --- a/pkg/resources/management.cattle.io/v3/cluster/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/cluster/validator_test.go @@ -5,13 +5,18 @@ import ( "encoding/json" "testing" + "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/auth" + "github.com/rancher/wrangler/v3/pkg/generic/fake" "github.com/stretchr/testify/assert" admissionv1 "k8s.io/api/admission/v1" authorizationv1 "k8s.io/api/authorization/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" v1 "k8s.io/client-go/kubernetes/typed/authorization/v1" ) @@ -32,6 +37,21 @@ func (m *mockReviewer) Create( } func TestAdmit(t *testing.T) { + ctrl := gomock.NewController(t) + userCache := fake.NewMockNonNamespacedCacheInterface[*v3.User](ctrl) + userCache.EXPECT().Get(gomock.Any()).DoAndReturn(func(name string) (*v3.User, error) { + if name == "u-12345" { + return &v3.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "u-12345", + }, + PrincipalIDs: []string{"keycloak_user://12345"}, + }, nil + } + + return nil, apierrors.NewNotFound(schema.GroupResource{}, name) + }).AnyTimes() + tests := []struct { name string oldCluster v3.Cluster @@ -45,6 +65,49 @@ func TestAdmit(t *testing.T) { operation: admissionv1.Create, expectAllowed: true, }, + { + name: "Create with creator principal", + newCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + operation: admissionv1.Create, + expectAllowed: true, + }, + { + name: "Create with creator principal but no creator id", + newCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + operation: admissionv1.Create, + expectAllowed: false, + expectedReason: metav1.StatusReasonBadRequest, + }, + { + name: "Create with creator principal and non-existent creator id", + newCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12346", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + operation: admissionv1.Create, + expectAllowed: false, + expectedReason: metav1.StatusReasonBadRequest, + }, { name: "UpdateWithUnsetFleetWorkspaceName", oldCluster: v3.Cluster{Spec: v3.ClusterSpec{FleetWorkspaceName: "fleet-default"}}, @@ -66,6 +129,94 @@ func TestAdmit(t *testing.T) { operation: admissionv1.Update, expectAllowed: true, }, + { + name: "Update changing creator id annotation", + oldCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + }, + }, + }, + newCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12346", + }, + }, + }, + operation: admissionv1.Update, + expectAllowed: false, + expectedReason: metav1.StatusReasonBadRequest, + }, + { + name: "Update changing principle name annotation", + oldCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + newCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12346", + }, + }, + }, + operation: admissionv1.Update, + expectAllowed: false, + expectedReason: metav1.StatusReasonBadRequest, + }, + { + name: "Update removing creator annotations", + oldCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + newCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + }, + }, + operation: admissionv1.Update, + expectAllowed: true, + expectedReason: metav1.StatusReasonBadRequest, + }, + { + name: "Update without changing creator annotations", + oldCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + newCluster: v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "c-2bmj5", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + }, + operation: admissionv1.Update, + expectAllowed: true, + expectedReason: metav1.StatusReasonBadRequest, + }, { name: "Delete", oldCluster: v3.Cluster{Spec: v3.ClusterSpec{FleetWorkspaceName: "fleet-default"}}, @@ -78,7 +229,8 @@ func TestAdmit(t *testing.T) { t.Run(tt.name, func(t *testing.T) { v := &Validator{ admitter: admitter{ - sar: &mockReviewer{}, + sar: &mockReviewer{}, + userCache: userCache, }, } @@ -105,7 +257,9 @@ func TestAdmit(t *testing.T) { assert.Equal(t, tt.expectAllowed, res.Allowed) if !tt.expectAllowed { - assert.Equal(t, tt.expectedReason, res.Result.Reason) + if tt.expectedReason != "" { + assert.Equal(t, tt.expectedReason, res.Result.Reason) + } } }) } diff --git a/pkg/resources/management.cattle.io/v3/project/Project.md b/pkg/resources/management.cattle.io/v3/project/Project.md index e428bdb50..d3bb2837b 100644 --- a/pkg/resources/management.cattle.io/v3/project/Project.md +++ b/pkg/resources/management.cattle.io/v3/project/Project.md @@ -2,7 +2,7 @@ ### ClusterName validation -ClusterName must be equal to the namespace, and must refer to an existing management.cattle.io/v3.Cluster object. In addition, users cannot update the field after creation. +ClusterName must be equal to the namespace, and must refer to an existing `management.cattle.io/v3.Cluster` object. In addition, users cannot update the field after creation. ### Protects system project @@ -19,6 +19,12 @@ The container default resource configuration must have properly formatted quanti Limits for any resource must not be less than requests. +### Annotations validation + +When a project is created and `field.cattle.io/creator-principal-name` annotation is set then `field.cattle.io/creatorId` annotation must be set as well. The value of `field.cattle.io/creator-principal-name` should match the creator's user principal id. + +When a project is updated `field.cattle.io/creator-principal-name` and `field.cattle.io/creatorId` annotations must stay the same or removed. + ## Mutations ### On create diff --git a/pkg/resources/management.cattle.io/v3/project/validator.go b/pkg/resources/management.cattle.io/v3/project/validator.go index 7c59519b7..8e78c0bba 100644 --- a/pkg/resources/management.cattle.io/v3/project/validator.go +++ b/pkg/resources/management.cattle.io/v3/project/validator.go @@ -10,6 +10,7 @@ import ( "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" + "github.com/rancher/webhook/pkg/resources/common" "github.com/rancher/wrangler/v3/pkg/data/convert" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" @@ -37,10 +38,11 @@ type Validator struct { } // NewValidator returns a project validator. -func NewValidator(clusterCache controllerv3.ClusterCache) *Validator { +func NewValidator(clusterCache controllerv3.ClusterCache, userCache controllerv3.UserCache) *Validator { return &Validator{ admitter: admitter{ clusterCache: clusterCache, + userCache: userCache, }, } } @@ -72,6 +74,7 @@ func (v *Validator) Admitters() []admission.Admitter { type admitter struct { clusterCache controllerv3.ClusterCache + userCache controllerv3.UserCache } // Admit handles the webhook admission request sent to this webhook. @@ -111,6 +114,15 @@ func (a *admitter) admitCreate(project *v3.Project) (*admissionv1.AdmissionRespo if fieldErr != nil { return admission.ResponseBadRequest(fieldErr.Error()), nil } + + fieldErr, err = common.CheckCreatorPrincipalName(a.userCache, project) + if err != nil { + return nil, fmt.Errorf("error checking creator principal: %w", err) + } + if fieldErr != nil { + return admission.ResponseBadRequest(fieldErr.Error()), nil + } + return a.admitCommonCreateUpdate(nil, project) } @@ -119,6 +131,11 @@ func (a *admitter) admitUpdate(oldProject, newProject *v3.Project) (*admissionv1 fieldErr := field.Invalid(projectSpecFieldPath.Child(clusterNameField), newProject.Spec.ClusterName, "field is immutable") return admission.ResponseBadRequest(fieldErr.Error()), nil } + + if fieldErr := common.CheckCreatorAnnotationsOnUpdate(oldProject, newProject); fieldErr != nil { + return admission.ResponseBadRequest(fieldErr.Error()), nil + } + return a.admitCommonCreateUpdate(oldProject, newProject) } diff --git a/pkg/resources/management.cattle.io/v3/project/validator_test.go b/pkg/resources/management.cattle.io/v3/project/validator_test.go index 82273b65f..4835fbd97 100644 --- a/pkg/resources/management.cattle.io/v3/project/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/project/validator_test.go @@ -10,10 +10,10 @@ import ( "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/auth" "github.com/rancher/wrangler/v3/pkg/generic/fake" "github.com/stretchr/testify/assert" admissionv1 "k8s.io/api/admission/v1" - v1 "k8s.io/api/admission/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -24,10 +24,11 @@ func TestProjectValidation(t *testing.T) { t.Parallel() type testState struct { clusterCache *fake.MockNonNamespacedCacheInterface[*v3.Cluster] + userCache *fake.MockNonNamespacedCacheInterface[*v3.User] } tests := []struct { name string - operation v1.Operation + operation admissionv1.Operation stateSetup func(state *testState) newProject *v3.Project oldProject *v3.Project @@ -43,7 +44,7 @@ func TestProjectValidation(t *testing.T) { }, { name: "no clusterName", - operation: v1.Create, + operation: admissionv1.Create, newProject: &v3.Project{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -58,7 +59,7 @@ func TestProjectValidation(t *testing.T) { }, { name: "clusterName doesn't match namespace", - operation: v1.Create, + operation: admissionv1.Create, newProject: &v3.Project{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -74,7 +75,7 @@ func TestProjectValidation(t *testing.T) { }, { name: "clusterName not found", - operation: v1.Create, + operation: admissionv1.Create, newProject: &v3.Project{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -97,7 +98,7 @@ func TestProjectValidation(t *testing.T) { }, { name: "error when validating if the cluster exists", - operation: v1.Create, + operation: admissionv1.Create, newProject: &v3.Project{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -117,7 +118,7 @@ func TestProjectValidation(t *testing.T) { }, { name: "nil return from the cluster cache", - operation: v1.Create, + operation: admissionv1.Create, newProject: &v3.Project{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -406,6 +407,146 @@ func TestProjectValidation(t *testing.T) { }, wantAllowed: false, }, + { + name: "create with principal name annotation", + operation: admissionv1.Create, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + auth.CreatorIDAnn: "u-12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + stateSetup: func(state *testState) { + state.clusterCache.EXPECT().Get("testcluster").Return(&v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster", + }, + }, nil) + state.userCache.EXPECT().Get("u-12345").Return(&v3.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "u-12345", + }, + PrincipalIDs: []string{"local://u-12345", "keycloak_user://12345"}, + }, nil) + }, + wantAllowed: true, + }, + { + name: "create with principal name annotation but no creator id", + operation: admissionv1.Create, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + stateSetup: func(state *testState) { + state.clusterCache.EXPECT().Get("testcluster").Return(&v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster", + }, + }, nil) + state.userCache.EXPECT().Get("u-12345").Times(0) + }, + wantAllowed: false, + }, + { + name: "create with principal name annotation that doesn't belong to creator id", + operation: admissionv1.Create, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12346", + auth.CreatorIDAnn: "u-12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + stateSetup: func(state *testState) { + state.clusterCache.EXPECT().Get("testcluster").Return(&v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster", + }, + }, nil) + state.userCache.EXPECT().Get("u-12345").Return(&v3.User{ + ObjectMeta: metav1.ObjectMeta{ + Name: "u-12345", + }, + PrincipalIDs: []string{"local://u-12345", "keycloak_user://12345"}, + }, nil) + }, + wantAllowed: false, + }, + { + name: "create with principal name annotation but creator doesn't exist", + operation: admissionv1.Create, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12346", + auth.CreatorIDAnn: "u-12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + stateSetup: func(state *testState) { + state.clusterCache.EXPECT().Get("testcluster").Return(&v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster", + }, + }, nil) + state.userCache.EXPECT().Get("u-12345").Return(nil, apierrors.NewNotFound(schema.GroupResource{}, "u-12345")) + }, + wantAllowed: false, + }, + { + name: "create with principal name annotation but error getting creator id", + operation: admissionv1.Create, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12346", + auth.CreatorIDAnn: "u-12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + stateSetup: func(state *testState) { + state.clusterCache.EXPECT().Get("testcluster").Return(&v3.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testcluster", + }, + }, nil) + state.userCache.EXPECT().Get("u-12345").Return(nil, fmt.Errorf("some error")) + }, + wantAllowed: false, + wantErr: true, + }, { name: "update with no quotas (noop)", operation: admissionv1.Update, @@ -878,6 +1019,122 @@ func TestProjectValidation(t *testing.T) { }, wantAllowed: false, }, + { + name: "update changing creator id annotation", + operation: admissionv1.Update, + oldProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "tescluster", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12346", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + wantAllowed: false, + }, + { + name: "update changing principle name annotation", + operation: admissionv1.Update, + oldProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "tescluster", + Annotations: map[string]string{ + auth.CreatorPrincipalNameAnn: "keycloak_user://12346", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + wantAllowed: false, + }, + { + name: "update removing creator annotations", + operation: admissionv1.Update, + oldProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "tescluster", + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + wantAllowed: true, + }, + { + name: "update without changing creator annotations", + operation: admissionv1.Update, + oldProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "tescluster", + Annotations: map[string]string{ + auth.CreatorIDAnn: "u-12345", + auth.CreatorPrincipalNameAnn: "keycloak_user://12345", + }, + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + }, + wantAllowed: true, + }, { name: "invalid operation", operation: admissionv1.Connect, @@ -910,13 +1167,14 @@ func TestProjectValidation(t *testing.T) { ctrl := gomock.NewController(t) state := testState{ clusterCache: fake.NewMockNonNamespacedCacheInterface[*v3.Cluster](ctrl), + userCache: fake.NewMockNonNamespacedCacheInterface[*v3.User](ctrl), } if test.stateSetup != nil { test.stateSetup(&state) } req, err := createProjectRequest(test.oldProject, test.newProject, test.operation, false) assert.NoError(t, err) - validator := NewValidator(state.clusterCache) + validator := NewValidator(state.clusterCache, state.userCache) admitters := validator.Admitters() assert.Len(t, admitters, 1) response, err := admitters[0].Admit(req) @@ -937,7 +1195,7 @@ func TestProjectContainerDefaultLimitsValidation(t *testing.T) { } tests := []struct { name string - operation v1.Operation + operation admissionv1.Operation limit *v3.ContainerResourceLimit wantAllowed bool }{ @@ -1119,7 +1377,7 @@ func TestProjectContainerDefaultLimitsValidation(t *testing.T) { } req, err := createProjectRequest(oldProject, newProject, test.operation, false) assert.NoError(t, err) - validator := NewValidator(state.clusterCache) + validator := NewValidator(state.clusterCache, nil) admitters := validator.Admitters() assert.Len(t, admitters, 1) response, err := admitters[0].Admit(req) @@ -1130,7 +1388,7 @@ func TestProjectContainerDefaultLimitsValidation(t *testing.T) { } } -func createProjectRequest(oldProject, newProject *v3.Project, operation v1.Operation, dryRun bool) (*admission.Request, error) { +func createProjectRequest(oldProject, newProject *v3.Project, operation admissionv1.Operation, dryRun bool) (*admission.Request, error) { gvk := metav1.GroupVersionKind{Group: "management.cattle.io", Version: "v3", Kind: "Project"} gvr := metav1.GroupVersionResource{Group: "management.cattle.io", Version: "v3", Resource: "projects"} req := &admission.Request{ @@ -1139,7 +1397,7 @@ func createProjectRequest(oldProject, newProject *v3.Project, operation v1.Opera if oldProject == nil && newProject == nil { return req, nil } - req.AdmissionRequest = v1.AdmissionRequest{ + req.AdmissionRequest = admissionv1.AdmissionRequest{ Kind: gvk, Resource: gvr, RequestKind: &gvk, diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index 8ab7ca11c..8ee2abba4 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -3,6 +3,7 @@ package server import ( "github.com/rancher/webhook/pkg/admission" "github.com/rancher/webhook/pkg/clients" + v3 "github.com/rancher/webhook/pkg/generated/controllers/management.cattle.io/v3" "github.com/rancher/webhook/pkg/resolvers" "github.com/rancher/webhook/pkg/resources/catalog.cattle.io/v1/clusterrepo" nshandler "github.com/rancher/webhook/pkg/resources/core/v1/namespace" @@ -31,9 +32,20 @@ import ( // Validation returns a list of all ValidatingAdmissionHandlers used by the webhook. func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandler, error) { + + var userCache v3.UserCache + if clients.MultiClusterManagement { + userCache = clients.Management.User().Cache() + } + clusters := managementCluster.NewValidator( + clients.K8s.AuthorizationV1().SubjectAccessReviews(), + clients.Management.PodSecurityAdmissionConfigurationTemplate().Cache(), + userCache, + ) + handlers := []admission.ValidatingAdmissionHandler{ feature.NewValidator(), - managementCluster.NewValidator(clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.Management.PodSecurityAdmissionConfigurationTemplate().Cache()), + clusters, provisioningCluster.NewProvisioningClusterValidator(clients), machineconfig.NewValidator(), nshandler.NewValidator(clients.K8s.AuthorizationV1().SubjectAccessReviews()), @@ -52,7 +64,7 @@ func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandle roleTemplates := roletemplate.NewValidator(clients.DefaultResolver, clients.RoleTemplateResolver, clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.Management.GlobalRole().Cache()) secrets := secret.NewValidator(clients.RBAC.Role().Cache(), clients.RBAC.RoleBinding().Cache()) nodeDriver := nodedriver.NewValidator(clients.Management.Node().Cache(), clients.Dynamic) - projects := project.NewValidator(clients.Management.Cluster().Cache()) + projects := project.NewValidator(clients.Management.Cluster().Cache(), clients.Management.User().Cache()) roles := role.NewValidator() rolebindings := rolebinding.NewValidator() setting := setting.NewValidator(clients.Management.Cluster().Cache(), clients.Management.Setting().Cache())