From e72261052ced43397706d84dc68e124eaa24924e Mon Sep 17 00:00:00 2001 From: Jonathan Crowther Date: Wed, 9 Apr 2025 12:38:58 -0400 Subject: [PATCH 1/3] Use backing namespace for projects (#869) * Use backing namespace for projects * Move to caches in project mutator * Fix error message * Switch client names to cache --- docs.md | 13 +- go.mod | 15 +- go.sum | 14 +- .../v3/project/Project.md | 12 ++ .../v3/project/mutator.go | 88 ++++++++- .../v3/project/mutator_test.go | 186 ++++++++++++------ .../v3/project/validator.go | 16 +- .../ProjectRoleTemplateBinding.md | 1 - .../projectroletemplatebinding/validator.go | 3 - .../validator_test.go | 16 -- pkg/server/handlers.go | 2 +- 11 files changed, 254 insertions(+), 112 deletions(-) diff --git a/docs.md b/docs.md index 69f5e15ed..9c0eca61a 100644 --- a/docs.md +++ b/docs.md @@ -188,6 +188,9 @@ This admission webhook prevents the disabling or deletion of a NodeDriver if the 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. +#### BackingNamespace validation +The BackingNamespace field cannot be changed once set. Projects without the BackingNamespace field can have it added. + #### Protects system project The system project cannot be deleted. @@ -200,8 +203,17 @@ Project quotas and default limits must be consistent with one another and must b #### On create +Populates the BackingNamespace field by concatenating `Project.ClusterName` and `Project.Name`. + +If the project is using a generated name (ie `GenerateName` is not empty), the generation happens within the mutating webhook. +The reason for this is that the BackingNamespace is made up of the `Project.Name`, and name generation happens after mutating webhooks and before validating webhooks. + Adds the authz.management.cattle.io/creator-role-bindings annotation. +#### On update + +If the BackingNamespace field is empty, populate the BackingNamespace field with the project name. + ## ProjectRoleTemplateBinding ### Validation Checks @@ -220,7 +232,6 @@ Users cannot create ProjectRoleTemplateBindings that violate the following const - The `ProjectName` field must be: - Provided as a non-empty value - Specified using the format of `clusterName:projectName`; `clusterName` is the `metadata.name` of a cluster, and `projectName` is the `metadata.name` of a project - - The `projectName` part of the field must match the namespace of the ProjectRoleTemplateBinding - Refer to a valid project and cluster (both must exist and project.Spec.ClusterName must equal the cluster) - Either a user subject (through `UserName` or `UserPrincipalName`), or a group subject (through `GroupName` or `GroupPrincipalName`), or a service account subject (through `ServiceAccount`) must be specified. Exactly one diff --git a/go.mod b/go.mod index 73f64565e..1cc06611d 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,8 @@ module github.com/rancher/webhook -go 1.22.0 +go 1.23.0 -toolchain go1.22.7 +toolchain go1.24.2 replace ( github.com/rancher/rke => github.com/rancher/rke v1.5.13 @@ -45,8 +45,9 @@ require ( github.com/gorilla/mux v1.8.1 github.com/rancher/dynamiclistener v0.4.0 github.com/rancher/lasso v0.1.0 - github.com/rancher/rancher/pkg/apis v0.0.0-20250317215908-5cc17ed521b4 + github.com/rancher/rancher/pkg/apis v0.0.0-20250411213817-1116d03f0676 github.com/rancher/rke v1.5.15 + github.com/rancher/wrangler v1.1.1 github.com/rancher/wrangler/v2 v2.1.4 github.com/robfig/cron v1.2.0 github.com/sirupsen/logrus v1.9.3 @@ -56,7 +57,7 @@ require ( golang.org/x/tools v0.28.0 k8s.io/api v0.30.0 k8s.io/apimachinery v0.30.0 - k8s.io/apiserver v0.28.9 + k8s.io/apiserver v0.28.12 k8s.io/client-go v12.0.0+incompatible k8s.io/kubernetes v1.28.9 k8s.io/pod-security-admission v0.28.6 @@ -141,8 +142,8 @@ require ( golang.org/x/net v0.34.0 // indirect golang.org/x/oauth2 v0.25.0 // indirect golang.org/x/sync v0.11.0 // indirect - golang.org/x/sys v0.29.0 // indirect - golang.org/x/term v0.28.0 // indirect + golang.org/x/sys v0.30.0 // indirect + golang.org/x/term v0.29.0 // indirect golang.org/x/time v0.5.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/genproto v0.0.0-20231106174013-bbf56f31fb17 // indirect @@ -157,7 +158,7 @@ require ( k8s.io/apiextensions-apiserver v0.28.9 // indirect k8s.io/cloud-provider v0.0.0 // indirect k8s.io/code-generator v0.28.9 // indirect - k8s.io/component-base v0.28.9 // indirect + k8s.io/component-base v0.28.12 // indirect k8s.io/component-helpers v0.28.6 // indirect k8s.io/controller-manager v0.28.6 // indirect k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01 // indirect diff --git a/go.sum b/go.sum index 623cc9e8b..fbc1c9866 100644 --- a/go.sum +++ b/go.sum @@ -298,10 +298,12 @@ github.com/rancher/lasso v0.1.0 h1:LOCNtL7GvyTp3KTyLiFE2JKmxyX2xF6HeFmtd0pvMHA= github.com/rancher/lasso v0.1.0/go.mod h1:pYKOe2r/5O0w3ypoc7xHQF8LvWCp5PsNRea1Jpq3vBU= github.com/rancher/norman v0.2.0 h1:PFIiHel0i0OFNZoygIS4oxJV6gRi1tffudzCqDnKLlA= github.com/rancher/norman v0.2.0/go.mod h1:WbNpu/HwChwKk54W0rWBdioxYVVZwVVz//UX84m/NvY= -github.com/rancher/rancher/pkg/apis v0.0.0-20250317215908-5cc17ed521b4 h1:9ioXT/yLX4RwwryQCD8IEgezERVZBUxf6JWrHK+Fj2M= -github.com/rancher/rancher/pkg/apis v0.0.0-20250317215908-5cc17ed521b4/go.mod h1:KsweP8b8wqois/Pke9C2aw/ExIXBdqFg1/UUE0Gl5g8= +github.com/rancher/rancher/pkg/apis v0.0.0-20250411213817-1116d03f0676 h1:ykXjqtx8TwCeZcGmb/YZ3+pLSPWPbxGvsEnmtlYEMqs= +github.com/rancher/rancher/pkg/apis v0.0.0-20250411213817-1116d03f0676/go.mod h1:8AfqZ7rm9zNt3F73883QnPoT9tGSM5g3QK62pNd1Vts= github.com/rancher/rke v1.5.13 h1:Y7e3qI0G2HbU3Vm5k3Sqeq+UR2w114K5yY6wT9VFKcY= github.com/rancher/rke v1.5.13/go.mod h1:/z9oyKqYpFwgRBV9rfLxqUdjydz/VMCTcjld4uUt7uM= +github.com/rancher/wrangler v1.1.1 h1:wmqUwqc2M7ADfXnBCJTFkTB5ZREWpD78rnZMzmxwMvM= +github.com/rancher/wrangler v1.1.1/go.mod h1:ioVbKupzcBOdzsl55MvEDN0R1wdGggj8iNCYGFI5JvM= github.com/rancher/wrangler/v2 v2.1.4 h1:ohov0i6A9dJHHO6sjfsH4Dqv93ZTdm5lIJVJdPzVdQc= github.com/rancher/wrangler/v2 v2.1.4/go.mod h1:af5OaGU/COgreQh1mRbKiUI64draT2NN34uk+PALFY8= github.com/robfig/cron v1.2.0 h1:ZjScXvvxeQ63Dbyxy76Fj3AT3Ut0aKsyd2/tl3DTMuQ= @@ -514,11 +516,11 @@ golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU= -golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= +golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.28.0 h1:/Ts8HFuMR2E6IP/jlo7QVLZHggjKQbhu/7H0LJFr3Gg= -golang.org/x/term v0.28.0/go.mod h1:Sw/lC2IAUZ92udQNf3WodGtn4k/XoLyZoh8v/8uiwek= +golang.org/x/term v0.29.0 h1:L6pJp37ocefwRRtYPKSWOWzOtWSxVajvz2ldH/xi3iU= +golang.org/x/term v0.29.0/go.mod h1:6bl4lRlvVuDgSf3179VpIxBF0o10JUpXWOnI7nErv7s= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/pkg/resources/management.cattle.io/v3/project/Project.md b/pkg/resources/management.cattle.io/v3/project/Project.md index faeaab53f..e3208dc51 100644 --- a/pkg/resources/management.cattle.io/v3/project/Project.md +++ b/pkg/resources/management.cattle.io/v3/project/Project.md @@ -4,6 +4,9 @@ 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. +### BackingNamespace validation +The BackingNamespace field cannot be changed once set. Projects without the BackingNamespace field can have it added. + ### Protects system project The system project cannot be deleted. @@ -16,4 +19,13 @@ Project quotas and default limits must be consistent with one another and must b ### On create +Populates the BackingNamespace field by concatenating `Project.ClusterName` and `Project.Name`. + +If the project is using a generated name (ie `GenerateName` is not empty), the generation happens within the mutating webhook. +The reason for this is that the BackingNamespace is made up of the `Project.Name`, and name generation happens after mutating webhooks and before validating webhooks. + Adds the authz.management.cattle.io/creator-role-bindings annotation. + +### On update + +If the BackingNamespace field is empty, populate the BackingNamespace field with the project name. \ No newline at end of file diff --git a/pkg/resources/management.cattle.io/v3/project/mutator.go b/pkg/resources/management.cattle.io/v3/project/mutator.go index b8e087d69..fe68326fb 100644 --- a/pkg/resources/management.cattle.io/v3/project/mutator.go +++ b/pkg/resources/management.cattle.io/v3/project/mutator.go @@ -3,16 +3,21 @@ package project import ( "encoding/json" "fmt" + "strings" v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/webhook/pkg/admission" ctrlv3 "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/patch" + "github.com/rancher/wrangler/pkg/name" + corev1controller "github.com/rancher/wrangler/v2/pkg/generated/controllers/core/v1" "github.com/sirupsen/logrus" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/storage/names" "k8s.io/utils/trace" ) @@ -31,13 +36,17 @@ var gvr = schema.GroupVersionResource{ // Mutator implements admission.MutatingAdmissionWebhook. type Mutator struct { roleTemplateCache ctrlv3.RoleTemplateCache + namespaceCache corev1controller.NamespaceCache + projectCache ctrlv3.ProjectCache } // NewMutator returns a new mutator which mutates projects -func NewMutator(roleTemplateCache ctrlv3.RoleTemplateCache) *Mutator { +func NewMutator(nsCache corev1controller.NamespaceCache, roleTemplateCache ctrlv3.RoleTemplateCache, projectCache ctrlv3.ProjectCache) *Mutator { roleTemplateCache.AddIndexer(mutatorCreatorRoleTemplateIndex, creatorRoleTemplateIndexer) return &Mutator{ roleTemplateCache: roleTemplateCache, + namespaceCache: nsCache, + projectCache: projectCache, } } @@ -58,6 +67,7 @@ func (m *Mutator) GVR() schema.GroupVersionResource { func (m *Mutator) Operations() []admissionregistrationv1.OperationType { return []admissionregistrationv1.OperationType{ admissionregistrationv1.Create, + admissionregistrationv1.Update, } } @@ -85,13 +95,24 @@ func (m *Mutator) Admit(request *admission.Request) (*admissionv1.AdmissionRespo } switch request.Operation { case admissionv1.Create: - return m.admitCreate(project, request) + project, err = m.admitCreate(project) + if err != nil { + return nil, err + } + case admissionv1.Update: + project = m.updateProjectNamespace(project) default: return nil, fmt.Errorf("operation type %q not handled", request.Operation) } + response := &admissionv1.AdmissionResponse{} + if err := patch.CreatePatch(request.Object.Raw, project, response); err != nil { + return nil, fmt.Errorf("failed to create patch: %w", err) + } + response.Allowed = true + return response, nil } -func (m *Mutator) admitCreate(project *v3.Project, request *admission.Request) (*admissionv1.AdmissionResponse, error) { +func (m *Mutator) admitCreate(project *v3.Project) (*v3.Project, error) { logrus.Debugf("[project-mutation] adding creator-role-bindings to project: %v", project.Name) newProject := project.DeepCopy() @@ -103,12 +124,11 @@ func (m *Mutator) admitCreate(project *v3.Project, request *admission.Request) ( return nil, fmt.Errorf("failed to add annotation to project %s: %w", project.Name, err) } newProject.Annotations[roleTemplatesRequired] = annotations - response := &admissionv1.AdmissionResponse{} - if err := patch.CreatePatch(request.Object.Raw, newProject, response); err != nil { - return nil, fmt.Errorf("failed to create patch: %w", err) + newProject, err = m.createProjectNamespace(newProject) + if err != nil { + return nil, fmt.Errorf("failed to create project namespace %s: %w", project.Status.BackingNamespace, err) } - response.Allowed = true - return response, nil + return newProject, nil } func (m *Mutator) getCreatorRoleTemplateAnnotations() (string, error) { @@ -126,3 +146,55 @@ func (m *Mutator) getCreatorRoleTemplateAnnotations() (string, error) { } return string(annotations), nil } +func (m *Mutator) createProjectNamespace(project *v3.Project) (*v3.Project, error) { + newProject := project.DeepCopy() + backingNamespace := "" + var err error + // When the project name is empty, that means we want to generate a name for it + // Name generation happens after mutating webhooks, so in order to have access to the name early + // for the backing namespace, we need to generate it ourselves + if project.Name == "" { + // If err is nil, (meaning "project exists", see below) we need to repeat the generation process to find a project name and backing namespace that isn't taken + for err == nil { + newName := names.SimpleNameGenerator.GenerateName(project.GenerateName) + _, err = m.projectCache.Get(newProject.Spec.ClusterName, newName) + if err == nil { + // A project with this name already exists. Generate a new name. + continue + } else if !apierrors.IsNotFound(err) { + return nil, err + } + + backingNamespace = name.SafeConcatName(newProject.Spec.ClusterName, strings.ToLower(newName)) + _, err = m.namespaceCache.Get(backingNamespace) + + // If the backing namespace already exists, generate a new project name + if err != nil && !apierrors.IsNotFound(err) { + return nil, err + } + } + } else { + backingNamespace = name.SafeConcatName(newProject.Spec.ClusterName, strings.ToLower(newProject.Name)) + _, err = m.namespaceCache.Get(backingNamespace) + if err == nil { + return nil, fmt.Errorf("namespace %v already exists", backingNamespace) + } else if !apierrors.IsNotFound(err) { + return nil, err + } + } + + newProject.Status.BackingNamespace = backingNamespace + return newProject, nil +} + +// updateProjectNamespace fills in BackingNamespace with the project name if it wasn't already set +// this was the naming convention of project namespaces prior to using the BackingNamespace field. Filling +// it here is just to maintain backwards compatibility +func (m *Mutator) updateProjectNamespace(project *v3.Project) *v3.Project { + if project.Status.BackingNamespace != "" { + return project + } + newProject := project.DeepCopy() + newProject.Status.BackingNamespace = newProject.Name + return newProject +} diff --git a/pkg/resources/management.cattle.io/v3/project/mutator_test.go b/pkg/resources/management.cattle.io/v3/project/mutator_test.go index 6f4a19bc8..72c69106d 100644 --- a/pkg/resources/management.cattle.io/v3/project/mutator_test.go +++ b/pkg/resources/management.cattle.io/v3/project/mutator_test.go @@ -5,12 +5,16 @@ import ( "fmt" "testing" + jsonpatch "github.com/evanphx/json-patch" "github.com/golang/mock/gomock" v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/wrangler/v2/pkg/generic/fake" "github.com/stretchr/testify/assert" admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" ) const ( @@ -18,23 +22,37 @@ const ( expectedIndexKey = "creatorDefaultUnlocked" ) +var ( + defaultProject = v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testproject", + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + }, + } + emptyProject = func() *v3.Project { + return &v3.Project{} + } +) + func TestAdmit(t *testing.T) { t.Parallel() tests := []struct { - name string - operation admissionv1.Operation - dryRun bool - oldProject *v3.Project - newProject *v3.Project - indexer func() ([]*v3.RoleTemplate, error) - wantPatch []map[string]interface{} - wantErr bool + name string + operation admissionv1.Operation + dryRun bool + oldProject func() *v3.Project + newProject func() *v3.Project + indexer func() ([]*v3.RoleTemplate, error) + wantProject func() *v3.Project + wantErr bool }{ { name: "dry run returns allowed", operation: admissionv1.Update, dryRun: true, - newProject: &v3.Project{}, + newProject: emptyProject, }, { name: "failure to decode project returns error", @@ -44,85 +62,100 @@ func TestAdmit(t *testing.T) { { name: "delete operation is invalid", operation: admissionv1.Delete, - newProject: &v3.Project{}, - oldProject: &v3.Project{}, + newProject: emptyProject, + oldProject: emptyProject, wantErr: true, }, { - name: "update operation is invalid", - operation: admissionv1.Update, - newProject: &v3.Project{}, - oldProject: &v3.Project{}, - wantErr: true, + name: "update operation is valid and adds backingNamespace", + operation: admissionv1.Update, + newProject: func() *v3.Project { + return &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p-abc123", + }, + } + }, + oldProject: func() *v3.Project { + return &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p-abc123", + }, + } + }, + wantProject: func() *v3.Project { + return &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p-abc123", + }, + Status: v3.ProjectStatus{ + BackingNamespace: "p-abc123", + }, + } + }, }, { name: "connect operation is invalid", operation: admissionv1.Connect, - newProject: &v3.Project{}, - oldProject: &v3.Project{}, + newProject: emptyProject, + oldProject: emptyProject, wantErr: true, }, { name: "indexer error", operation: admissionv1.Create, - newProject: &v3.Project{}, + newProject: emptyProject, indexer: func() ([]*v3.RoleTemplate, error) { return nil, fmt.Errorf("indexer error") }, wantErr: true, }, { name: "indexer returns empty", operation: admissionv1.Create, - newProject: &v3.Project{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testproject", - }, + newProject: func() *v3.Project { + return defaultProject.DeepCopy() }, indexer: func() ([]*v3.RoleTemplate, error) { return nil, nil }, - wantPatch: []map[string]interface{}{ - { - "op": "add", - "path": "/metadata/annotations", - "value": map[string]string{ - "authz.management.cattle.io/creator-role-bindings": "{}", - }, - }, + wantProject: func() *v3.Project { + p := defaultProject.DeepCopy() + p.Annotations = map[string]string{ + "authz.management.cattle.io/creator-role-bindings": "{}", + } + p.Status.BackingNamespace = "testcluster-testproject" + return p }, }, { name: "created project gets annotation added", operation: admissionv1.Create, - newProject: &v3.Project{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testproject", - }, + newProject: func() *v3.Project { + return defaultProject.DeepCopy() }, - wantPatch: []map[string]interface{}{ - { - "op": "add", - "path": "/metadata/annotations", - "value": map[string]string{ - "authz.management.cattle.io/creator-role-bindings": "{\"required\":[\"project-owner\"]}", - }, - }, + wantProject: func() *v3.Project { + p := defaultProject.DeepCopy() + p.Annotations = map[string]string{ + "authz.management.cattle.io/creator-role-bindings": "{\"required\":[\"project-owner\"]}", + } + p.Status.BackingNamespace = "testcluster-testproject" + return p }, }, { name: "override user-set annotations", operation: admissionv1.Create, - newProject: &v3.Project{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testproject", - Annotations: map[string]string{ - "authz.management.cattle.io/creator-role-bindings": "my own setting", - }, - }, + newProject: func() *v3.Project { + p := defaultProject.DeepCopy() + p.Annotations = map[string]string{ + "authz.management.cattle.io/creator-role-bindings": "my own setting", + } + return p }, - wantPatch: []map[string]interface{}{ - { - "op": "replace", - "path": "/metadata/annotations/authz.management.cattle.io~1creator-role-bindings", - "value": "{\"required\":[\"project-owner\"]}", - }, + wantProject: func() *v3.Project { + p := defaultProject.DeepCopy() + p.Annotations = map[string]string{ + "authz.management.cattle.io/creator-role-bindings": "{\"required\":[\"project-owner\"]}", + } + p.Status.BackingNamespace = "testcluster-testproject" + return p }, }, } @@ -145,8 +178,12 @@ func TestAdmit(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { t.Parallel() - req, err := createProjectRequest(test.oldProject, test.newProject, test.operation, test.dryRun) - assert.NoError(t, err) + + ctrl := gomock.NewController(t) + nsMock := fake.NewMockNonNamespacedCacheInterface[*corev1.Namespace](ctrl) + nsMock.EXPECT().Get(gomock.Any()).Return(nil, apierrors.NewNotFound(schema.GroupResource{}, "")).AnyTimes() + projectMock := fake.NewMockCacheInterface[*v3.Project](ctrl) + projectMock.EXPECT().Get(gomock.Any(), gomock.Any()).Return(nil, apierrors.NewNotFound(schema.GroupResource{}, "")).AnyTimes() roleTemplateCache := fake.NewMockNonNamespacedCacheInterface[*v3.RoleTemplate](gomock.NewController(t)) roleTemplateCache.EXPECT().AddIndexer(expectedIndexerName, gomock.Any()) indexer := defaultIndexer @@ -155,19 +192,40 @@ func TestAdmit(t *testing.T) { } returnedRTs, returnedErr := indexer() roleTemplateCache.EXPECT().GetByIndex(expectedIndexerName, expectedIndexKey).Return(returnedRTs, returnedErr).AnyTimes() - m := NewMutator(roleTemplateCache) + + var oldProject, newProject *v3.Project + if test.oldProject != nil { + oldProject = test.oldProject() + } + if test.newProject != nil { + newProject = test.newProject() + } + req, err := createProjectRequest(oldProject, newProject, test.operation, test.dryRun) + assert.NoError(t, err) + m := NewMutator(nsMock, roleTemplateCache, projectMock) + resp, err := m.Admit(req) if test.wantErr { assert.Error(t, err) return } + assert.NoError(t, err, "Admit failed") assert.Equal(t, true, resp.Allowed) - var wantPatch []byte - if test.wantPatch != nil { - wantPatch, err = json.Marshal(test.wantPatch) - assert.NoError(t, err) + if test.wantProject != nil { + patchObj, err := jsonpatch.DecodePatch(resp.Patch) + assert.NoError(t, err, "failed to decode patch from response") + + patchedJS, err := patchObj.Apply(req.Object.Raw) + assert.NoError(t, err, "failed to apply patch to Object") + + gotObj := &v3.Project{} + err = json.Unmarshal(patchedJS, gotObj) + assert.NoError(t, err, "failed to unmarshal patched Object") + + assert.Equal(t, test.wantProject(), gotObj) + } else { + assert.Nil(t, resp.Patch, "unexpected patch request received") } - assert.Equal(t, string(wantPatch), string(resp.Patch)) }) } } diff --git a/pkg/resources/management.cattle.io/v3/project/validator.go b/pkg/resources/management.cattle.io/v3/project/validator.go index e0ce19ec4..9584f0e0e 100644 --- a/pkg/resources/management.cattle.io/v3/project/validator.go +++ b/pkg/resources/management.cattle.io/v3/project/validator.go @@ -20,10 +20,11 @@ import ( ) const ( - systemProjectLabel = "authz.management.cattle.io/system-project" - projectQuotaField = "resourceQuota" - clusterNameField = "clusterName" - namespaceQuotaField = "namespaceDefaultResourceQuota" + systemProjectLabel = "authz.management.cattle.io/system-project" + projectQuotaField = "resourceQuota" + backingNamespaceField = "backingNamespace" + clusterNameField = "clusterName" + namespaceQuotaField = "namespaceDefaultResourceQuota" ) var projectSpecFieldPath = field.NewPath("project").Child("spec") @@ -112,8 +113,13 @@ func (a *admitter) admitCreate(project *v3.Project) (*admissionv1.AdmissionRespo } func (a *admitter) admitUpdate(oldProject, newProject *v3.Project) (*admissionv1.AdmissionResponse, error) { + var fieldErr *field.Error if oldProject.Spec.ClusterName != newProject.Spec.ClusterName { - fieldErr := field.Invalid(projectSpecFieldPath.Child(clusterNameField), newProject.Spec.ClusterName, "field is immutable") + fieldErr = field.Invalid(projectSpecFieldPath.Child(clusterNameField), newProject.Spec.ClusterName, "field is immutable") + } else if oldProject.Status.BackingNamespace != "" && oldProject.Status.BackingNamespace != newProject.Status.BackingNamespace { + fieldErr = field.Invalid(projectSpecFieldPath.Child(backingNamespaceField), newProject.Status.BackingNamespace, "field is immutable") + } + if fieldErr != nil { return admission.ResponseBadRequest(fieldErr.Error()), nil } return a.admitCommonCreateUpdate(oldProject, newProject) diff --git a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/ProjectRoleTemplateBinding.md b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/ProjectRoleTemplateBinding.md index dc57d547e..baa225921 100644 --- a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/ProjectRoleTemplateBinding.md +++ b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/ProjectRoleTemplateBinding.md @@ -14,7 +14,6 @@ Users cannot create ProjectRoleTemplateBindings that violate the following const - The `ProjectName` field must be: - Provided as a non-empty value - Specified using the format of `clusterName:projectName`; `clusterName` is the `metadata.name` of a cluster, and `projectName` is the `metadata.name` of a project - - The `projectName` part of the field must match the namespace of the ProjectRoleTemplateBinding - Refer to a valid project and cluster (both must exist and project.Spec.ClusterName must equal the cluster) - Either a user subject (through `UserName` or `UserPrincipalName`), or a group subject (through `GroupName` or `GroupPrincipalName`), or a service account subject (through `ServiceAccount`) must be specified. Exactly one diff --git a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator.go b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator.go index 1c80e77d9..745b74da8 100644 --- a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator.go +++ b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator.go @@ -210,9 +210,6 @@ func (a *admitter) validateCreateFields(newPRTB *apisv3.ProjectRoleTemplateBindi if clusterName == "" || projectName == "" { return field.Invalid(fieldPath.Child("projectName"), newPRTB.ProjectName, "projectName must be of the form cluster.metadata.name:project.metadata.name, and both must refer to an existing object") } - if projectName != newPRTB.Namespace { - return field.Forbidden(fieldPath, "namespace and the projectName part of projectName must match") - } cluster, err := a.clusterCache.Get(clusterName) clusterNotFoundErr := field.Invalid(fieldPath.Child("projectName"), newPRTB.ProjectName, fmt.Sprintf("specified cluster %s not found", clusterName)) if err != nil { diff --git a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator_test.go b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator_test.go index bcf08d0d6..2e5f78207 100644 --- a/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/projectroletemplatebinding/validator_test.go @@ -999,22 +999,6 @@ func (p *ProjectRoleTemplateBindingSuite) TestValidationOnCreate() { }, allowed: false, }, - { - name: "namespace and the project id part of the project name differ", - args: args{ - username: adminUser, - oldPRTB: func() *apisv3.ProjectRoleTemplateBinding { - return nil - }, - newPRTB: func() *apisv3.ProjectRoleTemplateBinding { - basePRTB := newBasePRTB() - basePRTB.ObjectMeta.Namespace = "default" - basePRTB.ProjectName = fmt.Sprintf("%s:%s", clusterID, "p-cgtq4") - return basePRTB - }, - }, - allowed: false, - }, { name: "missing cluster name", args: args{ diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index f137b3d56..d15b0b9d8 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -67,7 +67,7 @@ func Mutation(clients *clients.Clients) ([]admission.MutatingAdmissionHandler, e if clients.MultiClusterManagement { secrets := secret.NewMutator(clients.RBAC.Role(), clients.RBAC.RoleBinding()) - projects := project.NewMutator(clients.Management.RoleTemplate().Cache()) + projects := project.NewMutator(clients.Core.Namespace().Cache(), clients.Management.RoleTemplate().Cache(), clients.Management.Project().Cache()) grbs := globalrolebinding.NewMutator(clients.Management.GlobalRole().Cache()) mutators = append(mutators, secrets, projects, grbs) } From 43524020ab981ada23608c7ee824f710cc0cdb66 Mon Sep 17 00:00:00 2001 From: Jonathan Crowther Date: Mon, 14 Apr 2025 12:25:43 -0400 Subject: [PATCH 2/3] Update the project with the generated name --- pkg/resources/management.cattle.io/v3/project/mutator.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/resources/management.cattle.io/v3/project/mutator.go b/pkg/resources/management.cattle.io/v3/project/mutator.go index fe68326fb..7e464cec0 100644 --- a/pkg/resources/management.cattle.io/v3/project/mutator.go +++ b/pkg/resources/management.cattle.io/v3/project/mutator.go @@ -155,8 +155,9 @@ func (m *Mutator) createProjectNamespace(project *v3.Project) (*v3.Project, erro // for the backing namespace, we need to generate it ourselves if project.Name == "" { // If err is nil, (meaning "project exists", see below) we need to repeat the generation process to find a project name and backing namespace that isn't taken + newName := "" for err == nil { - newName := names.SimpleNameGenerator.GenerateName(project.GenerateName) + newName = names.SimpleNameGenerator.GenerateName(project.GenerateName) _, err = m.projectCache.Get(newProject.Spec.ClusterName, newName) if err == nil { // A project with this name already exists. Generate a new name. @@ -173,6 +174,7 @@ func (m *Mutator) createProjectNamespace(project *v3.Project) (*v3.Project, erro return nil, err } } + newProject.Name = newName } else { backingNamespace = name.SafeConcatName(newProject.Spec.ClusterName, strings.ToLower(newProject.Name)) _, err = m.namespaceCache.Get(backingNamespace) From 9164bc215728c528842c6967b0c6e37662b507f1 Mon Sep 17 00:00:00 2001 From: Jonathan Crowther Date: Mon, 14 Apr 2025 13:46:19 -0400 Subject: [PATCH 3/3] Sync go version with rancher/rancher --- Dockerfile.dapper | 2 +- go.mod | 3 +-- go.sum | 2 -- pkg/resources/management.cattle.io/v3/project/mutator.go | 2 +- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Dockerfile.dapper b/Dockerfile.dapper index 6485edf7e..e4bcca48b 100644 --- a/Dockerfile.dapper +++ b/Dockerfile.dapper @@ -1,4 +1,4 @@ -FROM registry.suse.com/bci/golang:1.22 +FROM registry.suse.com/bci/golang:1.23 ARG DAPPER_HOST_ARCH ENV ARCH $DAPPER_HOST_ARCH diff --git a/go.mod b/go.mod index 1cc06611d..85d5efc2b 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/rancher/webhook go 1.23.0 -toolchain go1.24.2 +toolchain go1.23.7 replace ( github.com/rancher/rke => github.com/rancher/rke v1.5.13 @@ -47,7 +47,6 @@ require ( github.com/rancher/lasso v0.1.0 github.com/rancher/rancher/pkg/apis v0.0.0-20250411213817-1116d03f0676 github.com/rancher/rke v1.5.15 - github.com/rancher/wrangler v1.1.1 github.com/rancher/wrangler/v2 v2.1.4 github.com/robfig/cron v1.2.0 github.com/sirupsen/logrus v1.9.3 diff --git a/go.sum b/go.sum index fbc1c9866..c21834ee6 100644 --- a/go.sum +++ b/go.sum @@ -302,8 +302,6 @@ github.com/rancher/rancher/pkg/apis v0.0.0-20250411213817-1116d03f0676 h1:ykXjqt github.com/rancher/rancher/pkg/apis v0.0.0-20250411213817-1116d03f0676/go.mod h1:8AfqZ7rm9zNt3F73883QnPoT9tGSM5g3QK62pNd1Vts= github.com/rancher/rke v1.5.13 h1:Y7e3qI0G2HbU3Vm5k3Sqeq+UR2w114K5yY6wT9VFKcY= github.com/rancher/rke v1.5.13/go.mod h1:/z9oyKqYpFwgRBV9rfLxqUdjydz/VMCTcjld4uUt7uM= -github.com/rancher/wrangler v1.1.1 h1:wmqUwqc2M7ADfXnBCJTFkTB5ZREWpD78rnZMzmxwMvM= -github.com/rancher/wrangler v1.1.1/go.mod h1:ioVbKupzcBOdzsl55MvEDN0R1wdGggj8iNCYGFI5JvM= github.com/rancher/wrangler/v2 v2.1.4 h1:ohov0i6A9dJHHO6sjfsH4Dqv93ZTdm5lIJVJdPzVdQc= github.com/rancher/wrangler/v2 v2.1.4/go.mod h1:af5OaGU/COgreQh1mRbKiUI64draT2NN34uk+PALFY8= github.com/robfig/cron v1.2.0 h1:ZjScXvvxeQ63Dbyxy76Fj3AT3Ut0aKsyd2/tl3DTMuQ= diff --git a/pkg/resources/management.cattle.io/v3/project/mutator.go b/pkg/resources/management.cattle.io/v3/project/mutator.go index 7e464cec0..2fc97b184 100644 --- a/pkg/resources/management.cattle.io/v3/project/mutator.go +++ b/pkg/resources/management.cattle.io/v3/project/mutator.go @@ -10,8 +10,8 @@ import ( ctrlv3 "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/patch" - "github.com/rancher/wrangler/pkg/name" corev1controller "github.com/rancher/wrangler/v2/pkg/generated/controllers/core/v1" + "github.com/rancher/wrangler/v2/pkg/name" "github.com/sirupsen/logrus" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1"