make config overrides compatible with documented behaviour and do not…

… merge attributes
krakend · Oct 14, 2024 · 58cc00d · 58cc00d
1 parent 695185d
commit 58cc00d
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 104 deletions.
diff --git a/config/lura.go b/config/lura.go
@@ -61,3 +61,22 @@ func LuraExtraCfg(extraCfg luraconfig.ExtraConfig) (*ConfigData, error) {
 
 	return cfg, nil
 }
+
+func LuraLayerExtraCfg(extraCfg luraconfig.ExtraConfig) (*LayersOpts, error) {
+	tmp, ok := extraCfg[Namespace]
+	if !ok {
+		return nil, ErrNoConfig
+	}
+
+	buf := new(bytes.Buffer)
+	if err := json.NewEncoder(buf).Encode(tmp); err != nil {
+		return nil, err
+	}
+
+	cfg := new(LayersOpts)
+	if err := json.NewDecoder(buf).Decode(cfg); err != nil {
+		return nil, err
+	}
+
+	return cfg, nil
+}
diff --git a/state/config.go b/state/config.go
@@ -7,11 +7,20 @@ import (
 
 type Config interface {
 	OTEL() OTEL
+	// GlobalOpts gets the configuration at the service level.
 	GlobalOpts() *config.GlobalOpts
 
 	// Gets the OTEL instance for a given endpoint
 	EndpointOTEL(cfg *luraconfig.EndpointConfig) OTEL
+	// EndpointPipeOpts retrieve "proxy" level configuration for a given
+	// endpoint.
 	EndpointPipeOpts(cfg *luraconfig.EndpointConfig) *config.PipeOpts
+	// EndpointBackendOpts should return a config for all the child
+	// backend of this endpoint.
+	//
+	// Deprecated: the interface should only need to fetch the BackendOpts
+	// from a luraconfig.Backend when configuring at the Backend level:
+	// the BackendOpts function must be used instead.
 	EndpointBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts
 
 	BackendOTEL(cfg *luraconfig.Backend) OTEL
@@ -39,44 +48,30 @@ func (*StateConfig) EndpointOTEL(_ *luraconfig.EndpointConfig) OTEL {
 	return GlobalState()
 }
 
+// EndpointPipeOpts checks if there is an override for pipe ("proxy")
+// options at the endpoint levels a fully replaces (it DOES NOT MERGE
+// attributes) the existing config from the service level configuration.
+// If none of those configs are found, it falls back to the defaults.
 func (s *StateConfig) EndpointPipeOpts(cfg *luraconfig.EndpointConfig) *config.PipeOpts {
-	var sOpts *config.PipeOpts
-	var extraPOpts *config.PipeOpts
-
+	var opts *config.PipeOpts
 	if s != nil && s.cfgData.Layers != nil {
-		sOpts = s.cfgData.Layers.Pipe
+		opts = s.cfgData.Layers.Pipe
 	}
 
-	cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig)
-	if err == nil && cfgExtra != nil && cfgExtra.Layers != nil {
-		extraPOpts = cfgExtra.Layers.Pipe
+	cfgLayer, err := config.LuraLayerExtraCfg(cfg.ExtraConfig)
+	if err == nil && cfgLayer != nil {
+		opts = cfgLayer.Pipe
 	}
 
-	if extraPOpts == nil {
-		if sOpts == nil {
-			return new(config.PipeOpts)
-		}
-		return sOpts
-	} else if sOpts == nil {
-		return extraPOpts
+	if opts == nil {
+		return new(config.PipeOpts)
 	}
-
-	pOpts := new(config.PipeOpts)
-	*pOpts = *sOpts
-
-	pOpts.MetricsStaticAttributes = append(
-		pOpts.MetricsStaticAttributes,
-		cfgExtra.Layers.Pipe.MetricsStaticAttributes...,
-	)
-
-	pOpts.TracesStaticAttributes = append(
-		pOpts.TracesStaticAttributes,
-		cfgExtra.Layers.Pipe.TracesStaticAttributes...,
-	)
-
-	return pOpts
+	return opts
 }
 
+// EndpointBackendOpts is a bad interface function, as is should receive
+// as a param a luraconfig.Endpoint .. but also makes no sense to have it
+// because we only need the backend configuration at
 func (s *StateConfig) EndpointBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts {
 	return s.mergedBackendOpts(cfg)
 }
@@ -90,45 +85,20 @@ func (s *StateConfig) BackendOpts(cfg *luraconfig.Backend) *config.BackendOpts {
 }
 
 func (s *StateConfig) mergedBackendOpts(cfg *luraconfig.Backend) *config.BackendOpts {
-	var extraBOpts *config.BackendOpts
-	var sOpts *config.BackendOpts
-
+	var opts *config.BackendOpts
 	if s != nil && s.cfgData.Layers != nil {
-		sOpts = s.cfgData.Layers.Backend
+		opts = s.cfgData.Layers.Backend
 	}
 
-	cfgExtra, err := config.LuraExtraCfg(cfg.ExtraConfig)
-	if err == nil && cfgExtra != nil && cfgExtra.Layers != nil {
-		extraBOpts = cfgExtra.Layers.Backend
+	cfgLayer, err := config.LuraLayerExtraCfg(cfg.ExtraConfig)
+	if err == nil && cfgLayer != nil {
+		opts = cfgLayer.Backend
 	}
 
-	if extraBOpts == nil {
-		if sOpts == nil {
-			return new(config.BackendOpts)
-		}
-		return sOpts
-	} else if sOpts == nil {
-		return extraBOpts
-	}
-
-	bOpts := new(config.BackendOpts)
-	*bOpts = *sOpts
-
-	if extraBOpts.Metrics != nil {
-		bOpts.Metrics.StaticAttributes = append(
-			bOpts.Metrics.StaticAttributes,
-			cfgExtra.Layers.Backend.Metrics.StaticAttributes...,
-		)
+	if opts == nil {
+		return new(config.BackendOpts)
 	}
-
-	if extraBOpts.Traces != nil {
-		bOpts.Traces.StaticAttributes = append(
-			bOpts.Traces.StaticAttributes,
-			cfgExtra.Layers.Backend.Traces.StaticAttributes...,
-		)
-	}
-
-	return bOpts
+	return opts
 }
 
 func (s *StateConfig) SkipEndpoint(endpoint string) bool {

diff --git a/state/config_test.go b/state/config_test.go
@@ -10,38 +10,43 @@ import (
 func TestEndpointPipeConfigOverride(t *testing.T) {
 	globalMetricAttrs := makeGlobalMetricAttr()
 	overrideMetricAttrs := makeOverrideMetricAttr()
-	expectedMetricAttrs := append(globalMetricAttrs, overrideMetricAttrs...) // skipcq: CRT-D0001
 
 	globalTraceAttrs := makeGlobalTraceAttr()
 	overrideTraceAttrs := makeOverrideTraceAttr()
-	expectedTraceAttrs := append(globalTraceAttrs, overrideTraceAttrs...) // skipcq: CRT-D0001
 
 	stateCfg := &StateConfig{
 		cfgData: makePipeConf(globalMetricAttrs, globalTraceAttrs),
 	}
 
 	pipeCfg := &luraconfig.EndpointConfig{
 		ExtraConfig: map[string]interface{}{
-			"telemetry/opentelemetry": makePipeConf(overrideMetricAttrs, overrideTraceAttrs),
+			"telemetry/opentelemetry": map[string]interface{}{
+				"proxy": map[string]interface{}{
+					"metrics_static_attributes": overrideMetricAttrs,
+					"traces_static_attributes":  overrideTraceAttrs,
+				},
+			},
 		},
 	}
 
 	pipeOpts := stateCfg.EndpointPipeOpts(pipeCfg)
+	if pipeOpts == nil {
+		t.Errorf("Unexpected nil for pipe opts")
+		return
+	}
 
-	if len(pipeOpts.MetricsStaticAttributes) != len(expectedMetricAttrs) {
+	if len(pipeOpts.MetricsStaticAttributes) != len(overrideMetricAttrs) {
 		t.Errorf(
 			"Incorrect number of attributes for metrics. returned: %+v - expected: %+v",
-			pipeOpts.MetricsStaticAttributes,
-			expectedMetricAttrs,
-		)
+			pipeOpts.MetricsStaticAttributes, overrideMetricAttrs)
+		return
 	}
 
-	if len(pipeOpts.TracesStaticAttributes) != len(expectedTraceAttrs) {
+	if len(pipeOpts.TracesStaticAttributes) != len(overrideTraceAttrs) {
 		t.Errorf(
 			"Incorrect number of attributes for traces. returned: %+v - expected: %+v",
-			pipeOpts.TracesStaticAttributes,
-			expectedTraceAttrs,
-		)
+			pipeOpts.TracesStaticAttributes, overrideTraceAttrs)
+		return
 	}
 }
 
@@ -56,12 +61,17 @@ func TestEndpointPipeNoOverride(t *testing.T) {
 	}
 
 	pipeOpts := stateCfg.EndpointPipeOpts(pipeCfg)
+	if pipeOpts == nil {
+		t.Errorf("unextpected nil pipeOpts")
+		return
+	}
 
 	if len(pipeOpts.MetricsStaticAttributes) > 1 {
 		t.Errorf(
 			"Incorrect number of attributes for metrics. returned: %+v",
 			pipeOpts.MetricsStaticAttributes,
 		)
+		return
 	}
 }
 
@@ -83,44 +93,63 @@ func TestEndpointPipeConfigOnlyOverride(t *testing.T) {
 			"Incorrect number of attributes for metrics. returned: %+v",
 			pipeOpts.MetricsStaticAttributes,
 		)
+		return
 	}
 }
 
 func TestBackendConfigOverride(t *testing.T) {
 	globalMetricAttrs := makeGlobalMetricAttr()
 	overrideMetricAttrs := makeOverrideMetricAttr()
-	expectedMetricAttrs := append(globalMetricAttrs, overrideMetricAttrs...) // skipcq: CRT-D0001
 
 	globalTraceAttrs := makeGlobalTraceAttr()
 	overrideTraceAttrs := makeOverrideTraceAttr()
-	expectedTraceAttrs := append(globalTraceAttrs, overrideTraceAttrs...) // skipcq: CRT-D0001
 
 	stateCfg := &StateConfig{
 		cfgData: makeBackendConf(globalMetricAttrs, globalTraceAttrs),
 	}
 
 	backendCfg := &luraconfig.Backend{
 		ExtraConfig: map[string]interface{}{
-			"telemetry/opentelemetry": makeBackendConf(overrideMetricAttrs, overrideTraceAttrs),
+			"telemetry/opentelemetry": map[string]interface{}{
+				"backend": map[string]interface{}{
+					"metrics": map[string]interface{}{
+						"static_attributes": overrideMetricAttrs,
+					},
+					"traces": map[string]interface{}{
+						"static_attributes": overrideTraceAttrs,
+					},
+				},
+			},
 		},
 	}
 
 	backendOpts := stateCfg.BackendOpts(backendCfg)
+	if backendOpts == nil {
+		t.Errorf("unexpected nil backendOpts")
+		return
+	}
+
+	if backendOpts.Metrics == nil {
+		t.Errorf("unexpected nil backendOpts.Metrics")
+		return
+	}
 
-	if len(backendOpts.Metrics.StaticAttributes) != len(expectedMetricAttrs) {
+	if len(backendOpts.Metrics.StaticAttributes) != len(overrideMetricAttrs) {
 		t.Errorf(
 			"Incorrect number of attributes for metrics. returned: %+v - expected: %+v",
-			backendOpts.Metrics.StaticAttributes,
-			expectedMetricAttrs,
-		)
+			backendOpts.Metrics.StaticAttributes, overrideMetricAttrs)
+		return
 	}
 
-	if len(backendOpts.Traces.StaticAttributes) != len(expectedTraceAttrs) {
+	if backendOpts.Traces == nil {
+		t.Errorf("unexpected nil backendOpts.Traces")
+		return
+	}
+	if len(backendOpts.Traces.StaticAttributes) != len(overrideTraceAttrs) {
 		t.Errorf(
 			"Incorrect number of attributes for traces. returned: %+v - expected: %+v",
-			backendOpts.Traces.StaticAttributes,
-			expectedTraceAttrs,
-		)
+			backendOpts.Traces.StaticAttributes, overrideTraceAttrs)
+		return
 	}
 }
 
@@ -135,28 +164,11 @@ func TestBackendConfigNoOverride(t *testing.T) {
 	}
 
 	backendOpts := stateCfg.BackendOpts(backendCfg)
-
-	if len(backendOpts.Metrics.StaticAttributes) > 1 {
-		t.Errorf(
-			"Incorrect number of attributes for metrics. returned: %+v",
-			backendOpts.Traces.StaticAttributes,
-		)
-	}
-}
-
-func TestBackendConfigOnlyOverride(t *testing.T) {
-	stateCfg := &StateConfig{
-		cfgData: makeBackendConf([]config.KeyValue{}, []config.KeyValue{}),
+	if backendOpts == nil {
+		t.Errorf("unexpected nil backendOpts")
+		return
 	}
 
-	backendCfg := &luraconfig.Backend{
-		ExtraConfig: map[string]interface{}{
-			"telemetry/opentelemetry": makeBackendConf(makeOverrideMetricAttr(), makeOverrideTraceAttr()),
-		},
-	}
-
-	backendOpts := stateCfg.BackendOpts(backendCfg)
-
 	if len(backendOpts.Metrics.StaticAttributes) > 1 {
 		t.Errorf(
 			"Incorrect number of attributes for metrics. returned: %+v",