diff --git a/pkg/resources/common/psa-validation.go b/pkg/resources/common/psa-validation.go index 560e4a847..98795dbca 100644 --- a/pkg/resources/common/psa-validation.go +++ b/pkg/resources/common/psa-validation.go @@ -23,7 +23,7 @@ var psaLabels = []string{ // IsUpdatingPSAConfig will indicate whether or not the labels being passed in // are attempting to update PSA-related configuration. -func IsUpdatingPSAConfig(old map[string]string, new map[string]string) bool { +func IsUpdatingPSAConfig(old, new map[string]string) bool { for _, label := range psaLabels { if old[label] != new[label] { return true diff --git a/pkg/resources/core/v1/namespace/projectannotations.go b/pkg/resources/core/v1/namespace/projectannotations.go index 1d9672acf..bbd9cc5b5 100644 --- a/pkg/resources/core/v1/namespace/projectannotations.go +++ b/pkg/resources/core/v1/namespace/projectannotations.go @@ -15,6 +15,8 @@ import ( ) const ( + fleetLocalNs = "fleet-local" + localNs = "local" manageNSVerb = "manage-namespaces" projectNSAnnotation = "field.cattle.io/projectId" ) @@ -23,8 +25,10 @@ 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. +// 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 as it could 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) @@ -36,6 +40,11 @@ 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 ab5e59644..0c70ed6cf 100644 --- a/pkg/resources/core/v1/namespace/projectannotations_test.go +++ b/pkg/resources/core/v1/namespace/projectannotations_test.go @@ -30,6 +30,7 @@ func TestValidateProjectNamespaceAnnotations(t *testing.T) { sarError bool wantError bool wantAllowed bool + namespaceName string }{ { name: "user can access, create", @@ -189,6 +190,26 @@ 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 @@ -212,7 +233,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) + request, err := createAnnotationNamespaceRequest(test.projectAnnotationValue, test.oldProjectAnnotationValue, test.includeProjectAnnotation, test.operationType, test.namespaceName) assert.NoError(t, err) response, err := admitter.Admit(request) if test.wantError { @@ -231,12 +252,16 @@ func sarIsForProjectGVR(sarSpec authorizationv1.SubjectAccessReviewSpec) bool { sarSpec.ResourceAttributes.Resource == projectsGVR.Resource } -func createAnnotationNamespaceRequest(newProjectAnnotation, oldProjectAnnotation string, includeProjectAnnotation bool, operation v1.Operation) (*admission.Request, error) { +func createAnnotationNamespaceRequest(newProjectAnnotation, oldProjectAnnotation string, includeProjectAnnotation bool, operation v1.Operation, namespaceName string) (*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: "test-ns", + Name: nsName, }, } if includeProjectAnnotation { @@ -266,7 +291,7 @@ func createAnnotationNamespaceRequest(newProjectAnnotation, oldProjectAnnotation if err != nil { return nil, err } - if operation == v1.Update { + if operation != v1.Create { 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 e4655f892..38adb1533 100644 --- a/pkg/resources/core/v1/namespace/validator.go +++ b/pkg/resources/core/v1/namespace/validator.go @@ -49,6 +49,7 @@ func (v *Validator) Operations() []admissionv1.OperationType { return []admissionv1.OperationType{ admissionv1.Update, admissionv1.Create, + admissionv1.Delete, } } @@ -87,7 +88,10 @@ func (v *Validator) ValidatingWebhook(clientConfig admissionv1.WebhookClientConf } kubeSystemCreateWebhook.FailurePolicy = admission.Ptr(admissionv1.Ignore) - return []admissionv1.ValidatingWebhook{*standardWebhook, *createWebhook, *kubeSystemCreateWebhook} + 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} } // 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 0dcfba11d..aa380a024 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, 2) + assert.Len(t, operations, 3) 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, 3) + assert.Len(t, webhooks, 4) 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}, operation, "only expected webhooks for create and update") + assert.Contains(t, []v1.OperationType{v1.Create, v1.Update, v1.Delete}, operation, "only expected webhooks for create, update and delete") 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 { + } else if operation == v1.Create { 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 d21e39354..8cd9b84a4 100644 --- a/pkg/resources/management.cattle.io/v3/cluster/validator.go +++ b/pkg/resources/management.cattle.io/v3/cluster/validator.go @@ -26,6 +26,8 @@ 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, @@ -81,6 +83,11 @@ 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) @@ -112,7 +119,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 == "local" || newCluster.Spec.RancherKubernetesEngineConfig == nil { + if newCluster.Name == localCluster || 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 dbaf2658b..35d91e5df 100644 --- a/pkg/resources/management.cattle.io/v3/cluster/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/cluster/validator_test.go @@ -309,6 +309,16 @@ 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 270de0df5..0289d281e 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go @@ -35,6 +35,7 @@ import ( const ( globalNamespace = "cattle-global-data" + localCluster = "local" systemAgentVarDirEnvVar = "CATTLE_AGENT_VAR_DIR" failureStatus = "Failure" ) @@ -92,6 +93,11 @@ 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 @@ -416,7 +422,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 == "local" || cluster.Spec.RKEConfig == nil { + if cluster.Name == localCluster || cluster.Spec.RKEConfig == nil { return nil } @@ -664,7 +670,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 == "local" { + if clusterName == localCluster { return clusterNamespace == "fleet-local" }