Merge pull request #10692 from bharath-123/feat/set-cluster-validation

Throw error if path being set by kops set is not present in struct

pkg/commands/set_cluster.go

@@ -84,7 +84,7 @@ func SetClusterFields(fields []string, cluster *api.Cluster) error {
 		key = strings.TrimPrefix(key, "cluster.")
 
 		if err := reflectutils.SetString(cluster, key, kv[1]); err != nil {
-			return fmt.Errorf("failed to set %s=%s: %v", kv[0], kv[1], err)
+			return err
 		}
 	}
 	return nil

pkg/commands/set_instancegroups.go

@@ -101,7 +101,7 @@ func SetInstancegroupFields(fields []string, instanceGroup *api.InstanceGroup) e
 		key = strings.TrimPrefix(key, "instancegroup.")
 
 		if err := reflectutils.SetString(instanceGroup, key, kv[1]); err != nil {
-			return fmt.Errorf("failed to set %s=%s: %v", kv[0], kv[1], err)
+			return err
 		}
 	}
 

util/pkg/reflectutils/access.go

@@ -31,6 +31,8 @@ func SetString(target interface{}, targetPath string, newValue string) error {
 		return fmt.Errorf("cannot parse field path %q: %w", targetPath, err)
 	}
 
+	var fieldSet = false
+
 	visitor := func(path *FieldPath, field *reflect.StructField, v reflect.Value) error {
 		if !targetFieldPath.HasPrefixMatch(path) {
 			return nil
@@ -45,6 +47,7 @@ func SetString(target interface{}, targetPath string, newValue string) error {
 				return fmt.Errorf("cannot set field %q: %v", path, err)
 			}
 
+			fieldSet = true
 			return nil
 		}
 
@@ -74,7 +77,16 @@ func SetString(target interface{}, targetPath string, newValue string) error {
 		return nil
 	}
 
-	return ReflectRecursive(targetValue, visitor, &ReflectOptions{JSONNames: true})
+	err = ReflectRecursive(targetValue, visitor, &ReflectOptions{JSONNames: true})
+	if err != nil {
+		return err
+	}
+
+	if !fieldSet {
+		return fmt.Errorf("field %s not found in %s", targetPath, BuildTypeName(reflect.TypeOf(target)))
+	}
+
+	return nil
 }
 
 func setPrimitive(v reflect.Value, newValue string) error {

util/pkg/reflectutils/access_test.go

@@ -202,3 +202,77 @@ func TestSet(t *testing.T) {
 		})
 	}
 }
+
+func TestSetInvalidPath(t *testing.T) {
+	grid := []struct {
+		Name          string
+		Input         string
+		Expected      string
+		Path          string
+		Value         string
+		ExpectedError string
+	}{
+		{
+			Name:          "setting with wildcard",
+			Input:         "{ 'spec': { 'containers': [ {} ] } }",
+			Expected:      "{ 'spec': { 'containers': [ { 'image': 'hello-world' } ] } }",
+			Path:          "spec.containers[*].wrongImagePathName",
+			Value:         "hello-world",
+			ExpectedError: "field spec.containers[*].wrongImagePathName not found in *fakeObject",
+		},
+		{
+			Name:          "creating missing objects",
+			Input:         "{ 'spec': { 'containers': [ {} ] } }",
+			Expected:      "{ 'spec': { 'containers': [ { 'policy': { 'name': 'allowed' } } ] } }",
+			Path:          "spec.containers[0].policy.wrongPolicyName",
+			Value:         "allowed",
+			ExpectedError: "field spec.containers[0].policy.wrongPolicyName not found in *fakeObject",
+		},
+		{
+			Name:          "set int",
+			Input:         "{ 'spec': { 'containers': [ {} ] } }",
+			Expected:      "{ 'spec': { 'containers': [ { 'int': 123 } ] } }",
+			Path:          "spec.wrongNameContainers[0].int",
+			Value:         "123",
+			ExpectedError: "field spec.wrongNameContainers[0].int not found in *fakeObject",
+		},
+		{
+			Name:          "set int32",
+			Input:         "{ 'spec': { 'containers': [ {} ] } }",
+			Expected:      "{ 'spec': { 'containers': [ { 'int32': 123 } ] } }",
+			Path:          "spec.containers[0].int32100",
+			Value:         "123",
+			ExpectedError: "field spec.containers[0].int32100 not found in *fakeObject",
+		},
+		{
+			Name:          "set int64",
+			Input:         "{ 'spec': { 'containers': [ {} ] } }",
+			Expected:      "{ 'spec': { 'containers': [ { 'int64': 123 } ] } }",
+			Path:          "wrong.path.check",
+			Value:         "123",
+			ExpectedError: "field wrong.path.check not found in *fakeObject",
+		},
+	}
+
+	for _, g := range grid {
+		g := g
+
+		t.Run(g.Name, func(t *testing.T) {
+
+			c := &fakeObject{}
+			if err := json.Unmarshal(toJSON(g.Input), c); err != nil {
+				t.Fatalf("failed to unmarshal input: %v", err)
+
+			}
+
+			err := SetString(c, g.Path, g.Value)
+			if err == nil {
+				t.Fatalf("Expected error for invalid path %s", g.Path)
+			}
+
+			if err.Error() != g.ExpectedError {
+				t.Fatalf("Expected Error: %s\n Actual Error: %s", g.ExpectedError, err.Error())
+			}
+		})
+	}
+}