Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent deletion of local cluster #551

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/resources/common/psa-validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 11 additions & 2 deletions pkg/resources/core/v1/namespace/projectannotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
)

const (
fleetLocalNs = "fleet-local"
localNs = "local"
manageNSVerb = "manage-namespaces"
projectNSAnnotation = "field.cattle.io/projectId"
)
Expand All @@ -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)
Expand All @@ -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
Expand Down
33 changes: 29 additions & 4 deletions pkg/resources/core/v1/namespace/projectannotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func TestValidateProjectNamespaceAnnotations(t *testing.T) {
sarError bool
wantError bool
wantAllowed bool
namespaceName string
}{
{
name: "user can access, create",
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/resources/core/v1/namespace/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func (v *Validator) Operations() []admissionv1.OperationType {
return []admissionv1.OperationType{
admissionv1.Update,
admissionv1.Create,
admissionv1.Delete,
}
}

Expand Down Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions pkg/resources/core/v1/namespace/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down
10 changes: 10 additions & 0 deletions pkg/resources/management.cattle.io/v3/cluster/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 8 additions & 2 deletions pkg/resources/provisioning.cattle.io/v1/cluster/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (

const (
globalNamespace = "cattle-global-data"
localCluster = "local"
systemAgentVarDirEnvVar = "CATTLE_AGENT_VAR_DIR"
failureStatus = "Failure"
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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"
}

Expand Down