Manager: erase configured variables when overlapping with implicit vars

When a variable is found in the config file, with the same name as an
implicit variable, it will be removed from the configuration (i.e. implicit
ones always win). A warning is logged when this happens.

internal/manager/config/settings.go

@@ -131,6 +131,10 @@ type Conf struct {
 	// Variable name → Variable definition
 	Variables map[string]Variable `yaml:"variables"`
 
+	// Implicit variables work as regular variables, but do not get written to the
+	// configuration file.
+	implicitVariables map[string]Variable `yaml:"-"`
+
 	// audience + platform + variable name → variable value.
 	// Used to look up variables for a given platform and audience.
 	// The 'audience' is never "all" or ""; only concrete audiences are stored here.
@@ -182,6 +186,7 @@ func DefaultConfig(override ...func(c *Conf)) Conf {
 		overrideFunc(&c)
 	}
 	c.addImplicitVariables()
+	c.ensureVariablesUnique()
 	c.constructVariableLookupTable(zerolog.TraceLevel)
 	return c
 }
@@ -225,6 +230,7 @@ func loadConf(filename string, overrides ...func(c *Conf)) (Conf, error) {
 	}
 
 	c.addImplicitVariables()
+	c.ensureVariablesUnique()
 	c.constructVariableLookupTable(zerolog.DebugLevel)
 	c.parseURLs()
 	c.checkDatabase()
@@ -235,6 +241,8 @@ func loadConf(filename string, overrides ...func(c *Conf)) (Conf, error) {
 }
 
 func (c *Conf) addImplicitVariables() {
+	c.implicitVariables = make(map[string]Variable)
+
 	if !c.Shaman.Enabled {
 		return
 	}
@@ -247,7 +255,7 @@ func (c *Conf) addImplicitVariables() {
 		log.Error().Err(err).Msg("unable to find absolute path of Shaman checkout path")
 		absPath = shamanCheckoutPath
 	}
-	c.Variables["jobs"] = Variable{
+	c.implicitVariables["jobs"] = Variable{
 		IsTwoWay: false,
 		Values: []VariableValue{
 			{
@@ -259,9 +267,33 @@ func (c *Conf) addImplicitVariables() {
 	}
 }
 
-func (c *Conf) constructVariableLookupTable(logLevel zerolog.Level) {
+// ensureVariablesUnique erases configured variables when there are implicit
-	lookup := map[VariableAudience]map[VariablePlatform]map[string]string{}
+// variables with the same name.
+func (c *Conf) ensureVariablesUnique() {
+	for varname := range c.implicitVariables {
+		if _, found := c.Variables[varname]; !found {
+			continue
+		}
+		log.Warn().Str("variable", varname).
+			Msg("configured variable will be removed, as there is an implicit variable with the same name")
+		delete(c.Variables, varname)
+	}
+}
 
+func (c *Conf) constructVariableLookupTable(logLevel zerolog.Level) {
+	if c.VariablesLookup == nil {
+		c.VariablesLookup = map[VariableAudience]map[VariablePlatform]map[string]string{}
+	}
+
+	c.constructVariableLookupTableForVars(logLevel, c.Variables)
+	c.constructVariableLookupTableForVars(logLevel, c.implicitVariables)
+
+	log.Trace().
+		Interface("variables", c.Variables).
+		Msg("constructed lookup table")
+}
+
+func (c *Conf) constructVariableLookupTableForVars(logLevel zerolog.Level, vars map[string]Variable) {
 	// Construct a list of all audiences except "" and "all"
 	concreteAudiences := []VariableAudience{}
 	isWildcard := map[VariableAudience]bool{"": true, VariableAudienceAll: true}
@@ -276,6 +308,9 @@ func (c *Conf) constructVariableLookupTable(logLevel zerolog.Level) {
 		Interface("isWildcard", isWildcard).
 		Msg("constructing variable lookup table")
 
+	// Just for brevity.
+	lookup := c.VariablesLookup
+
 	// setValue expands wildcard audiences into concrete ones.
 	var setValue func(audience VariableAudience, platform VariablePlatform, name, value string)
 	setValue = func(audience VariableAudience, platform VariablePlatform, name, value string) {
@@ -302,7 +337,7 @@ func (c *Conf) constructVariableLookupTable(logLevel zerolog.Level) {
 	}
 
 	// Construct the lookup table for each audience+platform+name
-	for name, variable := range c.Variables {
+	for name, variable := range vars {
 		log.WithLevel(logLevel).
 			Str("name", name).
 			Interface("variable", variable).
@@ -332,11 +367,6 @@ func (c *Conf) constructVariableLookupTable(logLevel zerolog.Level) {
 			}
 		}
 	}
-	log.Trace().
-		Interface("variables", c.Variables).
-		Interface("lookup", lookup).
-		Msg("constructed lookup table")
-	c.VariablesLookup = lookup
 }
 
 func updateMap[K comparable, V any](target map[K]V, updateWith map[K]V) {

internal/manager/config/settings_test.go

@@ -46,11 +46,24 @@ func TestShamanImplicitVariables(t *testing.T) {
 		// Having the Shaman enabled should create an implicit variable "{jobs}".
 		c.Shaman.Enabled = true
 		c.Shaman.StoragePath = "/path/to/shaman/storage"
+
+		c.Variables["jobs"] = Variable{
+			IsTwoWay: true,
+			Values: []VariableValue{
+				{
+					Audience: VariableAudienceAll,
+					Platform: VariablePlatformAll,
+					Value:    "this value should not be seen",
+				},
+			},
+		}
+
 	})
 
-	if !assert.Contains(t, c.Variables, "jobs") {
+	assert.NotContains(t, c.Variables, "jobs", "implicit variables should erase existing variables with the same name")
+	if !assert.Contains(t, c.implicitVariables, "jobs") {
 		t.FailNow()
 	}
-	assert.False(t, c.Variables["jobs"].IsTwoWay)
+	assert.False(t, c.implicitVariables["jobs"].IsTwoWay)
-	assert.Equal(t, c.Shaman.CheckoutPath(), c.Variables["jobs"].Values[0].Value)
+	assert.Equal(t, c.Shaman.CheckoutPath(), c.implicitVariables["jobs"].Values[0].Value)
 }