Ensure operation is update when validating RKEConfig

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.

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")
+			}
+		})
+	}
+}