From 4266c3301440145f06b3d3361af095a760b71a52 Mon Sep 17 00:00:00 2001 From: Manu Garg Date: Thu, 21 Sep 2023 09:40:03 -0700 Subject: [PATCH] [config] Provide a way to hide secret environment variables. (#536) - Provide an option to specify secret environment variables, using "envSecret" keyword. - When such variables are present, /config-running will be automatically disabled. - Add another page /config-parsed that shows the parsed config and makes it clear which variables are secret. --- cloudprober.go | 24 +++++++++++++++------- config/config.go | 40 +++++++++++++++++++++++++++++++----- config/config_test.go | 42 ++++++++++++++++++++++++++++++++++++++ config/config_tmpl.go | 3 ++- config/config_tmpl_test.go | 14 +++++++++++++ web/resources/header.go | 2 +- web/web.go | 22 ++++++++++++++++++-- 7 files changed, 131 insertions(+), 16 deletions(-) diff --git a/cloudprober.go b/cloudprober.go index 393856356fb..ec136b2a479 100644 --- a/cloudprober.go +++ b/cloudprober.go @@ -68,7 +68,8 @@ var cloudProber struct { prober *prober.Prober defaultServerLn net.Listener defaultGRPCLn net.Listener - textConfig string + rawConfig string + parsedConfig string config *configpb.ProberConfig cancelInitCtx context.CancelFunc sync.Mutex @@ -161,7 +162,7 @@ func InitFromConfig(configFile string) error { return err } - cfg, err := config.ParseConfig(configStr, configFormat, sysvars.Vars()) + cfg, parsedConfigStr, err := config.ParseConfig(configStr, configFormat, sysvars.Vars(), globalLogger) if err != nil { return err } @@ -222,7 +223,8 @@ func InitFromConfig(configFile string) error { cloudProber.prober = pr cloudProber.config = cfg - cloudProber.textConfig = configStr + cloudProber.rawConfig = configStr + cloudProber.parsedConfig = parsedConfigStr cloudProber.defaultServerLn = ln cloudProber.defaultGRPCLn = grpcLn cloudProber.cancelInitCtx = cancelFunc @@ -251,7 +253,8 @@ func Start(ctx context.Context) { defer cloudProber.Unlock() cloudProber.defaultServerLn = nil cloudProber.defaultGRPCLn = nil - cloudProber.textConfig = "" + cloudProber.rawConfig = "" + cloudProber.parsedConfig = "" cloudProber.config = nil cloudProber.prober = nil }() @@ -278,11 +281,18 @@ func GetConfig() *configpb.ProberConfig { return cloudProber.config } -// GetTextConfig returns the prober config in text proto format. -func GetTextConfig() string { +// GetRawConfig returns the prober config in text proto format. +func GetRawConfig() string { cloudProber.Lock() defer cloudProber.Unlock() - return cloudProber.textConfig + return cloudProber.rawConfig +} + +// GetParsedConfig returns the parsed prober config. +func GetParsedConfig() string { + cloudProber.Lock() + defer cloudProber.Unlock() + return cloudProber.parsedConfig } // GetInfo returns information on all the probes, servers and surfacers. diff --git a/config/config.go b/config/config.go index 0cf5f45b9df..9705331775b 100644 --- a/config/config.go +++ b/config/config.go @@ -19,6 +19,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "cloud.google.com/go/compute/metadata" "github.com/cloudprober/cloudprober/common/file" @@ -33,6 +34,8 @@ var ( configFile = flag.String("config_file", "", "Config file") ) +var envRegex = regexp.MustCompile(`\*\*\$([^$]+)\*\*`) + const ( configMetadataKeyName = "cloudprober_config" defaultConfigFile = "/etc/cloudprober.cfg" @@ -139,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 } @@ -160,11 +163,38 @@ func DumpConfig(fileName, outFormat string, baseVars map[string]string) ([]byte, } } -func ParseConfig(content, format string, vars map[string]string) (*configpb.ProberConfig, error) { - configStr, err := ParseTemplate(content, vars, nil) +// substEnvVars substitutes environment variables in the config string. +func substEnvVars(configStr string, l *logger.Logger) string { + m := envRegex.FindAllStringSubmatch(configStr, -1) + if len(m) == 0 { + return configStr + } + + var envVars []string + for _, match := range m { + if len(match) != 2 { + continue + } + envVars = append(envVars, match[1]) // match[0] is the whole string. + } + + for _, v := range envVars { + if os.Getenv(v) == "" { + l.Warningf("Environment variable %s not defined, skipping substitution.", v) + continue + } + configStr = envRegex.ReplaceAllString(configStr, os.Getenv(v)) + } + + return configStr +} + +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) + return nil, "", fmt.Errorf("error parsing config file as Go template. Err: %v", err) } - return configToProto(configStr, format) + cfg, err := configToProto(substEnvVars(parsedConfig, l), format) + return cfg, parsedConfig, err } diff --git a/config/config_test.go b/config/config_test.go index 3f40bbdcd68..ce91c5b5d57 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -15,11 +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" @@ -215,3 +219,41 @@ surfacer: { }) } } + +func TestSubstEnvVars(t *testing.T) { + os.Setenv("SECRET_PROBE_NAME", "probe-x") + // Make sure this env var is not set, for error behavior testing. + os.Unsetenv("SECRET_PROBEX_NAME") + + tests := []struct { + name string + configStr string + want string + wantLog string + }{ + { + name: "no_env_vars", + configStr: `probe {name: "dns_k8s"}`, + want: `probe {name: "dns_k8s"}`, + }, + { + name: "env_var", + configStr: `probe {name: "**$SECRET_PROBE_NAME**"}`, + want: `probe {name: "probe-x"}`, + }, + { + name: "env_var_not_defined", + 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) { + 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) + }) + } +} diff --git a/config/config_tmpl.go b/config/config_tmpl.go index e0734db12cc..cf084ad9b3c 100644 --- a/config/config_tmpl.go +++ b/config/config_tmpl.go @@ -184,10 +184,11 @@ func ParseTemplate(config string, sysVars map[string]string, getGCECustomMetadat } matches := r.FindStringSubmatch(s) if len(matches) <= n { - return "", fmt.Errorf("Match number %d not found. Regex: %s, String: %s", n, re, s) + return "", fmt.Errorf("match number %d not found. Regex: %s, String: %s", n, re, s) } return matches[n], nil }, + "envSecret": func(s string) string { return "**$" + s + "**" }, } for name, f := range sprig.TxtFuncMap() { diff --git a/config/config_tmpl_test.go b/config/config_tmpl_test.go index f73502a3d49..5d5f1055932 100644 --- a/config/config_tmpl_test.go +++ b/config/config_tmpl_test.go @@ -62,6 +62,20 @@ func TestParseTemplate(t *testing.T) { wantProbes: []string{"vm-to-google-02-testRegion"}, wantTargets: []string{"host_names:\"www.google.com\""}, }, + { + desc: "config-with-secret-env", + config: ` + probe { + name: "{{ envSecret "SECRET_PROBE_NAME" }}" + type: PING + targets { + host_names: "www.google.com" + } + } + `, + wantProbes: []string{"**$SECRET_PROBE_NAME**"}, + wantTargets: []string{"host_names:\"www.google.com\""}, + }, { desc: "config-with-map-and-template", config: ` diff --git a/web/resources/header.go b/web/resources/header.go index 17d363a5f3f..c34fb6d48cf 100644 --- a/web/resources/header.go +++ b/web/resources/header.go @@ -33,7 +33,7 @@ var t = template.Must(template.New("header").Parse(` Started: {{.StartTime}} -- up {{.Uptime}}
Version: {{.Version}}
Built at: {{.BuiltAt}}
- Other Links: /status, /config (raw), /alerts, /health
+ Other Links: /status, /config (parsed | raw), /alerts, /health
`)) diff --git a/web/web.go b/web/web.go index dab3398dd8a..524bb2ea9e2 100644 --- a/web/web.go +++ b/web/web.go @@ -21,6 +21,7 @@ import ( "fmt" "html/template" "net/http" + "strings" "github.com/cloudprober/cloudprober" "github.com/cloudprober/cloudprober/common/httputils" @@ -106,12 +107,29 @@ func Init() error { return fmt.Errorf("url %s is already handled", url) } } + srvMux.HandleFunc("/config", func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, cloudprober.GetTextConfig()) + fmt.Fprint(w, cloudprober.GetRawConfig()) + }) + + parsedConfig := cloudprober.GetParsedConfig() + srvMux.HandleFunc("/config-parsed", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, cloudprober.GetParsedConfig()) }) + + var configRunning string + if !strings.Contains(parsedConfig, "{{ secret:$") { + configRunning = runningConfig() + } else { + configRunning = ` +

Config contains secrets. /config-running is not available.
+ Visit /config-parsed to see the config.

+ ` + } srvMux.HandleFunc("/config-running", func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, runningConfig()) + fmt.Fprint(w, configRunning) }) + srvMux.HandleFunc("/alerts", func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, alertsState()) })