Prevent deletion of local cluster (#551) (#592)

* Prevent deletion of `local` cluster

It prevents deletion of both clusters.provisioning.cattle.io and
cluster.management.cattle.io of the name `local`.



* Prevent deletion of `local` and `fleet-local` namespaces



* Parameter type grouping



---------

Signed-off-by: Dharmit Shah <[email protected]>

Author: dharmit

Diff for: 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
 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 for: 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 for: pkg/resources/core/v1/namespace/validator.go

@@ -47,6 +47,7 @@ func (v *Validator) Operations() []admissionv1.OperationType {
 	return []admissionv1.OperationType{
 		admissionv1.Update,
 		admissionv1.Create,
+		admissionv1.Delete,
 	}
 }
 
@@ -85,7 +86,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 for: 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 for: 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 for: 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 for: 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"
 	}