Merge pull request #5925 from sbueringer/pr-allow-nested-valuefrom-variable

🐛 ClusterClass: allow accessing nestedFields via valueFrom.variable

Author: k8s-ci-robot

internal/webhooks/patch_validation.go

@@ -296,7 +296,12 @@ func validateJSONPatchValues(jsonPatch clusterv1.JSONPatch, variableSet map[stri
 					))
 			}
 		} else {
-			if _, ok := variableSet[*jsonPatch.ValueFrom.Variable]; !ok {
+			// Note: We're only validating if the variable name exists without
+			// validating if the whole path is an existing variable.
+			// This could be done by re-using getVariableValue of the json patch
+			// generator but requires a refactoring first.
+			variableName := getVariableName(*jsonPatch.ValueFrom.Variable)
+			if _, ok := variableSet[variableName]; !ok {
 				allErrs = append(allErrs,
 					field.Invalid(
 						path.Child("valueFrom", "variable"),
@@ -309,6 +314,12 @@ func validateJSONPatchValues(jsonPatch clusterv1.JSONPatch, variableSet map[stri
 	return allErrs
 }
 
+func getVariableName(variable string) string {
+	return strings.FieldsFunc(variable, func(r rune) bool {
+		return r == '[' || r == '.'
+	})[0]
+}
+
 // This contains a list of all of the valid builtin variables.
 // TODO(killianmuldoon): Match this list to controllers/topology/internal/extensions/patches/variables as those structs become available across the code base i.e. public or top-level internal.
 var builtinVariables = sets.NewString(

internal/webhooks/patch_validation_test.go

@@ -1185,6 +1185,115 @@ func TestValidatePatches(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "pass if jsonPatch uses a user-defined variable which is defined",
+			clusterClass: clusterv1.ClusterClass{
+				Spec: clusterv1.ClusterClassSpec{
+					ControlPlane: clusterv1.ControlPlaneClass{
+						LocalObjectTemplate: clusterv1.LocalObjectTemplate{
+							Ref: &corev1.ObjectReference{
+								APIVersion: "controlplane.cluster.x-k8s.io/v1beta1",
+								Kind:       "ControlPlaneTemplate",
+							},
+						},
+					},
+					Patches: []clusterv1.ClusterClassPatch{
+						{
+							Name: "patch1",
+							Definitions: []clusterv1.PatchDefinition{
+								{
+									Selector: clusterv1.PatchSelector{
+										APIVersion: "controlplane.cluster.x-k8s.io/v1beta1",
+										Kind:       "ControlPlaneTemplate",
+										MatchResources: clusterv1.PatchSelectorMatch{
+											ControlPlane: true,
+										},
+									},
+									JSONPatches: []clusterv1.JSONPatch{
+										{
+											Op:   "add",
+											Path: "/spec/template/spec/",
+											ValueFrom: &clusterv1.JSONPatchValue{
+												Variable: pointer.String("variableName"),
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+					Variables: []clusterv1.ClusterClassVariable{
+						{
+							Name:     "variableName",
+							Required: true,
+							Schema: clusterv1.VariableSchema{
+								OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+									Type: "string",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "pass if jsonPatch uses a nested user-defined variable which is defined",
+			clusterClass: clusterv1.ClusterClass{
+				Spec: clusterv1.ClusterClassSpec{
+					ControlPlane: clusterv1.ControlPlaneClass{
+						LocalObjectTemplate: clusterv1.LocalObjectTemplate{
+							Ref: &corev1.ObjectReference{
+								APIVersion: "controlplane.cluster.x-k8s.io/v1beta1",
+								Kind:       "ControlPlaneTemplate",
+							},
+						},
+					},
+					Patches: []clusterv1.ClusterClassPatch{
+						{
+							Name: "patch1",
+							Definitions: []clusterv1.PatchDefinition{
+								{
+									Selector: clusterv1.PatchSelector{
+										APIVersion: "controlplane.cluster.x-k8s.io/v1beta1",
+										Kind:       "ControlPlaneTemplate",
+										MatchResources: clusterv1.PatchSelectorMatch{
+											ControlPlane: true,
+										},
+									},
+									JSONPatches: []clusterv1.JSONPatch{
+										{
+											Op:   "add",
+											Path: "/spec/template/spec/",
+											ValueFrom: &clusterv1.JSONPatchValue{
+												Variable: pointer.String("variableName.nestedField"),
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+					Variables: []clusterv1.ClusterClassVariable{
+						{
+							Name:     "variableName",
+							Required: true,
+							Schema: clusterv1.VariableSchema{
+								OpenAPIV3Schema: clusterv1.JSONSchemaProps{
+									Type: "object",
+									Properties: map[string]clusterv1.JSONSchemaProps{
+										"nestedField": {
+											Type: "string",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
 		{
 			name: "error if jsonPatch uses a builtin variable which is not defined",
 			clusterClass: clusterv1.ClusterClass{
@@ -1628,3 +1737,35 @@ func Test_validateSelectors(t *testing.T) {
 		})
 	}
 }
+
+func TestGetVariableName(t *testing.T) {
+	tests := []struct {
+		name         string
+		variable     string
+		variableName string
+	}{
+		{
+			name:         "simple variable",
+			variable:     "variableA",
+			variableName: "variableA",
+		},
+		{
+			name:         "variable object",
+			variable:     "variableObject.field",
+			variableName: "variableObject",
+		},
+		{
+			name:         "variable array",
+			variable:     "variableArray[0]",
+			variableName: "variableArray",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			g.Expect(getVariableName(tt.variable)).To(Equal(tt.variableName))
+		})
+	}
+}