From 7a67472fc168e49bd589b7d7292d6d28f9e10c94 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Wed, 5 Aug 2015 18:04:34 +0200 Subject: [PATCH 1/2] Resolve relative paths on configuration loading This moves the concern of resolving the files relative to the config file into the configuration loading itself. It also fixes #921 which did not load the cert and token files relatively. --- cmd/prometheus/main.go | 2 -- config/config.go | 32 +++++++++++++++++++++++++++++++- config/config_test.go | 12 +++++++----- config/testdata/conf.good.yml | 4 +++- rules/manager.go | 7 ------- 5 files changed, 41 insertions(+), 16 deletions(-) diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go index 495254b9d..214ba7d75 100644 --- a/cmd/prometheus/main.go +++ b/cmd/prometheus/main.go @@ -21,7 +21,6 @@ import ( _ "net/http/pprof" // Comment this line to disable pprof endpoint. "os" "os/signal" - "path/filepath" "strings" "syscall" "text/template" @@ -77,7 +76,6 @@ func Main() int { NotificationHandler: notificationHandler, QueryEngine: queryEngine, ExternalURL: cfg.web.ExternalURL, - BaseDir: filepath.Dir(cfg.configFile), }) flags := map[string]string{} diff --git a/config/config.go b/config/config.go index 136a4f898..dc0fdca7e 100644 --- a/config/config.go +++ b/config/config.go @@ -5,6 +5,7 @@ import ( "fmt" "io/ioutil" "net/url" + "path/filepath" "regexp" "strings" "time" @@ -45,7 +46,12 @@ func LoadFromFile(filename string) (*Config, error) { if err != nil { return nil, err } - return Load(string(content)) + cfg, err := Load(string(content)) + if err != nil { + return nil, err + } + resolveFilepaths(filepath.Dir(filename), cfg) + return cfg, nil } // The defaults applied before parsing the respective config sections. @@ -113,6 +119,30 @@ type Config struct { original string } +// resolveFilepaths joins all relative paths in a configuration +// with a given base directory. +func resolveFilepaths(baseDir string, cfg *Config) { + join := func(fp string) string { + if len(fp) > 0 && !filepath.IsAbs(fp) { + fp = filepath.Join(baseDir, fp) + } + return fp + } + + for i, rf := range cfg.RuleFiles { + cfg.RuleFiles[i] = join(rf) + } + + for _, scfg := range cfg.ScrapeConfigs { + scfg.BearerTokenFile = join(scfg.BearerTokenFile) + + if scfg.ClientCert != nil { + scfg.ClientCert.Cert = join(scfg.ClientCert.Cert) + scfg.ClientCert.Key = join(scfg.ClientCert.Key) + } + } +} + func checkOverflow(m map[string]interface{}, ctx string) error { if len(m) > 0 { var keys []string diff --git a/config/config_test.go b/config/config_test.go index 64436fb36..5284699fa 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -27,9 +27,9 @@ var expectedConf = &Config{ }, RuleFiles: []string{ - "first.rules", - "second.rules", - "my/*.rules", + "testdata/first.rules", + "/absolute/second.rules", + "testdata/my/*.rules", }, ScrapeConfigs: []*ScrapeConfig{ @@ -43,6 +43,8 @@ var expectedConf = &Config{ MetricsPath: DefaultScrapeConfig.MetricsPath, Scheme: DefaultScrapeConfig.Scheme, + BearerTokenFile: "testdata/valid_token_file", + TargetGroups: []*TargetGroup{ { Targets: []clientmodel.LabelSet{ @@ -167,8 +169,8 @@ var expectedConf = &Config{ Scheme: "http", ClientCert: &ClientCert{ - Cert: "valid_cert_file", - Key: "valid_key_file", + Cert: "testdata/valid_cert_file", + Key: "testdata/valid_key_file", }, BearerToken: "avalidtoken", }, diff --git a/config/testdata/conf.good.yml b/config/testdata/conf.good.yml index df8cf62c6..33475960f 100644 --- a/config/testdata/conf.good.yml +++ b/config/testdata/conf.good.yml @@ -10,7 +10,7 @@ global: rule_files: - "first.rules" -- "second.rules" +- "/absolute/second.rules" - "my/*.rules" scrape_configs: @@ -45,6 +45,8 @@ scrape_configs: replacement: foo-${1} # action defaults to 'replace' + bearer_token_file: valid_token_file + - job_name: service-x diff --git a/rules/manager.go b/rules/manager.go index c862fe677..1dccfcf5c 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -104,7 +104,6 @@ type Manager struct { notificationHandler *notification.NotificationHandler externalURL *url.URL - baseDir string } // ManagerOptions bundles options for the Manager. @@ -116,7 +115,6 @@ type ManagerOptions struct { SampleAppender storage.SampleAppender ExternalURL *url.URL - BaseDir string } // NewManager returns an implementation of Manager, ready to be started @@ -131,7 +129,6 @@ func NewManager(o *ManagerOptions) *Manager { queryEngine: o.QueryEngine, notificationHandler: o.NotificationHandler, externalURL: o.ExternalURL, - baseDir: o.BaseDir, } return manager } @@ -330,10 +327,6 @@ func (m *Manager) ApplyConfig(conf *config.Config) bool { var files []string for _, pat := range conf.RuleFiles { - if !filepath.IsAbs(pat) { - pat = filepath.Join(m.baseDir, pat) - } - fs, err := filepath.Glob(pat) if err != nil { // The only error can be a bad pattern. From 73f1cc807d2aace53e804dab7e6122da0ad40587 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Wed, 5 Aug 2015 18:30:37 +0200 Subject: [PATCH 2/2] Check token and cert file existence in promtool --- cmd/prometheus/main.go | 2 +- cmd/promtool/main.go | 45 ++++++++++++++++++++++++++++++------------ config/config.go | 4 ++-- config/config_test.go | 6 +++--- 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go index 214ba7d75..f8d8aa993 100644 --- a/cmd/prometheus/main.go +++ b/cmd/prometheus/main.go @@ -170,7 +170,7 @@ type Reloadable interface { func reloadConfig(filename string, rls ...Reloadable) bool { log.Infof("Loading configuration file %s", filename) - conf, err := config.LoadFromFile(filename) + conf, err := config.LoadFile(filename) if err != nil { log.Errorf("Couldn't load configuration (-config.file=%s): %v", filename, err) log.Errorf("Note: The configuration format has changed with version 0.14. Please see the documentation (http://prometheus.io/docs/operating/configuration/) and the provided configuration migration tool (https://github.com/prometheus/migrate).") diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go index 791fa069f..b630b6166 100644 --- a/cmd/promtool/main.go +++ b/cmd/promtool/main.go @@ -22,8 +22,6 @@ import ( "strings" "text/template" - "gopkg.in/yaml.v2" - "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/util/cli" @@ -73,32 +71,53 @@ func checkConfig(t cli.Term, filename string) ([]string, error) { return nil, fmt.Errorf("is a directory") } - content, err := ioutil.ReadFile(filename) + cfg, err := config.LoadFile(filename) if err != nil { return nil, err } - var cfg config.Config - if err := yaml.Unmarshal(content, &cfg); err != nil { - return nil, err + check := func(fn string) error { + // Nothing set, nothing to error on. + if fn == "" { + return nil + } + _, err := os.Stat(fn) + return err } + var ruleFiles []string for _, rf := range cfg.RuleFiles { - if !filepath.IsAbs(rf) { - rf = filepath.Join(filepath.Dir(filename), rf) - } - rfs, err := filepath.Glob(rf) if err != nil { return nil, err } - // If an explicit file was given, error if it doesn't exist. - if !strings.Contains(rf, "*") && len(rfs) == 0 { - return nil, fmt.Errorf("%q does not point to an existing file", rf) + // If an explicit file was given, error if it is not accessible. + if !strings.Contains(rf, "*") { + if len(rfs) == 0 { + return nil, fmt.Errorf("%q does not point to an existing file", rf) + } + if err := check(rfs[0]); err != nil { + return nil, fmt.Errorf("error checking rule file %q: %s", rfs[0], err) + } } ruleFiles = append(ruleFiles, rfs...) } + for _, scfg := range cfg.ScrapeConfigs { + if err := check(scfg.BearerTokenFile); err != nil { + return nil, fmt.Errorf("error checking bearer token file %q: %s", scfg.BearerTokenFile, err) + } + + if scfg.ClientCert != nil { + if err := check(scfg.ClientCert.Cert); err != nil { + return nil, fmt.Errorf("error checking client cert file %q: %s", scfg.ClientCert.Cert, err) + } + if err := check(scfg.ClientCert.Key); err != nil { + return nil, fmt.Errorf("error checking client key file %q: %s", scfg.ClientCert.Key, err) + } + } + } + return ruleFiles, nil } diff --git a/config/config.go b/config/config.go index dc0fdca7e..34b9362c8 100644 --- a/config/config.go +++ b/config/config.go @@ -40,8 +40,8 @@ func Load(s string) (*Config, error) { return cfg, nil } -// LoadFromFile parses the given YAML file into a Config. -func LoadFromFile(filename string) (*Config, error) { +// LoadFile parses the given YAML file into a Config. +func LoadFile(filename string) (*Config, error) { content, err := ioutil.ReadFile(filename) if err != nil { return nil, err diff --git a/config/config_test.go b/config/config_test.go index 5284699fa..16ab2c604 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -181,11 +181,11 @@ var expectedConf = &Config{ func TestLoadConfig(t *testing.T) { // Parse a valid file that sets a global scrape timeout. This tests whether parsing // an overwritten default field in the global config permanently changes the default. - if _, err := LoadFromFile("testdata/global_timeout.good.yml"); err != nil { + if _, err := LoadFile("testdata/global_timeout.good.yml"); err != nil { t.Errorf("Error parsing %s: %s", "testdata/conf.good.yml", err) } - c, err := LoadFromFile("testdata/conf.good.yml") + c, err := LoadFile("testdata/conf.good.yml") if err != nil { t.Fatalf("Error parsing %s: %s", "testdata/conf.good.yml", err) } @@ -252,7 +252,7 @@ var expectedErrors = []struct { func TestBadConfigs(t *testing.T) { for _, ee := range expectedErrors { - _, err := LoadFromFile("testdata/" + ee.filename) + _, err := LoadFile("testdata/" + ee.filename) if err == nil { t.Errorf("Expected error parsing %s but got none", ee.filename) continue