From 13bd4fd3c81c8a483575a183139d830da8d1563e Mon Sep 17 00:00:00 2001
From: Julien Pivotto <roidelapluie@o11y.eu>
Date: Fri, 1 Jul 2022 14:38:49 +0200
Subject: [PATCH] Fix promtool check config not erroring properly on failures
 (#10952)

Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
---
 cmd/promtool/main.go                          |  1 +
 cmd/promtool/main_test.go                     | 74 +++++++++++++++++++
 .../testdata/prometheus-config.bad.yml        |  1 +
 .../testdata/prometheus-config.good.yml       |  0
 .../testdata/prometheus-config.lint.yml       |  2 +
 .../testdata/prometheus-rules.lint.yml        | 17 +++++
 6 files changed, 95 insertions(+)
 create mode 100644 cmd/promtool/testdata/prometheus-config.bad.yml
 create mode 100644 cmd/promtool/testdata/prometheus-config.good.yml
 create mode 100644 cmd/promtool/testdata/prometheus-config.lint.yml
 create mode 100644 cmd/promtool/testdata/prometheus-rules.lint.yml

diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go
index ad802e482..96d7f3851 100644
--- a/cmd/promtool/main.go
+++ b/cmd/promtool/main.go
@@ -344,6 +344,7 @@ func CheckConfig(agentMode, checkSyntaxOnly bool, lintSettings lintConfig, files
 		ruleFiles, err := checkConfig(agentMode, f, checkSyntaxOnly)
 		if err != nil {
 			fmt.Fprintln(os.Stderr, "  FAILED:", err)
+			hasErrors = true
 			failed = true
 		} else {
 			if len(ruleFiles) > 0 {
diff --git a/cmd/promtool/main_test.go b/cmd/promtool/main_test.go
index d4773fc91..40df3e524 100644
--- a/cmd/promtool/main_test.go
+++ b/cmd/promtool/main_test.go
@@ -14,13 +14,16 @@
 package main
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
 	"os"
+	"os/exec"
 	"runtime"
 	"strings"
+	"syscall"
 	"testing"
 	"time"
 
@@ -30,6 +33,21 @@ import (
 	"github.com/prometheus/prometheus/model/rulefmt"
 )
 
+var promtoolPath = os.Args[0]
+
+func TestMain(m *testing.M) {
+	for i, arg := range os.Args {
+		if arg == "-test.main" {
+			os.Args = append(os.Args[:i], os.Args[i+1:]...)
+			main()
+			return
+		}
+	}
+
+	exitCode := m.Run()
+	os.Exit(exitCode)
+}
+
 func TestQueryRange(t *testing.T) {
 	s, getRequest := mockServer(200, `{"status": "success", "data": {"resultType": "matrix", "result": []}}`)
 	defer s.Close()
@@ -359,3 +377,59 @@ func TestCheckMetricsExtended(t *testing.T) {
 		},
 	}, stats)
 }
+
+func TestExitCodes(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test in short mode.")
+	}
+
+	for _, c := range []struct {
+		file      string
+		exitCode  int
+		lintIssue bool
+	}{
+		{
+			file: "prometheus-config.good.yml",
+		},
+		{
+			file:     "prometheus-config.bad.yml",
+			exitCode: 1,
+		},
+		{
+			file:     "prometheus-config.nonexistent.yml",
+			exitCode: 1,
+		},
+		{
+			file:      "prometheus-config.lint.yml",
+			lintIssue: true,
+			exitCode:  3,
+		},
+	} {
+		t.Run(c.file, func(t *testing.T) {
+			for _, lintFatal := range []bool{true, false} {
+				t.Run(fmt.Sprintf("%t", lintFatal), func(t *testing.T) {
+					args := []string{"-test.main", "check", "config", "testdata/" + c.file}
+					if lintFatal {
+						args = append(args, "--lint-fatal")
+					}
+					tool := exec.Command(promtoolPath, args...)
+					err := tool.Run()
+					if c.exitCode == 0 || (c.lintIssue && !lintFatal) {
+						require.NoError(t, err)
+						return
+					}
+
+					require.Error(t, err)
+
+					var exitError *exec.ExitError
+					if errors.As(err, &exitError) {
+						status := exitError.Sys().(syscall.WaitStatus)
+						require.Equal(t, c.exitCode, status.ExitStatus())
+					} else {
+						t.Errorf("unable to retrieve the exit status for promtool: %v", err)
+					}
+				})
+			}
+		})
+	}
+}
diff --git a/cmd/promtool/testdata/prometheus-config.bad.yml b/cmd/promtool/testdata/prometheus-config.bad.yml
new file mode 100644
index 000000000..13e262e05
--- /dev/null
+++ b/cmd/promtool/testdata/prometheus-config.bad.yml
@@ -0,0 +1 @@
+not-prometheus:
diff --git a/cmd/promtool/testdata/prometheus-config.good.yml b/cmd/promtool/testdata/prometheus-config.good.yml
new file mode 100644
index 000000000..e69de29bb
diff --git a/cmd/promtool/testdata/prometheus-config.lint.yml b/cmd/promtool/testdata/prometheus-config.lint.yml
new file mode 100644
index 000000000..ffa7c3843
--- /dev/null
+++ b/cmd/promtool/testdata/prometheus-config.lint.yml
@@ -0,0 +1,2 @@
+rule_files:
+  - prometheus-rules.lint.yml
diff --git a/cmd/promtool/testdata/prometheus-rules.lint.yml b/cmd/promtool/testdata/prometheus-rules.lint.yml
new file mode 100644
index 000000000..b067675de
--- /dev/null
+++ b/cmd/promtool/testdata/prometheus-rules.lint.yml
@@ -0,0 +1,17 @@
+groups:
+- name: example
+  rules:
+  - alert: HighRequestLatency
+    expr: job:request_latency_seconds:mean5m{job="myjob"} > 0.5
+    for: 10m
+    labels:
+      severity: page
+    annotations:
+      summary: High request latency
+  - alert: HighRequestLatency
+    expr: job:request_latency_seconds:mean5m{job="myjob"} > 0.5
+    for: 10m
+    labels:
+      severity: page
+    annotations:
+      summary: High request latency