Prevent deletion of namespaces and cluster (#651)

* Ignore namespace delete operation

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

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

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

* Prevent deletion of local cluster

It prevents deletion of both clusters.provisioning.cattle.io and
cluster.management.cattle.io resources of the named local.

* Fixes to tests based on CI feedback

---------

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

pkg/resources/core/v1/namespace/deleteNamespace.go

@@ -0,0 +1,29 @@
+package namespace
+
+import (
+	"fmt"
+
+	"github.com/rancher/webhook/pkg/admission"
+	objectsv1 "github.com/rancher/webhook/pkg/generated/objects/core/v1"
+	admissionv1 "k8s.io/api/admission/v1"
+	"k8s.io/utils/trace"
+)
+
+// deleteNamespaceAdmitter handles namespace deletion scenarios
+type deleteNamespaceAdmitter struct{}
+
+func (d deleteNamespaceAdmitter) 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)
+
+	if request.Operation != admissionv1.Delete {
+		return admission.ResponseAllowed(), nil
+	}
+
+	oldNs, _, err := objectsv1.NamespaceOldAndNewFromRequest(&request.AdmissionRequest)
+	if err != nil {
+		return nil, fmt.Errorf("failed to decode namespace from request: %w", err)
+	}
+
+	return admission.ResponseBadRequest(fmt.Sprintf("%q namespace my not be deleted\n", oldNs.Name)), nil
+}

pkg/resources/core/v1/namespace/deleteNamespace_test.go

@@ -0,0 +1,65 @@
+package namespace
+
+import (
+	"testing"
+
+	"github.com/rancher/webhook/pkg/admission"
+	"github.com/stretchr/testify/assert"
+	admissionv1 "k8s.io/api/admission/v1"
+)
+
+func Test_Admit(t *testing.T) {
+	tests := []struct {
+		name          string
+		namespaceName string
+		operationType admissionv1.Operation
+		wantAllowed   bool
+		wantErr       bool
+	}{
+		{
+			name:          "Allow creating namespace",
+			namespaceName: "local",
+			operationType: admissionv1.Create,
+			wantAllowed:   true,
+			wantErr:       false,
+		},
+		{
+			name:          "Prevent deletion of 'local' namespace",
+			namespaceName: "local",
+			operationType: admissionv1.Delete,
+			wantAllowed:   false,
+			wantErr:       true,
+		},
+		{
+			name:          "Prevent deletion of 'fleet-local' namespace",
+			namespaceName: "fleet-local",
+			operationType: admissionv1.Delete,
+			wantAllowed:   false,
+			wantErr:       true,
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			d := deleteNamespaceAdmitter{}
+			request := createRequest(test.name, test.namespaceName, test.operationType)
+			response, err := d.Admit(request)
+			if test.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, test.wantAllowed, response.Allowed)
+			}
+		})
+	}
+}
+
+func createRequest(name, namespaceName string, operation admissionv1.Operation) *admission.Request {
+	return &admission.Request{
+		AdmissionRequest: admissionv1.AdmissionRequest{
+			Name:      name,
+			Namespace: namespaceName,
+			Operation: operation,
+		},
+	}
+}

pkg/resources/core/v1/namespace/projectannotations.go

@@ -26,6 +26,10 @@ type projectNamespaceAdmitter struct {
 // 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) {
+	if request.Operation == admissionv1.Delete {
+		return admission.ResponseAllowed(), nil
+	}
+
 	listTrace := trace.New("Namespace Admit", trace.Field{Key: "user", Value: request.UserInfo.Username})
 	defer listTrace.LogIfLong(admission.SlowTraceDuration)
 

pkg/resources/core/v1/namespace/psalabels.go

@@ -22,6 +22,10 @@ type psaLabelAdmitter struct {
 
 // Admit ensures that users have sufficient permissions to add/remove PSAs to a namespace.
 func (p *psaLabelAdmitter) Admit(request *admission.Request) (*admissionv1.AdmissionResponse, error) {
+	if request.Operation == admissionv1.Delete {
+		return admission.ResponseAllowed(), nil
+	}
+
 	listTrace := trace.New("Namespace Admit", trace.Field{Key: "user", Value: request.UserInfo.Username})
 	defer listTrace.LogIfLong(admission.SlowTraceDuration)
 

pkg/resources/core/v1/namespace/validator.go

@@ -18,13 +18,15 @@ var projectsGVR = schema.GroupVersionResource{
 
 // Validator validates the namespace admission request.
 type Validator struct {
+	deleteNamespaceAdmitter  deleteNamespaceAdmitter
 	psaAdmitter              psaLabelAdmitter
 	projectNamespaceAdmitter projectNamespaceAdmitter
 }
 
 // NewValidator returns a new validator used for validation of namespace requests.
 func NewValidator(sar authorizationv1.SubjectAccessReviewInterface) *Validator {
 	return &Validator{
+		deleteNamespaceAdmitter: deleteNamespaceAdmitter{},
 		psaAdmitter: psaLabelAdmitter{
 			sar: sar,
 		},
@@ -47,6 +49,7 @@ func (v *Validator) Operations() []admissionv1.OperationType {
 	return []admissionv1.OperationType{
 		admissionv1.Update,
 		admissionv1.Create,
+		admissionv1.Delete,
 	}
 }
 
@@ -85,10 +88,22 @@ func (v *Validator) ValidatingWebhook(clientConfig admissionv1.WebhookClientConf
 	}
 	kubeSystemCreateWebhook.FailurePolicy = admission.Ptr(admissionv1.Ignore)
 
-	return []admissionv1.ValidatingWebhook{*standardWebhook, *createWebhook, *kubeSystemCreateWebhook}
+	deleteNamespaceWebhook := admission.NewDefaultValidatingWebhook(v, clientConfig, admissionv1.ClusterScope, []admissionv1.OperationType{admissionv1.Delete})
+	deleteNamespaceWebhook.Name = admission.CreateWebhookName(v, "delete-namespace")
+	deleteNamespaceWebhook.NamespaceSelector = &metav1.LabelSelector{
+		MatchExpressions: []metav1.LabelSelectorRequirement{
+			{
+				Key:      corev1.LabelMetadataName,
+				Operator: metav1.LabelSelectorOpIn,
+				Values:   []string{"fleet-local", "local"},
+			},
+		},
+	}
+
+	return []admissionv1.ValidatingWebhook{*deleteNamespaceWebhook, *standardWebhook, *createWebhook, *kubeSystemCreateWebhook}
 }
 
 // Admitters returns the psaAdmitter and the projectNamespaceAdmitter for namespaces.
 func (v *Validator) Admitters() []admission.Admitter {
-	return []admission.Admitter{&v.psaAdmitter, &v.projectNamespaceAdmitter}
+	return []admission.Admitter{&v.psaAdmitter, &v.projectNamespaceAdmitter, &v.deleteNamespaceAdmitter}
 }

pkg/resources/core/v1/namespace/validator_test.go

@@ -20,15 +20,15 @@ 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)
 }
 
 func TestAdmitters(t *testing.T) {
 	validator := NewValidator(nil)
 	admitters := validator.Admitters()
-	assert.Len(t, admitters, 2)
+	assert.Len(t, admitters, 3)
 	hasPSAAdmitter := false
 	hasProjectNamespaceAdmitter := false
 	for i := range admitters {
@@ -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)

pkg/resources/management.cattle.io/v3/cluster/validator.go

@@ -24,6 +24,8 @@ import (
 	authorizationv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
 )
 
+const localCluster = "local"
+
 var parsedRangeLessThan123 = semver.MustParseRange("< 1.23.0-rancher0")
 
 // NewValidator returns a new validator for management clusters.
@@ -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("local cluster may not be deleted"), nil
+	}
+
 	response, err := a.validateFleetPermissions(request, oldCluster, newCluster)
 	if err != nil {
 		return nil, fmt.Errorf("failed to validate fleet permissions: %w", err)

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 {

pkg/resources/provisioning.cattle.io/v1/cluster/validator.go

@@ -37,6 +37,7 @@ const (
 	globalNamespace         = "cattle-global-data"
 	systemAgentVarDirEnvVar = "CATTLE_AGENT_VAR_DIR"
 	failureStatus           = "Failure"
+	localCluster            = "local"
 )
 
 var (
@@ -97,6 +98,11 @@ func (p *provisioningAdmitter) Admit(request *admission.Request) (*admissionv1.A
 		return nil, err
 	}
 
+	if request.Operation == admissionv1.Delete && oldCluster.Name == localCluster {
+		// deleting "local" cluster could corrupt the cluster Rancher is deployed in
+		return admission.ResponseBadRequest("local cluster may not be deleted"), nil
+	}
+
 	response := &admissionv1.AdmissionResponse{}
 	if request.Operation == admissionv1.Create || request.Operation == admissionv1.Update {
 		if err := p.validateClusterName(request, response, cluster); err != nil || response.Result != nil {
@@ -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"
 	}