Skip to content

Commit

Permalink
If env var is not defined, just log a warning
Browse files Browse the repository at this point in the history
  • Loading branch information
manugarg committed Sep 21, 2023
1 parent 8d1e330 commit 97bf737
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 28 deletions.
2 changes: 1 addition & 1 deletion cloudprober.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func InitFromConfig(configFile string) error {
return err
}

cfg, parsedConfigStr, err := config.ParseConfig(configStr, configFormat, sysvars.Vars())
cfg, parsedConfigStr, err := config.ParseConfig(configStr, configFormat, sysvars.Vars(), globalLogger)
if err != nil {
return err
}
Expand Down
22 changes: 9 additions & 13 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var (
configFile = flag.String("config_file", "", "Config file")
)

var envRegex = regexp.MustCompile(`{{ secret:\$([^$]+) }}`)
var envRegex = regexp.MustCompile(`\*\*\$([^$]+)\*\*`)

const (
configMetadataKeyName = "cloudprober_config"
Expand Down Expand Up @@ -142,7 +142,7 @@ func DumpConfig(fileName, outFormat string, baseVars map[string]string) ([]byte,
return nil, err
}

cfg, _, err := ParseConfig(content, configFormat, baseVars)
cfg, _, err := ParseConfig(content, configFormat, baseVars, nil)
if err != nil {
return nil, err
}
Expand All @@ -164,10 +164,10 @@ func DumpConfig(fileName, outFormat string, baseVars map[string]string) ([]byte,
}

// substEnvVars substitutes environment variables in the config string.
func substEnvVars(configStr string) (string, error) {
func substEnvVars(configStr string, l *logger.Logger) string {
m := envRegex.FindAllStringSubmatch(configStr, -1)
if len(m) == 0 {
return configStr, nil
return configStr
}

var envVars []string
Expand All @@ -180,25 +180,21 @@ func substEnvVars(configStr string) (string, error) {

for _, v := range envVars {
if os.Getenv(v) == "" {
return "", fmt.Errorf("environment variable %s not defined", v)
l.Warningf("Environment variable %s not defined, skipping substitution.", v)
continue
}
configStr = envRegex.ReplaceAllString(configStr, os.Getenv(v))
}

return configStr, nil
return configStr
}

func ParseConfig(content, format string, vars map[string]string) (*configpb.ProberConfig, string, error) {
func ParseConfig(content, format string, vars map[string]string, l *logger.Logger) (*configpb.ProberConfig, string, error) {
parsedConfig, err := ParseTemplate(content, vars, nil)
if err != nil {
return nil, "", fmt.Errorf("error parsing config file as Go template. Err: %v", err)
}

finalConfigStr, err := substEnvVars(parsedConfig)
if err != nil {
return nil, "", fmt.Errorf("error substituting secret environment variables in the config string. Err: %v", err)
}

cfg, err := configToProto(finalConfigStr, format)
cfg, err := configToProto(substEnvVars(parsedConfig, l), format)
return cfg, parsedConfig, err
}
24 changes: 12 additions & 12 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@
package config

import (
"bytes"
"context"
"encoding/json"
"os"
"strings"
"testing"

configpb "github.com/cloudprober/cloudprober/config/proto"
"github.com/cloudprober/cloudprober/logger"
probespb "github.com/cloudprober/cloudprober/probes/proto"
surfacerspb "github.com/cloudprober/cloudprober/surfacers/proto"
targetspb "github.com/cloudprober/cloudprober/targets/proto"
Expand Down Expand Up @@ -226,7 +229,7 @@ func TestSubstEnvVars(t *testing.T) {
name string
configStr string
want string
wantErr bool
wantLog string
}{
{
name: "no_env_vars",
Expand All @@ -235,25 +238,22 @@ func TestSubstEnvVars(t *testing.T) {
},
{
name: "env_var",
configStr: `probe {name: "{{ secret:$SECRET_PROBE_NAME }}"}`,
configStr: `probe {name: "**$SECRET_PROBE_NAME**"}`,
want: `probe {name: "probe-x"}`,
},
{
name: "env_var_not_defined",
configStr: `probe {name: "{{ secret:$SECRET_PROBEX_NAME }}"}`,
wantErr: true,
configStr: `probe {name: "**$SECRET_PROBEX_NAME**"}`,
want: `probe {name: "**$SECRET_PROBEX_NAME**"}`,
wantLog: "SECRET_PROBEX_NAME not defined",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := substEnvVars(tt.configStr)
if (err != nil) != tt.wantErr {
t.Errorf("substEnvVars() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("substEnvVars() = %v, want %v", got, tt.want)
}
var buf bytes.Buffer
l, _ := logger.New(context.Background(), "config_test", logger.WithWriter(&buf))
assert.Equal(t, tt.want, substEnvVars(tt.configStr, l))
assert.Contains(t, buf.String(), tt.wantLog)
})
}
}
2 changes: 1 addition & 1 deletion config/config_tmpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func ParseTemplate(config string, sysVars map[string]string, getGCECustomMetadat
}
return matches[n], nil
},
"envSecret": func(s string) string { return "{{ secret:$" + s + " }}" },
"envSecret": func(s string) string { return "**$" + s + "**" },
}

for name, f := range sprig.TxtFuncMap() {
Expand Down
2 changes: 1 addition & 1 deletion config/config_tmpl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestParseTemplate(t *testing.T) {
}
}
`,
wantProbes: []string{"{{ secret:$SECRET_PROBE_NAME }}"},
wantProbes: []string{"**$SECRET_PROBE_NAME**"},
wantTargets: []string{"host_names:\"www.google.com\""},
},
{
Expand Down

0 comments on commit 97bf737

Please sign in to comment.