From 42628899b5e5381ffc3c7a7b22442b0576b19504 Mon Sep 17 00:00:00 2001 From: zzehring Date: Tue, 30 Nov 2021 11:11:48 -0500 Subject: [PATCH] promtool: Add `--syntax-only` flag for `check config` This commit adds a `--syntax-only` flag for `promtool check config`. When passing in this flag, promtool will omit various file existence checks that would cause the check to fail (e.g. the check would not fail if `rule_files` files don't exist at their respective paths). This functionality will allow CI systems to check the syntax of configs without worrying about referenced files. Fixes: #5222 Signed-off-by: zzehring --- cmd/promtool/main.go | 72 ++++++++------ cmd/promtool/main_test.go | 93 ++++++++++++++++++- .../testdata/config_with_rule_files.yml | 3 + .../config_with_service_discovery_files.yml | 12 +++ .../testdata/config_with_tls_files.yml | 5 + 5 files changed, 155 insertions(+), 30 deletions(-) create mode 100644 cmd/promtool/testdata/config_with_rule_files.yml create mode 100644 cmd/promtool/testdata/config_with_service_discovery_files.yml create mode 100644 cmd/promtool/testdata/config_with_tls_files.yml diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go index e01b95eeb..93af7678c 100644 --- a/cmd/promtool/main.go +++ b/cmd/promtool/main.go @@ -73,6 +73,7 @@ func main() { "config-files", "The config files to check.", ).Required().ExistingFiles() + checkConfigSyntaxOnly := checkConfigCmd.Flag("syntax-only", "Only check the config file syntax, ignoring file and content validation referenced in the config").Bool() checkWebConfigCmd := checkCmd.Command("web-config", "Check if the web config files are valid or not.") webConfigFiles := checkWebConfigCmd.Arg( @@ -211,7 +212,7 @@ func main() { os.Exit(CheckSD(*sdConfigFile, *sdJobName, *sdTimeout)) case checkConfigCmd.FullCommand(): - os.Exit(CheckConfig(*agentMode, *configFiles...)) + os.Exit(CheckConfig(*agentMode, *checkConfigSyntaxOnly, *configFiles...)) case checkWebConfigCmd.FullCommand(): os.Exit(CheckWebConfig(*webConfigFiles...)) @@ -267,16 +268,19 @@ func main() { } // CheckConfig validates configuration files. -func CheckConfig(agentMode bool, files ...string) int { +func CheckConfig(agentMode, checkSyntaxOnly bool, files ...string) int { failed := false for _, f := range files { - ruleFiles, err := checkConfig(agentMode, f) + ruleFiles, err := checkConfig(agentMode, f, checkSyntaxOnly) if err != nil { fmt.Fprintln(os.Stderr, " FAILED:", err) failed = true } else { - fmt.Printf(" SUCCESS: %d rule files found\n", len(ruleFiles)) + if len(ruleFiles) > 0 { + fmt.Printf(" SUCCESS: %d rule files found\n", len(ruleFiles)) + } + fmt.Printf(" SUCCESS: %s is valid prometheus config file syntax\n", f) } fmt.Println() @@ -326,7 +330,7 @@ func checkFileExists(fn string) error { return err } -func checkConfig(agentMode bool, filename string) ([]string, error) { +func checkConfig(agentMode bool, filename string, checkSyntaxOnly bool) ([]string, error) { fmt.Println("Checking", filename) cfg, err := config.LoadFile(filename, agentMode, false, log.NewNopLogger()) @@ -335,41 +339,46 @@ func checkConfig(agentMode bool, filename string) ([]string, error) { } var ruleFiles []string - for _, rf := range cfg.RuleFiles { - rfs, err := filepath.Glob(rf) - if err != nil { - return nil, err - } - // If an explicit file was given, error if it is not accessible. - if !strings.Contains(rf, "*") { - if len(rfs) == 0 { - return nil, errors.Errorf("%q does not point to an existing file", rf) + if !checkSyntaxOnly { + for _, rf := range cfg.RuleFiles { + rfs, err := filepath.Glob(rf) + if err != nil { + return nil, err } - if err := checkFileExists(rfs[0]); err != nil { - return nil, errors.Wrapf(err, "error checking rule file %q", rfs[0]) + // If an explicit file was given, error if it is not accessible. + if !strings.Contains(rf, "*") { + if len(rfs) == 0 { + return nil, errors.Errorf("%q does not point to an existing file", rf) + } + if err := checkFileExists(rfs[0]); err != nil { + return nil, errors.Wrapf(err, "error checking rule file %q", rfs[0]) + } } + ruleFiles = append(ruleFiles, rfs...) } - ruleFiles = append(ruleFiles, rfs...) } for _, scfg := range cfg.ScrapeConfigs { - if scfg.HTTPClientConfig.Authorization != nil { + if !checkSyntaxOnly && scfg.HTTPClientConfig.Authorization != nil { if err := checkFileExists(scfg.HTTPClientConfig.Authorization.CredentialsFile); err != nil { return nil, errors.Wrapf(err, "error checking authorization credentials or bearer token file %q", scfg.HTTPClientConfig.Authorization.CredentialsFile) } } - if err := checkTLSConfig(scfg.HTTPClientConfig.TLSConfig); err != nil { + if err := checkTLSConfig(scfg.HTTPClientConfig.TLSConfig, checkSyntaxOnly); err != nil { return nil, err } for _, c := range scfg.ServiceDiscoveryConfigs { switch c := c.(type) { case *kubernetes.SDConfig: - if err := checkTLSConfig(c.HTTPClientConfig.TLSConfig); err != nil { + if err := checkTLSConfig(c.HTTPClientConfig.TLSConfig, checkSyntaxOnly); err != nil { return nil, err } case *file.SDConfig: + if checkSyntaxOnly { + break + } for _, file := range c.Files { files, err := filepath.Glob(file) if err != nil { @@ -403,6 +412,9 @@ func checkConfig(agentMode bool, filename string) ([]string, error) { for _, c := range amcfg.ServiceDiscoveryConfigs { switch c := c.(type) { case *file.SDConfig: + if checkSyntaxOnly { + break + } for _, file := range c.Files { files, err := filepath.Glob(file) if err != nil { @@ -434,14 +446,7 @@ func checkConfig(agentMode bool, filename string) ([]string, error) { return ruleFiles, nil } -func checkTLSConfig(tlsConfig config_util.TLSConfig) error { - if err := checkFileExists(tlsConfig.CertFile); err != nil { - return errors.Wrapf(err, "error checking client cert file %q", tlsConfig.CertFile) - } - if err := checkFileExists(tlsConfig.KeyFile); err != nil { - return errors.Wrapf(err, "error checking client key file %q", tlsConfig.KeyFile) - } - +func checkTLSConfig(tlsConfig config_util.TLSConfig, checkSyntaxOnly bool) error { if len(tlsConfig.CertFile) > 0 && len(tlsConfig.KeyFile) == 0 { return errors.Errorf("client cert file %q specified without client key file", tlsConfig.CertFile) } @@ -449,6 +454,17 @@ func checkTLSConfig(tlsConfig config_util.TLSConfig) error { return errors.Errorf("client key file %q specified without client cert file", tlsConfig.KeyFile) } + if checkSyntaxOnly { + return nil + } + + if err := checkFileExists(tlsConfig.CertFile); err != nil { + return errors.Wrapf(err, "error checking client cert file %q", tlsConfig.CertFile) + } + if err := checkFileExists(tlsConfig.KeyFile); err != nil { + return errors.Wrapf(err, "error checking client key file %q", tlsConfig.KeyFile) + } + return nil } diff --git a/cmd/promtool/main_test.go b/cmd/promtool/main_test.go index 59a15b7a7..82b5323c6 100644 --- a/cmd/promtool/main_test.go +++ b/cmd/promtool/main_test.go @@ -18,6 +18,8 @@ import ( "net/http" "net/http/httptest" "net/url" + "runtime" + "strings" "testing" "time" @@ -194,7 +196,7 @@ func TestCheckTargetConfig(t *testing.T) { } for _, test := range cases { t.Run(test.name, func(t *testing.T) { - _, err := checkConfig(false, "testdata/"+test.file) + _, err := checkConfig(false, "testdata/"+test.file, false) if test.err != "" { require.Equalf(t, test.err, err.Error(), "Expected error %q, got %q", test.err, err.Error()) return @@ -204,6 +206,93 @@ func TestCheckTargetConfig(t *testing.T) { } } +func TestCheckConfigSyntax(t *testing.T) { + cases := []struct { + name string + file string + syntaxOnly bool + err string + errWindows string + }{ + { + name: "check with syntax only succeeds with nonexistent rule files", + file: "config_with_rule_files.yml", + syntaxOnly: true, + err: "", + errWindows: "", + }, + { + name: "check without syntax only fails with nonexistent rule files", + file: "config_with_rule_files.yml", + syntaxOnly: false, + err: "\"testdata/non-existent-file.yml\" does not point to an existing file", + errWindows: "\"testdata\\\\non-existent-file.yml\" does not point to an existing file", + }, + { + name: "check with syntax only succeeds with nonexistent service discovery files", + file: "config_with_service_discovery_files.yml", + syntaxOnly: true, + err: "", + errWindows: "", + }, + // The test below doesn't fail because the file verification for ServiceDiscoveryConfigs doesn't fail the check if + // file isn't found; it only outputs a warning message. + { + name: "check without syntax only succeeds with nonexistent service discovery files", + file: "config_with_service_discovery_files.yml", + syntaxOnly: false, + err: "", + errWindows: "", + }, + { + name: "check with syntax only succeeds with nonexistent TLS files", + file: "config_with_tls_files.yml", + syntaxOnly: true, + err: "", + errWindows: "", + }, + { + name: "check without syntax only fails with nonexistent TLS files", + file: "config_with_tls_files.yml", + syntaxOnly: false, + err: "error checking client cert file \"testdata/nonexistent_cert_file.yml\": " + + "stat testdata/nonexistent_cert_file.yml: no such file or directory", + errWindows: "error checking client cert file \"testdata\\\\nonexistent_cert_file.yml\": " + + "CreateFile testdata\\nonexistent_cert_file.yml: The system cannot find the file specified.", + }, + { + name: "check with syntax only succeeds with nonexistent credentials file", + file: "authorization_credentials_file.bad.yml", + syntaxOnly: true, + err: "", + errWindows: "", + }, + { + name: "check without syntax only fails with nonexistent credentials file", + file: "authorization_credentials_file.bad.yml", + syntaxOnly: false, + err: "error checking authorization credentials or bearer token file \"/random/file/which/does/not/exist.yml\": " + + "stat /random/file/which/does/not/exist.yml: no such file or directory", + errWindows: "error checking authorization credentials or bearer token file \"testdata\\\\random\\\\file\\\\which\\\\does\\\\not\\\\exist.yml\": " + + "CreateFile testdata\\random\\file\\which\\does\\not\\exist.yml: The system cannot find the path specified.", + }, + } + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + _, err := checkConfig(false, "testdata/"+test.file, test.syntaxOnly) + expectedErrMsg := test.err + if strings.Contains(runtime.GOOS, "windows") { + expectedErrMsg = test.errWindows + } + if expectedErrMsg != "" { + require.Equalf(t, expectedErrMsg, err.Error(), "Expected error %q, got %q", test.err, err.Error()) + return + } + require.NoError(t, err) + }) + } +} + func TestAuthorizationConfig(t *testing.T) { cases := []struct { name string @@ -224,7 +313,7 @@ func TestAuthorizationConfig(t *testing.T) { for _, test := range cases { t.Run(test.name, func(t *testing.T) { - _, err := checkConfig(false, "testdata/"+test.file) + _, err := checkConfig(false, "testdata/"+test.file, false) if test.err != "" { require.Contains(t, err.Error(), test.err, "Expected error to contain %q, got %q", test.err, err.Error()) return diff --git a/cmd/promtool/testdata/config_with_rule_files.yml b/cmd/promtool/testdata/config_with_rule_files.yml new file mode 100644 index 000000000..ec4816066 --- /dev/null +++ b/cmd/promtool/testdata/config_with_rule_files.yml @@ -0,0 +1,3 @@ +rule_files: + - non-existent-file.yml + - /etc/non/existent/file.yml diff --git a/cmd/promtool/testdata/config_with_service_discovery_files.yml b/cmd/promtool/testdata/config_with_service_discovery_files.yml new file mode 100644 index 000000000..13b6d7faf --- /dev/null +++ b/cmd/promtool/testdata/config_with_service_discovery_files.yml @@ -0,0 +1,12 @@ +scrape_configs: + - job_name: prometheus + file_sd_configs: + - files: + - nonexistent_file.yml +alerting: + alertmanagers: + - scheme: http + api_version: v1 + file_sd_configs: + - files: + - nonexistent_file.yml diff --git a/cmd/promtool/testdata/config_with_tls_files.yml b/cmd/promtool/testdata/config_with_tls_files.yml new file mode 100644 index 000000000..126a0bc38 --- /dev/null +++ b/cmd/promtool/testdata/config_with_tls_files.yml @@ -0,0 +1,5 @@ +scrape_configs: + - job_name: "some job" + tls_config: + cert_file: nonexistent_cert_file.yml + key_file: nonexistent_key_file.yml