diff --git a/pkg/resources/core/v1/namespace/projectannotations.go b/pkg/resources/core/v1/namespace/projectannotations.go index 879efec73..a4a85eb58 100644 --- a/pkg/resources/core/v1/namespace/projectannotations.go +++ b/pkg/resources/core/v1/namespace/projectannotations.go @@ -15,8 +15,6 @@ import ( ) const ( - fleetLocalNs = "fleet-local" - localNs = "local" manageNSVerb = "manage-namespaces" projectNSAnnotation = "field.cattle.io/projectId" ) @@ -25,10 +23,8 @@ type projectNamespaceAdmitter struct { sar authorizationv1.SubjectAccessReviewInterface } -// Admit ensures that the: -// - user has permission to change the namespace annotation for project membership, effectively moving a project from -// one namespace to another. -// - deletion of `local` and `fleet-local` namespace is not allowed +// Admit ensures that the user has permission to change the namespace annotation for +// project membership, effectively moving a project from one namespace to another. func (p *projectNamespaceAdmitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) { listTrace := trace.New("Namespace Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) defer listTrace.LogIfLong(admission.SlowTraceDuration) @@ -40,11 +36,6 @@ func (p *projectNamespaceAdmitter) Admit(request *admission.Request) (*admission return nil, fmt.Errorf("failed to decode namespace from request: %w", err) } - if request.Operation == admissionv1.Delete { - if oldNs.Name == localNs || oldNs.Name == fleetLocalNs { - return admission.ResponseBadRequest(fmt.Sprintf("deletion of namespace %q is not allowed\n", request.Name)), nil - } - } projectAnnoValue, ok := newNs.Annotations[projectNSAnnotation] if !ok { // this namespace doesn't belong to a project, let standard RBAC handle it diff --git a/pkg/resources/core/v1/namespace/projectannotations_test.go b/pkg/resources/core/v1/namespace/projectannotations_test.go index 20c5c5f2b..e0f5ca30e 100644 --- a/pkg/resources/core/v1/namespace/projectannotations_test.go +++ b/pkg/resources/core/v1/namespace/projectannotations_test.go @@ -31,7 +31,6 @@ func TestValidateProjectNamespaceAnnotations(t *testing.T) { sarError bool wantError bool wantAllowed bool - namespaceName string }{ { name: "user can access, create", @@ -192,26 +191,6 @@ func TestValidateProjectNamespaceAnnotations(t *testing.T) { wantError: true, wantAllowed: false, }, - { - name: "Prevent deletion of 'local' namespace", - operationType: v1.Delete, - wantError: false, - wantAllowed: false, - namespaceName: "local", - }, - { - name: "Prevent deletion of 'fleet-local' namespace", - operationType: v1.Delete, - wantError: false, - wantAllowed: false, - namespaceName: "fleet-local", - }, - { - name: "Allow deletion of namespace", - operationType: v1.Delete, - wantAllowed: true, - wantError: false, - }, } for _, test := range tests { test := test @@ -235,7 +214,7 @@ func TestValidateProjectNamespaceAnnotations(t *testing.T) { // if this wasn't for our project, don't handle the response return false, nil, nil }) - request, err := createAnnotationNamespaceRequest(test.projectAnnotationValue, test.oldProjectAnnotationValue, test.includeProjectAnnotation, test.operationType, test.namespaceName) + request, err := createAnnotationNamespaceRequest(test.projectAnnotationValue, test.oldProjectAnnotationValue, test.includeProjectAnnotation, test.operationType) assert.NoError(t, err) response, err := admitter.Admit(request) if test.wantError { @@ -257,16 +236,12 @@ func sarIsForProjectGVR(sarSpec authorizationv1.SubjectAccessReviewSpec) bool { sarSpec.ResourceAttributes.Resource == projectsGVR.Resource } -func createAnnotationNamespaceRequest(newProjectAnnotation, oldProjectAnnotation string, includeProjectAnnotation bool, operation v1.Operation, namespaceName string) (*admission.Request, error) { +func createAnnotationNamespaceRequest(newProjectAnnotation, oldProjectAnnotation string, includeProjectAnnotation bool, operation v1.Operation) (*admission.Request, error) { gvk := metav1.GroupVersionKind{Version: "v1", Kind: "Namespace"} gvr := metav1.GroupVersionResource{Version: "v1", Resource: "namespace"} - nsName := "test-ns" - if namespaceName != "" { - nsName = namespaceName - } ns := corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: nsName, + Name: "test-ns", }, } if includeProjectAnnotation { @@ -296,7 +271,7 @@ func createAnnotationNamespaceRequest(newProjectAnnotation, oldProjectAnnotation if err != nil { return nil, err } - if operation != v1.Create { + if operation == v1.Update { if includeProjectAnnotation { ns.Annotations[projectNSAnnotation] = oldProjectAnnotation } diff --git a/pkg/resources/core/v1/namespace/validator.go b/pkg/resources/core/v1/namespace/validator.go index 38adb1533..e4655f892 100644 --- a/pkg/resources/core/v1/namespace/validator.go +++ b/pkg/resources/core/v1/namespace/validator.go @@ -49,7 +49,6 @@ func (v *Validator) Operations() []admissionv1.OperationType { return []admissionv1.OperationType{ admissionv1.Update, admissionv1.Create, - admissionv1.Delete, } } @@ -88,10 +87,7 @@ func (v *Validator) ValidatingWebhook(clientConfig admissionv1.WebhookClientConf } kubeSystemCreateWebhook.FailurePolicy = admission.Ptr(admissionv1.Ignore) - deleteWebhook := admission.NewDefaultValidatingWebhook(v, clientConfig, admissionv1.ClusterScope, []admissionv1.OperationType{admissionv1.Delete}) - deleteWebhook.Name = admission.CreateWebhookName(v, "delete-only") - - return []admissionv1.ValidatingWebhook{*standardWebhook, *createWebhook, *kubeSystemCreateWebhook, *deleteWebhook} + return []admissionv1.ValidatingWebhook{*standardWebhook, *createWebhook, *kubeSystemCreateWebhook} } // Admitters returns the psaAdmitter and the projectNamespaceAdmitter for namespaces. diff --git a/pkg/resources/core/v1/namespace/validator_test.go b/pkg/resources/core/v1/namespace/validator_test.go index aa380a024..0dcfba11d 100644 --- a/pkg/resources/core/v1/namespace/validator_test.go +++ b/pkg/resources/core/v1/namespace/validator_test.go @@ -20,7 +20,7 @@ func TestGVR(t *testing.T) { func TestOperations(t *testing.T) { validator := NewValidator(nil) operations := validator.Operations() - assert.Len(t, operations, 3) + assert.Len(t, operations, 2) assert.Contains(t, operations, v1.Update) assert.Contains(t, operations, v1.Create) } @@ -56,7 +56,7 @@ func TestValidatingWebhook(t *testing.T) { wantURL := "test.cattle.io/namespaces" validator := NewValidator(nil) webhooks := validator.ValidatingWebhook(clientConfig) - assert.Len(t, webhooks, 4) + assert.Len(t, webhooks, 3) hasAllUpdateWebhook := false hasCreateNonKubeSystemWebhook := false hasCreateKubeSystemWebhook := false @@ -71,7 +71,7 @@ func TestValidatingWebhook(t *testing.T) { operation := operations[0] assert.Equal(t, v1.ClusterScope, *rule.Scope) - assert.Contains(t, []v1.OperationType{v1.Create, v1.Update, v1.Delete}, operation, "only expected webhooks for create, update and delete") + assert.Contains(t, []v1.OperationType{v1.Create, v1.Update}, operation, "only expected webhooks for create and update") if operation == v1.Update { assert.False(t, hasAllUpdateWebhook, "had more than one webhook validating update calls, exepcted only one") hasAllUpdateWebhook = true @@ -81,7 +81,7 @@ func TestValidatingWebhook(t *testing.T) { // failure policy defaults to fail, but if we specify one it needs to be fail assert.Equal(t, v1.Fail, *webhook.FailurePolicy) } - } else if operation == v1.Create { + } else { assert.NotNil(t, webhook.NamespaceSelector) matchExpressions := webhook.NamespaceSelector.MatchExpressions assert.Len(t, matchExpressions, 1) diff --git a/pkg/resources/management.cattle.io/v3/cluster/validator.go b/pkg/resources/management.cattle.io/v3/cluster/validator.go index 8cd9b84a4..d21e39354 100644 --- a/pkg/resources/management.cattle.io/v3/cluster/validator.go +++ b/pkg/resources/management.cattle.io/v3/cluster/validator.go @@ -26,8 +26,6 @@ import ( var parsedRangeLessThan123 = semver.MustParseRange("< 1.23.0-rancher0") -const localCluster = "local" - // NewValidator returns a new validator for management clusters. func NewValidator( sar authorizationv1.SubjectAccessReviewInterface, @@ -83,11 +81,6 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp return nil, fmt.Errorf("failed get old and new clusters from request: %w", err) } - if request.Operation == admissionv1.Delete && oldCluster.Name == localCluster { - // deleting "local" cluster could corrupt the cluster Rancher is deployed in - return admission.ResponseBadRequest("cannot delete the local cluster"), nil - } - response, err := a.validateFleetPermissions(request, oldCluster, newCluster) if err != nil { return nil, fmt.Errorf("failed to validate fleet permissions: %w", err) @@ -119,7 +112,7 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp if request.Operation == admissionv1.Create || request.Operation == admissionv1.Update { // no need to validate the PodSecurityAdmissionConfigurationTemplate on a local cluster, // or imported cluster which represents a KEv2 cluster (GKE/EKS/AKS) or v1 Provisioning Cluster - if newCluster.Name == localCluster || newCluster.Spec.RancherKubernetesEngineConfig == nil { + if newCluster.Name == "local" || newCluster.Spec.RancherKubernetesEngineConfig == 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 35d91e5df..dbaf2658b 100644 --- a/pkg/resources/management.cattle.io/v3/cluster/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/cluster/validator_test.go @@ -309,16 +309,6 @@ func TestAdmit(t *testing.T) { expectAllowed: true, expectedReason: metav1.StatusReasonBadRequest, }, - { - name: "Delete local cluster where Rancher is deployed", - operation: admissionv1.Delete, - oldCluster: v3.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "local", - }, - }, - expectAllowed: false, - }, } for _, tt := range tests { diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go index 0289d281e..270de0df5 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go @@ -35,7 +35,6 @@ import ( const ( globalNamespace = "cattle-global-data" - localCluster = "local" systemAgentVarDirEnvVar = "CATTLE_AGENT_VAR_DIR" failureStatus = "Failure" ) @@ -93,11 +92,6 @@ func (p *provisioningAdmitter) Admit(request *admission.Request) (*admissionv1.A listTrace := trace.New("provisioningClusterValidator Admit", trace.Field{Key: "user", Value: request.UserInfo.Username}) defer listTrace.LogIfLong(admission.SlowTraceDuration) - if request.Operation == admissionv1.Delete && request.Name == localCluster { - // deleting "local" cluster could corrupt the cluster Rancher is deployed in - return admission.ResponseBadRequest("can't delete local cluster"), nil - } - oldCluster, cluster, err := objectsv1.ClusterOldAndNewFromRequest(&request.AdmissionRequest) if err != nil { return nil, err @@ -422,7 +416,7 @@ func (p *provisioningAdmitter) validateMachinePoolNames(request *admission.Reque // validatePSACT validate if the cluster and underlying secret are configured properly when PSACT is enabled or disabled func (p *provisioningAdmitter) validatePSACT(request *admission.Request, response *admissionv1.AdmissionResponse, cluster *v1.Cluster) error { - if cluster.Name == localCluster || cluster.Spec.RKEConfig == nil { + if cluster.Name == "local" || cluster.Spec.RKEConfig == nil { return nil } @@ -670,7 +664,7 @@ func validateACEConfig(cluster *v1.Cluster) *metav1.Status { func isValidName(clusterName, clusterNamespace string, clusterExists bool) bool { // A provisioning cluster with name "local" is only expected to be created in the "fleet-local" namespace. - if clusterName == localCluster { + if clusterName == "local" { return clusterNamespace == "fleet-local" }