Make module metadata validators module-scoped and add Blueprint.YamlCtx to detect explicit module settings

Author: saara-tyagi27

community/modules/compute/gke-nodeset/metadata.yaml

@@ -18,3 +18,10 @@ spec:
     services:
     - container.googleapis.com
     - storage.googleapis.com
+ghpc:
+  validators:
+  - validator: regex
+    inputs:
+      vars: [slurm_cluster_name]
+      pattern: "^[a-z](?:[a-z0-9]{0,9})$"
+    error_message: "'slurm_cluster_name' must contain only lowercase letters and numbers, start with a letter, and be 1-10 characters long."

community/modules/compute/notebook/metadata.yaml

@@ -18,11 +18,3 @@ spec:
     services:
     - notebooks.googleapis.com
     - storage.googleapis.com
-ghpc:
-  validators:
-  - validator: regex
-    inputs:
-      vars: [deployment_name]
-      pattern: "^[a-z](?:[a-z0-9]{0,6})$"
-      scope: module
-    error_message: "'deployment_name' must contain only lowercase letters and numbers, start with a letter, and be 1-7 characters long."

community/modules/compute/schedmd-slurm-gcp-v6-nodeset/metadata.yaml

@@ -19,10 +19,3 @@ spec:
 ghpc:
   inject_module_id: name
   has_to_be_used: true
-  validators:
-  - validator: regex
-    inputs:
-      vars: [slurm_cluster_name]
-      scope: blueprint
-      pattern: "^[a-z](?:[a-z0-9]{0,9})$"
-    error_message: "'slurm_cluster_name' must contain only lowercase letters and numbers, start with a letter, and be 1-10 characters long."

community/modules/compute/schedmd-slurm-gcp-v6-partition/metadata.yaml

@@ -23,5 +23,4 @@ ghpc:
     inputs:
       vars: [partition_name]
       pattern: "^[a-z](?:[a-z0-9]*)$"
-      scope: blueprint
     error_message: "'partition_name' must start with a lowercase letter and can only contain lowercase letters and numbers."

community/modules/scheduler/schedmd-slurm-gcp-v6-controller/metadata.yaml

@@ -19,3 +19,10 @@ spec:
     - compute.googleapis.com
     - iam.googleapis.com
     - storage.googleapis.com
+ghpc:
+  validators:
+  - validator: regex
+    inputs:
+      vars: [slurm_cluster_name]
+      pattern: "^[a-z](?:[a-z0-9]{0,9})$"
+    error_message: "'slurm_cluster_name' must contain only lowercase letters and numbers, start with a letter, and be 1-10 characters long."

modules/compute/gke-node-pool/metadata.yaml

@@ -24,5 +24,4 @@ ghpc:
     inputs:
       vars: [name]
       pattern: "^[a-z]([-a-z0-9]{0,34}[a-z0-9])?$"
-      scope: module
     error_message: "'name' must start with a lowercase letter, followed by up to 34 lowercase letters, numbers, or hyphens, and cannot end with a hyphen."

modules/compute/resource-policy/metadata.yaml

@@ -23,5 +23,4 @@ ghpc:
     inputs:
       vars: [name]
       pattern: "^[a-z]([-a-z0-9]{0,52}[a-z0-9])?$"
-      scope: module
     error_message: "'name' must start with a lowercase letter, followed by up to 52 lowercase letters, numbers, or hyphens, and cannot end with a hyphen."

pkg/config/config.go

@@ -291,6 +291,9 @@ type Blueprint struct {
 	path string
 	// records of intentions to stage file (populated by ghpc_stage function)
 	stagedFiles map[string]string
+	// YamlCtx holds parsed YAML positions so validators can tell if a module setting
+	// was explicitly present in the user's source (runtime-only, not serialized).
+	YamlCtx *YamlCtx `yaml:"-"`
 }
 
 func (bp *Blueprint) Clone() Blueprint {
@@ -467,6 +470,9 @@ func NewBlueprint(path string) (Blueprint, *YamlCtx, error) {
 		return Blueprint{}, &ctx, err
 	}
 	bp.path = absPath
+	// Attach parsed YAML context to the Blueprint so validators can determine
+	// whether a module.setting path was explicitly present in the user's YAML.
+	bp.YamlCtx = &ctx
 	return bp, &ctx, nil
 }
 

pkg/config/config_test.go

@@ -355,6 +355,9 @@ func (s *zeroSuite) TestNewBlueprint(c *C) {
 	c.Assert(err, IsNil)
 
 	bp.path = outFile // set expected path
+	// NewBlueprint populates a runtime-only YamlCtx (positions in source YAML).
+	// Reflect that in the expected blueprint before doing a DeepEquals compare.
+	bp.YamlCtx = newBp.YamlCtx
 	c.Assert(bp, DeepEquals, newBp)
 }
 

pkg/validators/metadata_validator_helpers.go

@@ -62,67 +62,32 @@ func evaluateAndFlatten(val cty.Value) []cty.Value {
 	return values
 }
 
-// getBlueprintValues retrieves a cty.Value from blueprint variables by name,
-// evaluates it and returns the flattened values and the path to use for errors.
-func getBlueprintValues(bp config.Blueprint, varName string) ([]cty.Value, config.Path, error) {
-	var nilPath config.Path
-
-	if !bp.Vars.Has(varName) {
-		return nil, nilPath, fmt.Errorf("variable %q not found in blueprint vars", varName)
-	}
-	val := bp.Vars.Get(varName)
-	if evaledVal, err := bp.Eval(val); err == nil {
-		val = evaledVal
-	}
-	values := evaluateAndFlatten(val)
-	return values, config.Root.Vars.Dot(varName), nil
-}
-
 // getModuleSettingValues retrieves a cty.Value from module settings using a dot-separated path,
 // evaluates expressions via bp.Eval and returns flattened slice + path for errors.
 func getModuleSettingValues(bp config.Blueprint, group config.Group, modIdx int, mod config.Module, settingName string) ([]cty.Value, config.Path, error) {
 	var nilPath config.Path
 
-	val, found := getNestedValue(mod.Settings, settingName)
-	if !found {
-		return nil, nilPath, fmt.Errorf("setting %q not found in module %q settings", settingName, mod.ID)
-	}
-	if evaledVal, err := bp.Eval(val); err == nil {
-		val = evaledVal
-	}
-	values := evaluateAndFlatten(val)
-
+	// Determine canonical path for this module.setting in the blueprint.
 	groupIndex := bp.GroupIndex(group.Name)
 	path := config.Root.Groups.At(groupIndex).Modules.At(modIdx).Settings.Dot(settingName)
 
-	return values, path, nil
-}
+	// If YAML context exists and the path is not present in the user's YAML,
+	// treat the setting as absent so validators skip it (honoring optional:true).
 
-// valuesEqualBlueprint returns true when the provided flattened module values are
-// equal to the flattened blueprint var with the same name. Comparison is only done
-// for string values; non-comparable types return false.
-func valuesEqualBlueprint(bp config.Blueprint, varName string, moduleValues []cty.Value) bool {
-	if !bp.Vars.Has(varName) {
-		return false
-	}
-	bpVal := bp.Vars.Get(varName)
-	if evaledBp, err := bp.Eval(bpVal); err == nil {
-		bpVal = evaledBp
+	if bp.YamlCtx != nil {
+		if _, ok := bp.YamlCtx.Pos(path); !ok {
+			return nil, nilPath, fmt.Errorf("setting %q not present in blueprint YAML for module %q", settingName, mod.ID)
+		}
 	}
-	bpVals := evaluateAndFlatten(bpVal)
 
-	if len(bpVals) != len(moduleValues) {
-		return false
-	}
-	for i := range bpVals {
-		if bpVals[i].Type() != cty.String || moduleValues[i].Type() != cty.String {
-			return false
-		}
-		if bpVals[i].AsString() != moduleValues[i].AsString() {
-			return false
-		}
+	val, _ := getNestedValue(mod.Settings, settingName)
+
+	if evaledVal, err := bp.Eval(val); err == nil {
+		val = evaledVal
 	}
-	return true
+	values := evaluateAndFlatten(val)
+
+	return values, path, nil
 }
 
 // parseStringList normalizes an input that may be a single string, []interface{} or nil into []string.
@@ -179,42 +144,6 @@ func processModuleSettings(bp config.Blueprint, mod config.Module, group config.
 	return nil
 }
 
-// processVarsAsBlueprint processes a list of names as blueprint vars.
-func processVarsAsBlueprint(bp config.Blueprint, list []string, optional bool, handler func(Target) error) error {
-	for _, name := range list {
-		values, path, err := getBlueprintValues(bp, name)
-		if err != nil {
-			if optional {
-				continue
-			}
-			// legacy behavior: skip when missing blueprint var even if optional == false
-			continue
-		}
-		if err := handler(Target{Name: name, Values: values, Path: path, IsBlueprint: true}); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-// processVarsPreferModuleSetting prefers module.setting for each name; skips if module.setting absent.
-func processVarsPreferModuleSetting(bp config.Blueprint, mod config.Module, group config.Group, modIdx int, list []string, handler func(Target) error) error {
-	for _, vname := range list {
-		if val, ok := getNestedValue(mod.Settings, vname); ok {
-			if evaled, err := bp.Eval(val); err == nil {
-				val = evaled
-			}
-			values := evaluateAndFlatten(val)
-			path := config.Root.Groups.At(bp.GroupIndex(group.Name)).Modules.At(modIdx).Settings.Dot(vname)
-			if err := handler(Target{Name: vname, Values: values, Path: path, IsBlueprint: false}); err != nil {
-				return err
-			}
-		}
-		// else: skip (do not fallback to blueprint var)
-	}
-	return nil
-}
-
 // IterateRuleTargets resolves vars/settings from a validation rule according to scope and optional semantics,
 // and calls the provided handler for each resolved Target. The handler may return an error to stop iteration.
 func IterateRuleTargets(
@@ -226,32 +155,16 @@ func IterateRuleTargets(
 	handler func(Target) error,
 ) error {
 
+	// Only support `vars:` in module metadata validators (each treated as a module.setting name).
+	// `settings:` support has been removed to enforce a single convention.
 	varsList, _ := parseStringList(rule.Inputs["vars"])
-	settingsList, _ := parseStringList(rule.Inputs["settings"])
-	scope, _ := rule.Inputs["scope"].(string)
 	optional := true
 	if v, ok := rule.Inputs["optional"]; ok {
 		if b, ok := v.(bool); ok {
 			optional = b
 		}
 	}
 
-	switch scope {
-	case "module":
-		if len(settingsList) > 0 {
-			return processModuleSettings(bp, mod, group, modIdx, settingsList, optional, handler)
-		}
-		return processModuleSettings(bp, mod, group, modIdx, varsList, optional, handler)
-
-	case "blueprint":
-		return processVarsAsBlueprint(bp, varsList, optional, handler)
-
-	default:
-		if len(settingsList) > 0 {
-			if err := processModuleSettings(bp, mod, group, modIdx, settingsList, optional, handler); err != nil {
-				return err
-			}
-		}
-		return processVarsPreferModuleSetting(bp, mod, group, modIdx, varsList, handler)
-	}
+	// Interpret each var name as module.settings.<name>
+	return processModuleSettings(bp, mod, group, modIdx, varsList, optional, handler)
 }