From 38bba2b54b6707105d002fad4b11bb5378a5c41c Mon Sep 17 00:00:00 2001 From: Jake Hyde Date: Fri, 17 Oct 2025 11:13:20 -0400 Subject: [PATCH] Ensure operation is update when validating RKEConfig --- .../v1/cluster/validator.go | 20 +++ .../v1/cluster/validator_test.go | 152 ++++++++++++++++++ 2 files changed, 172 insertions(+) diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go index 28ee8aa3c..1cd89c61b 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go @@ -115,6 +115,10 @@ func (p *provisioningAdmitter) Admit(request *admission.Request) (*admissionv1.A return response, err } + if response := p.validateRKEConfigChanged(request, oldCluster, cluster); !response.Allowed { + return response, nil + } + if err := p.validateMachinePoolNames(request, response, cluster); err != nil || response.Result != nil { return response, err } @@ -180,6 +184,22 @@ func getEnvVar(name string, envVars []rkev1.EnvVar) *rkev1.EnvVar { return envVar } +// validateRKEConfigChanged validates that after creation, the `spec.rkeConfig` cannot be set to a non-nil value if it +// was nil, and likewise cannot be set to a nil value if it was not. The local cluster is explicitly exempted from +// setting rkeConfig from nil to not nil, as it is a valid usecase to do so for rancherd in harvester environments. +func (p *provisioningAdmitter) validateRKEConfigChanged(request *admission.Request, oldCluster, newCluster *v1.Cluster) *admissionv1.AdmissionResponse { + if request.Operation != admissionv1.Update { + return admission.ResponseAllowed() + } + if oldCluster.Spec.RKEConfig == nil && newCluster.Spec.RKEConfig != nil && oldCluster.Name != localCluster { + return admission.ResponseBadRequest("RKEConfig cannot be changed from null after cluster creation") + } else if oldCluster.Spec.RKEConfig != nil && newCluster.Spec.RKEConfig == nil { + return admission.ResponseBadRequest("RKEConfig cannot be made null after cluster creation") + } + + return admission.ResponseAllowed() +} + // validateSystemAgentDataDirectory validates the effective system agent data directory, ensuring that the intended // previously configured "CATTLE_AGENT_VAR_DIR" is used during and post migration to the SystemAgent data directory // field. Once this migration is performed and the field is set, the existing of the env var is completely disallowed. diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go b/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go index 500584682..8102f620d 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go @@ -2733,3 +2733,155 @@ func Test_validateS3Secret(t *testing.T) { }) } } + +func Test_ValidateRKEConfigChanged(t *testing.T) { + tests := []struct { + name string + op admissionv1.Operation + oldCluster *v1.Cluster + newCluster *v1.Cluster + expected bool + }{ + { + name: "create", + op: admissionv1.Create, + oldCluster: &v1.Cluster{}, + newCluster: &v1.Cluster{}, + expected: true, + }, + { + name: "delete", + op: admissionv1.Delete, + oldCluster: &v1.Cluster{}, + newCluster: &v1.Cluster{}, + expected: true, + }, + { + name: "no change - nil", + op: admissionv1.Update, + oldCluster: &v1.Cluster{}, + newCluster: &v1.Cluster{}, + expected: true, + }, + { + name: "no change - nil - local", + op: admissionv1.Update, + oldCluster: &v1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "local"}}, + newCluster: &v1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "local"}}, + expected: true, + }, + { + name: "no change - not nil", + op: admissionv1.Update, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + RKEConfig: &v1.RKEConfig{}, + }, + }, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + RKEConfig: &v1.RKEConfig{}, + }, + }, + expected: true, + }, + { + name: "no change - not nil - local", + op: admissionv1.Update, + oldCluster: &v1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "local", + }, + Spec: v1.ClusterSpec{ + RKEConfig: &v1.RKEConfig{}, + }, + }, + newCluster: &v1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "local", + }, + Spec: v1.ClusterSpec{ + RKEConfig: &v1.RKEConfig{}, + }, + }, + expected: true, + }, + { + name: "change - was nil", + op: admissionv1.Update, + oldCluster: &v1.Cluster{}, + newCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + RKEConfig: &v1.RKEConfig{}, + }, + }, + expected: false, + }, + { + name: "change - was nil - local", + op: admissionv1.Update, + oldCluster: &v1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "local", + }, + }, + newCluster: &v1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "local", + }, + Spec: v1.ClusterSpec{ + RKEConfig: &v1.RKEConfig{}, + }, + }, + expected: true, + }, + { + name: "change - was not nil", + op: admissionv1.Update, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + RKEConfig: &v1.RKEConfig{}, + }, + }, + newCluster: &v1.Cluster{}, + expected: false, + }, + { + name: "change - was not nil - local", + op: admissionv1.Update, + oldCluster: &v1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "local", + }, + Spec: v1.ClusterSpec{ + RKEConfig: &v1.RKEConfig{}, + }, + }, + newCluster: &v1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "local", + }, + }, + expected: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + p := provisioningAdmitter{} + req := &admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: tt.op, + }, + } + response := p.validateRKEConfigChanged(req, tt.oldCluster, tt.newCluster) + if tt.expected { + assert.True(t, response.Allowed, "Expected change to be admitted") + } else { + assert.False(t, response.Allowed, "Expected change not to be admitted") + } + }) + } +}