pkg/settings: fix SettingMap.GetOrDefault

Author: jmank88

pkg/settings/cresettings/settings.go

@@ -40,6 +40,8 @@ func reinit() {
 		if err != nil {
 			log.Fatalf("failed to initialize settings: %v", err)
 		}
+	} else {
+		DefaultGetter = nil
 	}
 }
 

pkg/settings/cresettings/settings_test.go

@@ -199,6 +199,10 @@ func TestDefaultGetter_SettingMap(t *testing.T) {
 	}
 }`)
 	reinit() // set default vars
+
+	// ensure merged values; defaults must remain
+	require.Equal(t, "true", Default.PerWorkflow.ChainAllowed.Values["3379446385462418246"])
+	// confirm
 	got, err = limit.GetOrDefault(ctx, DefaultGetter)
 	require.NoError(t, err)
 	require.False(t, got)
@@ -233,13 +237,62 @@ func TestDefaultGetter_SettingMap(t *testing.T) {
 	require.True(t, got)
 }
 
-func TestChainAllows(t *testing.T) {
-	gl, err := limits.MakeGateLimiter(limits.Factory{Logger: logger.Test(t)}, Default.PerWorkflow.ChainAllowed)
+func TestDefaultEnvVars(t *testing.T) {
+	// confirm defaults
+	require.Equal(t, "", Default.PerWorkflow.ChainAllowed.Values["1234"])
+	require.Equal(t, "true", Default.PerWorkflow.ChainAllowed.Values["3379446385462418246"])
+
+	t.Cleanup(reinit) // restore after
+
+	// update defaults
+	t.Setenv(envNameSettingsDefault, `{
+	"PerWorkflow": {
+		"ChainAllowed": {
+			"Values": {
+				"1234": "true"
+			}
+		}
+	}
+}`)
+	reinit() // set default vars
+
+	// confirm through Default
+	require.Equal(t, "true", Default.PerWorkflow.ChainAllowed.Values["1234"])
+	// without affecting others (they must merge)
+	require.Equal(t, "true", Default.PerWorkflow.ChainAllowed.Values["3379446385462418246"])
+
+	// confirm through DefaultGetter
+	gl, err := limits.MakeGateLimiter(limits.Factory{Logger: logger.Test(t), Settings: DefaultGetter}, Default.PerWorkflow.ChainAllowed)
 	require.NoError(t, err)
 
-	ctx := contexts.WithCRE(t.Context(), contexts.CRE{Owner: "owner-id", Workflow: "foo"})
+	ctx := contexts.WithCRE(t.Context(), contexts.CRE{Org: "foo", Owner: "owner-id", Workflow: "foo"})
+	// defaults and global override allowed
+	assert.NoError(t, gl.AllowErr(contexts.WithChainSelector(ctx, 3379446385462418246)))
+	assert.NoError(t, gl.AllowErr(contexts.WithChainSelector(ctx, 12922642891491394802)))
+	assert.NoError(t, gl.AllowErr(contexts.WithChainSelector(ctx, 1234)))
+
+	// update overrides
+	t.Setenv(envNameSettingsDefault, "{}")
+	t.Setenv(envNameSettings, `{
+	"global": {
+		"PerWorkflow": {
+			"ChainAllowed": {
+				"Values": {
+					"1234": "true"
+				}
+			}
+		}
+	}
+}`)
+
+	reinit() // set default vars
+
+	// confirm through DefaultGetter
+	gl, err = limits.MakeGateLimiter(limits.Factory{Logger: logger.Test(t), Settings: DefaultGetter}, Default.PerWorkflow.ChainAllowed)
+	require.NoError(t, err)
 
+	// defaults and global override allowed
 	assert.NoError(t, gl.AllowErr(contexts.WithChainSelector(ctx, 3379446385462418246)))
 	assert.NoError(t, gl.AllowErr(contexts.WithChainSelector(ctx, 12922642891491394802)))
-	assert.ErrorIs(t, gl.AllowErr(contexts.WithChainSelector(ctx, 1234)), limits.ErrorNotAllowed{})
+	assert.NoError(t, gl.AllowErr(contexts.WithChainSelector(ctx, 1234)))
 }

pkg/settings/map.go

@@ -45,16 +45,20 @@ func (s *SettingMap[T]) GetOrDefault(ctx context.Context, g Getter) (value T, er
 	if err != nil {
 		return s.Default.DefaultValue, fmt.Errorf("failed to get value from context: %w", err)
 	}
-	if g == nil {
+	//TODO confirm via test
+	valueOrDefault := func() (T, error) {
 		if str, ok := s.Values[strconv.FormatUint(k, 10)]; ok {
 			value, err = s.Default.Parse(str)
 			if err != nil {
 				return s.Default.DefaultValue, err
 			}
-			return
+			return value, nil
 		}
 		return s.Default.DefaultValue, nil
 	}
+	if g == nil {
+		return valueOrDefault()
+	}
 
 	valueKey := s.Default.Key + ".Values." + strconv.FormatUint(k, 10)
 	defaultKey := s.Default.Key + ".Default"
@@ -66,20 +70,20 @@ func (s *SettingMap[T]) GetOrDefault(ctx context.Context, g Getter) (value T, er
 	} else if str != "" {
 		value, err = s.Default.Parse(str)
 		if err != nil {
-			return s.Default.DefaultValue, err
+			return valueOrDefault()
 		}
 		return
 	}
 
 	// Default override
 	str, err = g.GetScoped(ctx, s.Default.Scope, defaultKey)
 	if err != nil || str == "" {
-		return s.Default.DefaultValue, err
+		return valueOrDefault()
 	}
 
 	value, err = s.Default.Parse(str)
 	if err != nil {
-		return s.Default.DefaultValue, err
+		return valueOrDefault()
 	}
 	return
 }