Merge pull request #2551 from phanimarupaka/NestedSubst

Nested substitutions

cmd/config/internal/commands/cmdcreatesubstitution.go

@@ -88,16 +88,31 @@ func (r *CreateSubstitutionRunner) preRunE(c *cobra.Command, args []string) erro
 	re := regexp.MustCompile(`\$\{([^}]*)\}`)
 	markers := re.FindAll([]byte(r.CreateSubstitution.Pattern), -1)
 	if len(markers) == 0 {
-		return errors.Errorf("unable to find setter names in pattern, " +
+		return errors.Errorf("unable to find setter or substitution names in pattern, " +
 			"setter names must be enclosed in ${}")
 	}
 
 	for _, marker := range markers {
-		ref := fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix +
+		name := strings.TrimSuffix(strings.TrimPrefix(string(marker), "${"), "}")
-			strings.TrimSuffix(strings.TrimPrefix(string(marker), "${"), "}")
+
+		ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + name)
+		if err != nil {
+			return err
+		}
+
+		var markerRef string
+		subst, _ := openapi.Resolve(&ref)
+		// check if the substitution exists with the marker name or fall back to creating setter
+		// ref with the name
+		if subst != nil {
+			markerRef = fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + name
+		} else {
+			markerRef = fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + name
+		}
+
 		r.CreateSubstitution.Values = append(
 			r.CreateSubstitution.Values,
-			setters2.Value{Marker: string(marker), Ref: ref},
+			setters2.Value{Marker: string(marker), Ref: markerRef},
 		)
 	}
 

cmd/config/internal/commands/cmdcreatesubstitution_test.go

@@ -149,16 +149,6 @@ apiVersion: v1alpha1
 kind: Example
 openAPI:
   definitions:
-    io.k8s.cli.setters.my-image-setter:
-      x-k8s-cli:
-        setter:
-          name: my-image-setter
-          value: nginx
-    io.k8s.cli.setters.my-tag-setter:
-      x-k8s-cli:
-        setter:
-          name: my-tag-setter
-          value: 1.7.9
     io.k8s.cli.substitutions.my-image-subst:
       x-k8s-cli:
         substitution:
@@ -169,6 +159,16 @@ openAPI:
             ref: '#/definitions/io.k8s.cli.setters.my-image-setter'
           - marker: ${my-tag-setter}
             ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: nginx
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
  `,
 			expectedResources: `
 apiVersion: apps/v1
@@ -186,6 +186,228 @@ spec:
         image: sidecar:1.7.9
  `,
 		},
+		{
+			name: "nested substitution",
+			args: []string{
+				"my-nested-subst", "--field-value", "something/nginx::1.7.9/nginxotherthing",
+				"--pattern", "something/${my-image-subst}/${my-other-setter}"},
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: something/nginx::1.7.9/nginxotherthing
+      - name: sidecar
+        image: nginx::1.7.9 # {"$openapi":"my-image-subst"}
+ `,
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: nginx
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
+    io.k8s.cli.substitutions.my-image-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-image-subst
+          pattern: ${my-image-setter}::${my-tag-setter}
+          values:
+          - marker: ${my-image-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-image-setter'
+          - marker: ${my-tag-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+ `,
+			expectedOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: nginx
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
+    io.k8s.cli.substitutions.my-image-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-image-subst
+          pattern: ${my-image-setter}::${my-tag-setter}
+          values:
+          - marker: ${my-image-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-image-setter'
+          - marker: ${my-tag-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+    io.k8s.cli.substitutions.my-nested-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-nested-subst
+          pattern: something/${my-image-subst}/${my-other-setter}
+          values:
+          - marker: ${my-image-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst'
+          - marker: ${my-other-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-other-setter'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+ `,
+			expectedResources: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"}
+      - name: sidecar
+        image: nginx::1.7.9 # {"$openapi":"my-image-subst"}
+ `,
+		},
+		{
+			name: "nested cyclic substitution",
+			args: []string{"my-nested-subst", "--field-value", "something/nginx::1.7.9/nginxotherthing",
+				"--pattern", "something/${my-image-subst}/${my-other-setter}"},
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: nginx
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
+    io.k8s.cli.substitutions.my-image-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-image-subst
+          pattern: ${my-nested-subst}::${my-tag-setter}
+          values:
+          - marker: ${my-nested-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-nested-subst'
+          - marker: ${my-tag-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+    io.k8s.cli.substitutions.my-nested-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-nested-subst
+          pattern: something/${my-image-subst}/${my-other-setter}
+          values:
+          - marker: ${my-image-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst'
+          - marker: ${my-other-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-other-setter'
+ `,
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"}
+      - name: sidecar
+        image: nginx::1.7.9 # {"$openapi":"my-image-subst"}
+ `,
+			expectedOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: nginx
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
+    io.k8s.cli.substitutions.my-image-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-image-subst
+          pattern: ${my-nested-subst}::${my-tag-setter}
+          values:
+          - marker: ${my-nested-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-nested-subst'
+          - marker: ${my-tag-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+    io.k8s.cli.substitutions.my-nested-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-nested-subst
+          pattern: something/${my-image-subst}/${my-other-setter}
+          values:
+          - marker: ${my-image-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst'
+          - marker: ${my-other-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-other-setter'
+
+ `,
+			expectedResources: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"}
+      - name: sidecar
+        image: nginx::1.7.9 # {"$openapi":"my-image-subst"}
+`,
+			err: "cyclic substitution detected with name my-nested-subst",
+		},
 	}
 	for i := range tests {
 		test := tests[i]

cmd/config/internal/commands/cmdset_test.go

@@ -238,7 +238,6 @@ openAPI:
             ref: '#/definitions/io.k8s.cli.setters.image-setter'
           - marker: TAG
             ref: '#/definitions/io.k8s.cli.setters.tag'
-
  `,
 			expectedResources: `
 apiVersion: apps/v1
@@ -432,7 +431,6 @@ openAPI:
             ref: '#/definitions/io.k8s.cli.setters.image'
           - marker: TAG
             ref: '#/definitions/io.k8s.cli.setters.tag'
-
  `,
 			expectedResources: `
 apiVersion: apps/v1
@@ -702,6 +700,240 @@ spec:
 list in body must be of type integer: "boolean"
 list in body should have at most 2 items`,
 		},
+		{
+			name: "nested substitution",
+			args: []string{"my-image-setter", "ubuntu"},
+			out:  "set 2 fields\n",
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: nginx
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
+    io.k8s.cli.substitutions.my-image-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-image-subst
+          pattern: ${my-image-setter}::${my-tag-setter}
+          values:
+          - marker: ${my-image-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-image-setter'
+          - marker: ${my-tag-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+    io.k8s.cli.substitutions.my-nested-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-nested-subst
+          pattern: something/${my-image-subst}/${my-other-setter}
+          values:
+          - marker: ${my-image-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst'
+          - marker: ${my-other-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-other-setter'
+ `,
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"}
+      - name: sidecar
+        image: nginx::1.7.9 # {"$openapi":"my-image-subst"}
+ `,
+			expectedOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: ubuntu
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
+    io.k8s.cli.substitutions.my-image-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-image-subst
+          pattern: ${my-image-setter}::${my-tag-setter}
+          values:
+          - marker: ${my-image-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-image-setter'
+          - marker: ${my-tag-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+    io.k8s.cli.substitutions.my-nested-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-nested-subst
+          pattern: something/${my-image-subst}/${my-other-setter}
+          values:
+          - marker: ${my-image-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst'
+          - marker: ${my-other-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-other-setter'
+ `,
+			expectedResources: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: something/ubuntu::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"}
+      - name: sidecar
+        image: ubuntu::1.7.9 # {"$openapi":"my-image-subst"}
+`,
+		},
+		{
+			name: "nested cyclic substitution",
+			args: []string{"my-image-setter", "ubuntu"},
+			inputOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: nginx
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
+    io.k8s.cli.substitutions.my-image-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-image-subst
+          pattern: ${my-nested-subst}::${my-tag-setter}
+          values:
+          - marker: ${my-nested-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-nested-subst'
+          - marker: ${my-tag-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+    io.k8s.cli.substitutions.my-nested-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-nested-subst
+          pattern: something/${my-image-subst}/${my-other-setter}
+          values:
+          - marker: ${my-image-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst'
+          - marker: ${my-other-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-other-setter'
+ `,
+			input: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"}
+      - name: sidecar
+        image: nginx::1.7.9 # {"$openapi":"my-image-subst"}
+ `,
+			expectedOpenAPI: `
+apiVersion: v1alpha1
+kind: Example
+openAPI:
+  definitions:
+    io.k8s.cli.setters.my-image-setter:
+      x-k8s-cli:
+        setter:
+          name: my-image-setter
+          value: nginx
+    io.k8s.cli.setters.my-tag-setter:
+      x-k8s-cli:
+        setter:
+          name: my-tag-setter
+          value: 1.7.9
+    io.k8s.cli.substitutions.my-image-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-image-subst
+          pattern: ${my-nested-subst}::${my-tag-setter}
+          values:
+          - marker: ${my-nested-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-nested-subst'
+          - marker: ${my-tag-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-tag-setter'
+    io.k8s.cli.setters.my-other-setter:
+      x-k8s-cli:
+        setter:
+          name: my-other-setter
+          value: nginxotherthing
+    io.k8s.cli.substitutions.my-nested-subst:
+      x-k8s-cli:
+        substitution:
+          name: my-nested-subst
+          pattern: something/${my-image-subst}/${my-other-setter}
+          values:
+          - marker: ${my-image-subst}
+            ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst'
+          - marker: ${my-other-setter}
+            ref: '#/definitions/io.k8s.cli.setters.my-other-setter'
+ `,
+			expectedResources: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"}
+      - name: sidecar
+        image: nginx::1.7.9 # {"$openapi":"my-image-subst"}
+`,
+			errMsg: "cyclic substitution detected with name my-nested-subst",
+		},
 	}
 	for i := range tests {
 		test := tests[i]

kyaml/setters2/set.go

@@ -97,7 +97,7 @@ func (s *Set) visitScalar(object *yaml.RNode, p string, schema *openapi.Resource
 	}
 
 	// perform a substitution of the field if it matches
-	sub, err := s.substitute(object, ext, schema.Schema)
+	sub, err := s.substitute(object, ext)
 	if err != nil {
 		return err
 	}
@@ -109,62 +109,32 @@ func (s *Set) visitScalar(object *yaml.RNode, p string, schema *openapi.Resource
 
 // substitute updates the value of field from ext if ext contains a substitution that
 // depends on a setter whose name matches s.Name.
-func (s *Set) substitute(field *yaml.RNode, ext *cliExtension, _ *spec.Schema) (bool, error) {
+func (s *Set) substitute(field *yaml.RNode, ext *CliExtension) (bool, error) {
-	nameMatch := false
-
 	// check partial setters to see if they contain the setter as part of a
 	// substitution
 	if ext.Substitution == nil {
 		return false, nil
 	}
 
-	p := ext.Substitution.Pattern
+	// track the visited nodes to detect cycles in nested substitutions
+	visited := sets.String{}
 
-	// substitute each setter into the pattern to get the new value
+	// nameMatch indicates if the input substitution depends on the specified setter,
-	for _, v := range ext.Substitution.Values {
+	// the substitution in ext is parsed recursively and if the setter in Set is hit while
-		if v.Ref == "" {
+	// parsing, it indicates the match
-			return false, errors.Errorf(
+	nameMatch := false
-				"missing reference on substitution " + ext.Substitution.Name)
-		}
-		ref, err := spec.NewRef(v.Ref)
-		if err != nil {
-			return false, errors.Wrap(err)
-		}
-		setter, err := openapi.Resolve(&ref) // resolve the setter to its openAPI def
-		if err != nil {
-			return false, errors.Wrap(err)
-		}
-		subSetter, err := getExtFromSchema(setter) // parse the extension out of the openAPI
-		if err != nil {
-			return false, errors.Wrap(err)
-		}
 
-		if err := validateAgainstSchema(subSetter, setter); err != nil {
+	res, err := s.substituteUtil(ext, visited, &nameMatch)
+	if err != nil {
 		return false, err
 	}
 
-		if val, found := subSetter.Setter.EnumValues[subSetter.Setter.Value]; found {
-			// the setter has an enum-map.  we should replace the marker with the
-			// enum value looked up from the map rather than the enum key
-			p = strings.ReplaceAll(p, v.Marker, val)
-		} else {
-			// substitute the setters current value into the substitution pattern
-			p = strings.ReplaceAll(p, v.Marker, subSetter.Setter.Value)
-		}
-
-		if s.isMatch(subSetter.Setter.Name) {
-			// the substitution depends on the specified setter
-			nameMatch = true
-		}
-	}
 	if !nameMatch {
 		// doesn't depend on the setter, don't modify its value
 		return false, nil
 	}
 
-	// TODO(pwittrock): validate the field value
+	field.YNode().Value = res
-
-	field.YNode().Value = p
 
 	// substitutions are always strings
 	field.YNode().Tag = yaml.StringTag
@@ -172,8 +142,74 @@ func (s *Set) substitute(field *yaml.RNode, ext *cliExtension, _ *spec.Schema) (
 	return true, nil
 }
 
+// substituteUtil recursively parses nested substitutions in ext and sets the setter value
+// returns error if cyclic substitution is detected or any other unexpected errors
+func (s *Set) substituteUtil(ext *CliExtension, visited sets.String, nameMatch *bool) (string, error) {
+	// check if the substitution has already been visited and throw error as cycles
+	// are not allowed in nested substitutions
+	if visited.Has(ext.Substitution.Name) {
+		return "", errors.Errorf(
+			"cyclic substitution detected with name " + ext.Substitution.Name)
+	}
+
+	visited.Insert(ext.Substitution.Name)
+	pattern := ext.Substitution.Pattern
+
+	// substitute each setter into the pattern to get the new value
+	// if substitution references to another substitution, recursively
+	// process the nested substitutions to replace the pattern with setter values
+	for _, v := range ext.Substitution.Values {
+		if v.Ref == "" {
+			return "", errors.Errorf(
+				"missing reference on substitution " + ext.Substitution.Name)
+		}
+		ref, err := spec.NewRef(v.Ref)
+		if err != nil {
+			return "", errors.Wrap(err)
+		}
+		def, err := openapi.Resolve(&ref) // resolve the def to its openAPI def
+		if err != nil {
+			return "", errors.Wrap(err)
+		}
+		defExt, err := GetExtFromSchema(def) // parse the extension out of the openAPI
+		if err != nil {
+			return "", errors.Wrap(err)
+		}
+
+		if defExt.Substitution != nil {
+			// parse recursively if it reference is substitution
+			substVal, err := s.substituteUtil(defExt, visited, nameMatch)
+			if err != nil {
+				return "", err
+			}
+			pattern = strings.ReplaceAll(pattern, v.Marker, substVal)
+			continue
+		}
+
+		// if code reaches this point, this is a setter, so validate the setter schema
+		if err := validateAgainstSchema(defExt, def); err != nil {
+			return "", err
+		}
+
+		if s.isMatch(defExt.Setter.Name) {
+			// the substitution depends on the specified setter
+			*nameMatch = true
+		}
+
+		if val, found := defExt.Setter.EnumValues[defExt.Setter.Value]; found {
+			// the setter has an enum-map.  we should replace the marker with the
+			// enum value looked up from the map rather than the enum key
+			pattern = strings.ReplaceAll(pattern, v.Marker, val)
+		} else {
+			pattern = strings.ReplaceAll(pattern, v.Marker, defExt.Setter.Value)
+		}
+	}
+
+	return pattern, nil
+}
+
 // set applies the value from ext to field if its name matches s.Name
-func (s *Set) set(field *yaml.RNode, ext *cliExtension, sch *spec.Schema) (bool, error) {
+func (s *Set) set(field *yaml.RNode, ext *CliExtension, sch *spec.Schema) (bool, error) {
 	// check full setter
 	if ext.Setter == nil || !s.isMatch(ext.Setter.Name) {
 		return false, nil
@@ -200,7 +236,7 @@ func (s *Set) set(field *yaml.RNode, ext *cliExtension, sch *spec.Schema) (bool,
 
 // validateAgainstSchema validates the input setter value against user provided
 // openAI schema
-func validateAgainstSchema(ext *cliExtension, sch *spec.Schema) error {
+func validateAgainstSchema(ext *CliExtension, sch *spec.Schema) error {
 	sc := spec.Schema{}
 	sc.Properties = map[string]spec.Schema{}
 	sc.Properties[ext.Setter.Name] = *sch

kyaml/setters2/settersutil/substitutioncreator.go

@@ -5,12 +5,16 @@ package settersutil
 
 import (
 	"fmt"
+	"io/ioutil"
+	"os"
 	"strings"
 
+	"github.com/go-openapi/spec"
 	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/fieldmeta"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/openapi"
+	"sigs.k8s.io/kustomize/kyaml/sets"
 	"sigs.k8s.io/kustomize/kyaml/setters2"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
@@ -47,7 +51,15 @@ func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error {
 		Pattern: c.Pattern,
 	}
 
-	err := c.CreateSettersForSubstitution(openAPIPath)
+	// the input substitution definition is updated in the openAPI file and then parsed
+	// to check if there are any cycles in nested substitutions, if there are
+	// any, the openAPI file will be reverted to current state and error is thrown
+	stat, err := os.Stat(openAPIPath)
+	if err != nil {
+		return err
+	}
+
+	curOpenAPI, err := ioutil.ReadFile(openAPIPath)
 	if err != nil {
 		return err
 	}
@@ -61,6 +73,40 @@ func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error {
 		return err
 	}
 
+	visited := sets.String{}
+	ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + c.Name)
+	if err != nil {
+		return err
+	}
+
+	schema, err := openapi.Resolve(&ref)
+	if err != nil {
+		return err
+	}
+
+	ext, err := setters2.GetExtFromSchema(schema)
+	if err != nil {
+		return err
+	}
+
+	err = c.CreateSettersForSubstitution(openAPIPath)
+	if err != nil {
+		return err
+	}
+
+	// Load the updated definitions after setters are created
+	if err := openapi.AddSchemaFromFile(openAPIPath); err != nil {
+		return err
+	}
+
+	// revert openAPI file if there are cycles detected in created input substitution
+	if err := checkForCycles(ext, visited); err != nil {
+		if writeErr := ioutil.WriteFile(openAPIPath, curOpenAPI, stat.Mode().Perm()); writeErr != nil {
+			return writeErr
+		}
+		return err
+	}
+
 	// Update the resources with the setter reference
 	inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath}
 	return kio.Pipeline{
@@ -88,19 +134,25 @@ func (c SubstitutionCreator) CreateSettersForSubstitution(openAPIPath string) er
 		return err
 	}
 
-	// for each ref in values, check if the setter already exists, if not create them
+	// for each ref in values, check if the setter or substitution already exists, if not create setter
 	for _, value := range c.Values {
-		obj, err := y.Pipe(yaml.Lookup(
+		// continue if ref is a substitution, as it has already been checked if it exists
+		// as part of preRunE
+		if strings.Contains(value.Ref, fieldmeta.SubstitutionDefinitionPrefix) {
+			fmt.Printf("found a substitution with name %s\n", value.Marker)
+			continue
+		}
+		setterObj, err := y.Pipe(yaml.Lookup(
 			// get the setter key from ref. Ex: from #/definitions/io.k8s.cli.setters.image_setter
 			// extract io.k8s.cli.setters.image_setter
-			"openAPI", "definitions", strings.TrimPrefix(value.Ref, "#/definitions/")))
+			"openAPI", "definitions", strings.TrimPrefix(value.Ref, fieldmeta.DefinitionsPrefix)))
 
 		if err != nil {
 			return err
 		}
 
-		if obj == nil {
+		if setterObj == nil {
-			name := strings.TrimPrefix(value.Ref, "#/definitions/io.k8s.cli.setters.")
+			name := strings.TrimPrefix(value.Ref, fieldmeta.DefinitionsPrefix+fieldmeta.SetterDefinitionPrefix)
 			value := m[value.Marker]
 			fmt.Printf("unable to find setter with name %s, creating new setter with value %s\n", name, value)
 			sd := setters2.SetterDefinition{
@@ -118,6 +170,49 @@ func (c SubstitutionCreator) CreateSettersForSubstitution(openAPIPath string) er
 	return nil
 }
 
+func checkForCycles(ext *setters2.CliExtension, visited sets.String) error {
+	// check if the substitution has already been visited and throw error as cycles
+	// are not allowed in nested substitutions
+	if visited.Has(ext.Substitution.Name) {
+		return errors.Errorf(
+			"cyclic substitution detected with name " + ext.Substitution.Name)
+	}
+
+	visited.Insert(ext.Substitution.Name)
+
+	// substitute each setter into the pattern to get the new value
+	// if substitution references to another substitution, recursively
+	// process the nested substitutions to replace the pattern with setter values
+	for _, v := range ext.Substitution.Values {
+		if v.Ref == "" {
+			return errors.Errorf(
+				"missing reference on substitution " + ext.Substitution.Name)
+		}
+		ref, err := spec.NewRef(v.Ref)
+		if err != nil {
+			return errors.Wrap(err)
+		}
+		def, err := openapi.Resolve(&ref) // resolve the def to its openAPI def
+		if err != nil {
+			return errors.Wrap(err)
+		}
+		defExt, err := setters2.GetExtFromSchema(def) // parse the extension out of the openAPI
+		if err != nil {
+			return errors.Wrap(err)
+		}
+
+		if defExt.Substitution != nil {
+			// parse recursively if it reference is substitution
+			err := checkForCycles(defExt, visited)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 // GetValuesForMarkers parses the pattern and field value to derive values for the
 // markers in the pattern string. Returns error if the marker values can't be derived
 func (c SubstitutionCreator) GetValuesForMarkers() (map[string]string, error) {

kyaml/setters2/types.go

@@ -11,7 +11,7 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/openapi"
 )
 
-type cliExtension struct {
+type CliExtension struct {
 	Setter       *setter       `yaml:"setter,omitempty" json:"setter,omitempty"`
 	Substitution *substitution `yaml:"substitution,omitempty" json:"substitution,omitempty"`
 }
@@ -37,8 +37,8 @@ type substitutionSetterReference struct {
 //K8sCliExtensionKey is the name of the OpenAPI field containing the setter extensions
 const K8sCliExtensionKey = "x-k8s-cli"
 
-// getExtFromSchema returns the cliExtension openAPI extension if it is present in schema
+// GetExtFromSchema returns the cliExtension openAPI extension if it is present in schema
-func getExtFromSchema(schema *spec.Schema) (*cliExtension, error) {
+func GetExtFromSchema(schema *spec.Schema) (*CliExtension, error) {
 	cep := schema.VendorExtensible.Extensions[K8sCliExtensionKey]
 	if cep == nil {
 		return nil, nil
@@ -47,7 +47,7 @@ func getExtFromSchema(schema *spec.Schema) (*cliExtension, error) {
 	if err != nil {
 		return nil, err
 	}
-	val := &cliExtension{}
+	val := &CliExtension{}
 	if err := json.Unmarshal(b, val); err != nil {
 		return nil, err
 	}
@@ -56,7 +56,7 @@ func getExtFromSchema(schema *spec.Schema) (*cliExtension, error) {
 
 // getExtFromComment returns the cliExtension openAPI extension if it is present as
 // a comment on the field.
-func getExtFromComment(schema *openapi.ResourceSchema) (*cliExtension, error) {
+func getExtFromComment(schema *openapi.ResourceSchema) (*CliExtension, error) {
 	if schema == nil {
 		// no schema found
 		// TODO(pwittrock): should this be an error if it doesn't resolve?
@@ -64,7 +64,7 @@ func getExtFromComment(schema *openapi.ResourceSchema) (*cliExtension, error) {
 	}
 
 	// get the cli extension from the openapi (contains setter information)
-	ext, err := getExtFromSchema(schema.Schema)
+	ext, err := GetExtFromSchema(schema.Schema)
 	if err != nil {
 		return nil, errors.Wrap(err)
 	}