Skip to content

Commit 19ef646

Browse files
FuFikCZVlk, Ondrej
FuFikCZ
and
Vlk, Ondrej
authored
fix: prometheusIngester headers from env (#96)
* fix: prometheusIngester headers from env original implementation counted with viper using yaml library and UnmarshalYAML interface. It was naive, viper uses mapstructure instead of yaml package Co-authored-by: Vlk, Ondrej <[email protected]>
1 parent 49f361a commit 19ef646

File tree

3 files changed

+128
-128
lines changed

3 files changed

+128
-128
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
55
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
66

77
## Unreleased
8+
### Fixed
9+
- [#96](https://github.com/seznam/slo-exporter/pull/96) prometheusIngester headers from environment value now works
810

911
## [v6.12.0] 2022-10-06
1012
### Added

‎pkg/prometheus_ingester/prometheus_ingester.go‎

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -77,60 +77,52 @@ type queryOptions struct {
7777
}
7878

7979
type httpHeaderValueFromEnv struct {
80-
Name string `yaml:"name"`
81-
ValuePrefix string `yaml:"valuePrefix"`
80+
Name string
81+
ValuePrefix string
8282
}
8383

8484
type httpHeader struct {
85-
Name string `yaml:"name"`
86-
ValueFromEnv *httpHeaderValueFromEnv `yaml:"valueFromEnv"`
87-
Value *string `yaml:"value"`
85+
Name string
86+
ValueFromEnv *httpHeaderValueFromEnv
87+
Value *string
8888
}
8989

90-
func (h *httpHeader) UnmarshalYAML(unmarshal func(interface{}) error) error {
91-
// we have to declare new type in order to avoid infinite recursion when using unmarshal function
92-
type httpHeaderCustom httpHeader
93-
var out httpHeaderCustom
94-
95-
if err := unmarshal(&out); err != nil {
96-
return err
97-
}
98-
99-
*h = httpHeader(out)
100-
90+
func (h *httpHeader) getValue() (string, error) {
10191
if h.Name == "" {
102-
return fmt.Errorf("header name must be set")
92+
return "", fmt.Errorf("header name must be set")
10393
}
10494

10595
if (h.ValueFromEnv == nil) == (h.Value == nil) {
106-
return fmt.Errorf("exactly one of 'Value' or 'ValueFromEnv' must be set")
96+
return "", fmt.Errorf("exactly one of 'Value' or 'ValueFromEnv' must be set")
10797
}
10898

10999
if h.ValueFromEnv != nil {
110100
if h.ValueFromEnv.Name == "" {
111-
return fmt.Errorf("environment variable name is not set")
101+
return "", fmt.Errorf("environment variable name is not set")
112102
}
113103
value, ok := os.LookupEnv(h.ValueFromEnv.Name)
114104
if !ok {
115-
return fmt.Errorf("environment variable '%s' is not set", h.ValueFromEnv.Name)
105+
return "", fmt.Errorf("environment variable '%s' is not set", h.ValueFromEnv.Name)
116106
}
117-
value = h.ValueFromEnv.ValuePrefix + value
118-
h.Value = &value
107+
return h.ValueFromEnv.ValuePrefix + value, nil
119108
}
120109

121-
return nil
122-
110+
return *h.Value, nil
123111
}
124112

125113
type httpHeaders []httpHeader
126114

127-
func (hs httpHeaders) toMap() map[string]string {
128-
httpHeaders := map[string]string{}
115+
func (hs httpHeaders) toMap() (map[string]string, error) {
116+
headersMap := map[string]string{}
129117
for _, h := range hs {
130-
httpHeaders[h.Name] = *h.Value
118+
value, err := h.getValue()
119+
if err != nil {
120+
return nil, err
121+
}
122+
headersMap[h.Name] = value
131123
}
132124

133-
return httpHeaders
125+
return headersMap, nil
134126
}
135127

136128
type PrometheusIngesterConfig struct {
@@ -215,11 +207,16 @@ func New(initConfig PrometheusIngesterConfig, logger logrus.FieldLogger) (*Prome
215207
ingester = PrometheusIngester{}
216208
)
217209

210+
headers, err := initConfig.HttpHeaders.toMap()
211+
if err != nil {
212+
return nil, err
213+
}
214+
218215
client, err := api.NewClient(api.Config{
219216
Address: initConfig.ApiUrl,
220217
RoundTripper: httpHeadersRoundTripper{
221218
roudTripper: initConfig.RoundTripper,
222-
headers: initConfig.HttpHeaders.toMap(),
219+
headers: headers,
223220
},
224221
})
225222
if err != nil {

‎pkg/prometheus_ingester/prometheus_ingester_test.go‎

Lines changed: 100 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,16 @@ import (
88
"io/ioutil"
99
"net/http"
1010
"os"
11+
"strings"
1112
"testing"
1213
"time"
1314

1415
"github.com/prometheus/common/model"
1516
"github.com/seznam/slo-exporter/pkg/event"
1617
"github.com/seznam/slo-exporter/pkg/stringmap"
1718
"github.com/sirupsen/logrus"
19+
"github.com/spf13/viper"
1820
"github.com/stretchr/testify/assert"
19-
"gopkg.in/yaml.v2"
2021
)
2122

2223
type MockedRoundTripper struct {
@@ -721,133 +722,133 @@ func Test_processHistogramIncrease(t *testing.T) {
721722
}
722723
}
723724

724-
func TestPrometheusIngesterHttpHeader_UnmarshalYAML(t *testing.T) {
725-
const envName = "ENVNAME"
726-
var expectedValue = "value"
727-
var expectedValueWithPrefix = "xxxvalue"
728-
729-
if err := os.Unsetenv("NON_EXISTING_ENV_NAME"); err != nil {
730-
t.Fatal(err)
731-
}
732-
733-
if err := os.Setenv(envName, expectedValue); err != nil {
734-
t.Fatal(err)
735-
}
725+
func Test_httpHeaders_toMap(t *testing.T) {
726+
var headerValue = "value"
727+
var headerValue2 = "value2"
736728

737729
tests := []struct {
738-
name string
739-
data []byte
740-
expectedValue *string
741-
wantErr bool
730+
name string
731+
data string
732+
want map[string]string
733+
wantErr bool
742734
}{
743735
{
744-
name: "value from string",
745-
data: []byte(`
746-
name: name
747-
value: value
748-
`),
749-
expectedValue: &expectedValue,
750-
},
751-
{
752-
name: "value from env",
753-
data: []byte(`
754-
name: name
755-
valueFromEnv:
756-
name: ENVNAME
757-
`),
758-
expectedValue: &expectedValue,
759-
},
760-
{
761-
name: "value from env with no env name",
762-
data: []byte(`
763-
name: name
764-
valueFromEnv: {}
765-
`),
766-
wantErr: true,
767-
},
768-
{
769-
name: "value from env with valuePrefix",
770-
data: []byte(`
771-
name: name
772-
valueFromEnv:
773-
name: ENVNAME
774-
valuePrefix: xxx
775-
`),
776-
expectedValue: &expectedValueWithPrefix,
777-
},
778-
{
779-
name: "both valueFromString and valueFromEnv is set",
780-
data: []byte(`
781-
name: name
782-
valueFromEnv:
783-
name: ENVNAME
784-
value: value
785-
`),
786-
wantErr: true,
736+
name: "empty headers",
737+
data: `httpHeaders: []`,
738+
want: map[string]string{},
787739
},
788740
{
789-
name: "none of valueFromString or valueFromEnv is set",
790-
data: []byte("name: name"),
791-
wantErr: true,
741+
name: "multiple headers",
742+
data: `
743+
httpHeaders:
744+
- name: header1
745+
value: value
746+
- name: header2
747+
value: value2
748+
`,
749+
want: map[string]string{"header1": headerValue, "header2": headerValue2},
792750
},
793751
{
794-
name: "none of valueFromString or valueFromEnv is set",
795-
data: []byte("value: value"),
796-
wantErr: true,
752+
name: "headers overwrite",
753+
data: `
754+
httpHeaders:
755+
- name: header1
756+
value: value
757+
- name: header2
758+
value: value2
759+
- name: header1
760+
value: value2
761+
`,
762+
want: map[string]string{"header1": headerValue2, "header2": headerValue2},
797763
},
798764
{
799-
name: "non existing environment variable with name valueFromEnv",
800-
data: []byte(`
801-
name: name
802-
valueFromEnv:
803-
name: NON_EXISTING_ENV_NAME
804-
`),
765+
name: "validate fail - no header name",
766+
data: `
767+
httpHeaders:
768+
- value: value
769+
`,
805770
wantErr: true,
806771
},
807772
}
808773
for _, tt := range tests {
809774
t.Run(tt.name, func(t *testing.T) {
810775

811-
h := httpHeader{}
812-
err := yaml.Unmarshal(tt.data, &h)
813-
if (err != nil) != tt.wantErr {
814-
t.Errorf("PrometheusIngesterHttpHeader.UnmarshalYAML() error = %v, wantErr %v", err, tt.wantErr)
776+
var headers struct {
777+
HttpHeaders httpHeaders
815778
}
816-
if err == nil {
817-
assert.EqualValues(t, tt.expectedValue, h.Value)
779+
v := viper.New()
780+
v.SetConfigType("yaml")
781+
if err := v.ReadConfig(strings.NewReader(tt.data)); err != nil {
782+
t.Fatal(err)
818783
}
784+
if err := v.UnmarshalExact(&headers); err != nil {
785+
t.Fatal(err)
786+
}
787+
788+
got, err := headers.HttpHeaders.toMap()
789+
if (err != nil) != tt.wantErr {
790+
t.Errorf("httpHeader.getValue() error = %v, wantErr %v", err, tt.wantErr)
791+
return
792+
}
793+
assert.Equal(t, tt.want, got)
819794
})
820795
}
821796
}
822797

823-
func Test_httpHeaders_toMap(t *testing.T) {
824-
var headerValue = "value"
825-
var headerValue2 = "value2"
798+
func Test_httpHeader_getValue(t *testing.T) {
799+
var headerName = "headerName"
800+
var headerValue = "headerValue"
801+
var headerValueFromEnvValue = "headerValueFromEnv"
802+
var headerValueFromEnvPrefix = "Prefix"
803+
var envName = "envName"
804+
var nonExistingEnv = "NON_EXISTING_ENV_NAME"
826805

806+
if err := os.Unsetenv(nonExistingEnv); err != nil {
807+
t.Fatal(err)
808+
}
809+
810+
if err := os.Setenv(envName, headerValueFromEnvValue); err != nil {
811+
t.Fatal(err)
812+
}
813+
814+
type fields struct {
815+
Name string
816+
ValueFromEnv *httpHeaderValueFromEnv
817+
Value *string
818+
}
827819
tests := []struct {
828-
name string
829-
hs httpHeaders
830-
want map[string]string
820+
name string
821+
fields fields
822+
want string
823+
wantErr bool
831824
}{
825+
{name: "value", fields: fields{Name: headerName, Value: &headerValue}, want: headerValue},
826+
{name: "valueFromEnv", fields: fields{Name: headerName, ValueFromEnv: &httpHeaderValueFromEnv{Name: envName}}, want: headerValueFromEnvValue},
832827
{
833-
name: "empty headers",
834-
hs: httpHeaders{},
835-
want: map[string]string{},
836-
},
837-
{
838-
name: "multiple headers",
839-
hs: httpHeaders{{Name: "header1", Value: &headerValue}, {Name: "header2", Value: &headerValue2}},
840-
want: map[string]string{"header1": headerValue, "header2": headerValue2},
841-
},
842-
{
843-
name: "headers overwrite",
844-
hs: httpHeaders{{Name: "header1", Value: &headerValue}, {Name: "header2", Value: &headerValue2}, {Name: "header1", Value: &headerValue2}},
845-
want: map[string]string{"header1": headerValue2, "header2": headerValue2},
846-
},
828+
name: "valueFromEnv with prefix",
829+
fields: fields{
830+
Name: headerName,
831+
ValueFromEnv: &httpHeaderValueFromEnv{Name: envName, ValuePrefix: headerValueFromEnvPrefix}},
832+
want: headerValueFromEnvPrefix + headerValueFromEnvValue},
833+
{name: "valueFromEnv non existing env", fields: fields{Name: headerName, ValueFromEnv: &httpHeaderValueFromEnv{Name: nonExistingEnv}}, wantErr: true},
834+
{name: "valueFromEnv no env name set", fields: fields{Name: headerName, ValueFromEnv: &httpHeaderValueFromEnv{}}, wantErr: true},
835+
{name: "header name not set", fields: fields{Name: "", Value: &headerValue}, wantErr: true},
836+
{name: "no value neither valueFromEnv set", fields: fields{Name: headerName}, wantErr: true},
837+
{name: "value and valueFromEnv", fields: fields{Name: headerName, ValueFromEnv: &httpHeaderValueFromEnv{}, Value: &headerValue}, wantErr: true},
847838
}
848839
for _, tt := range tests {
849840
t.Run(tt.name, func(t *testing.T) {
850-
assert.Equal(t, tt.want, tt.hs.toMap())
841+
h := &httpHeader{
842+
Name: tt.fields.Name,
843+
ValueFromEnv: tt.fields.ValueFromEnv,
844+
Value: tt.fields.Value,
845+
}
846+
got, err := h.getValue()
847+
if (err != nil) != tt.wantErr {
848+
t.Errorf("httpHeader.getValue() error = %v, wantErr %v", err, tt.wantErr)
849+
return
850+
}
851+
assert.Equal(t, tt.want, got)
851852
})
852853
}
853854
}

0 commit comments

Comments
 (0)