Fix rule check broken (#12715)

Signed-off-by: DrAuYueng <ouyang1204@gmail.com>

cmd/promtool/main.go

@@ -733,30 +733,7 @@ func CheckRules(ls lintConfig, files ...string) int {
 	failed := false
 	hasErrors := false
 	if len(files) == 0 {
-		fmt.Println("Checking standard input")
-		data, err := io.ReadAll(os.Stdin)
-		if err != nil {
-			fmt.Fprintln(os.Stderr, "  FAILED:", err)
-			return failureExitCode
-		}
-		rgs, errs := rulefmt.Parse(data)
-		for _, e := range errs {
-			fmt.Fprintln(os.Stderr, e.Error())
-			return failureExitCode
-		}
-		if n, errs := checkRuleGroups(rgs, ls); errs != nil {
-			fmt.Fprintln(os.Stderr, "  FAILED:")
-			for _, e := range errs {
-				fmt.Fprintln(os.Stderr, e.Error())
-			}
-			failed = true
-			for _, err := range errs {
-				hasErrors = hasErrors || !errors.Is(err, lintError)
-			}
-		} else {
-			fmt.Printf("  SUCCESS: %d rules found\n", n)
-		}
-		fmt.Println()
+		failed, hasErrors = checkRulesFromStdin(ls)
 	} else {
 		failed, hasErrors = checkRules(files, ls)
 	}
@@ -771,6 +748,44 @@ func CheckRules(ls lintConfig, files ...string) int {
 	return successExitCode
 }
 
+// checkRulesFromStdin validates rule from stdin.
+func checkRulesFromStdin(ls lintConfig) (bool, bool) {
+	failed := false
+	hasErrors := false
+	fmt.Println("Checking standard input")
+	data, err := io.ReadAll(os.Stdin)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "  FAILED:", err)
+		return true, true
+	}
+	rgs, errs := rulefmt.Parse(data)
+	if errs != nil {
+		failed = true
+		fmt.Fprintln(os.Stderr, "  FAILED:")
+		for _, e := range errs {
+			fmt.Fprintln(os.Stderr, e.Error())
+			hasErrors = hasErrors || !errors.Is(e, lintError)
+		}
+		if hasErrors {
+			return failed, hasErrors
+		}
+	}
+	if n, errs := checkRuleGroups(rgs, ls); errs != nil {
+		fmt.Fprintln(os.Stderr, "  FAILED:")
+		for _, e := range errs {
+			fmt.Fprintln(os.Stderr, e.Error())
+		}
+		failed = true
+		for _, err := range errs {
+			hasErrors = hasErrors || !errors.Is(err, lintError)
+		}
+	} else {
+		fmt.Printf("  SUCCESS: %d rules found\n", n)
+	}
+	fmt.Println()
+	return failed, hasErrors
+}
+
 // checkRules validates rule files.
 func checkRules(files []string, ls lintConfig) (bool, bool) {
 	failed := false
@@ -780,8 +795,15 @@ func checkRules(files []string, ls lintConfig) (bool, bool) {
 		rgs, errs := rulefmt.ParseFile(f)
 		if errs != nil {
 			failed = true
+			fmt.Fprintln(os.Stderr, "  FAILED:")
+			for _, e := range errs {
+				fmt.Fprintln(os.Stderr, e.Error())
+				hasErrors = hasErrors || !errors.Is(e, lintError)
+			}
+			if hasErrors {
 				continue
 			}
+		}
 		if n, errs := checkRuleGroups(rgs, ls); errs != nil {
 			fmt.Fprintln(os.Stderr, "  FAILED:")
 			for _, e := range errs {

cmd/promtool/main_test.go

@@ -464,3 +464,88 @@ func TestDocumentation(t *testing.T) {
 
 	require.Equal(t, string(expectedContent), generatedContent, "Generated content does not match documentation. Hint: run `make cli-documentation`.")
 }
+
+func TestCheckRules(t *testing.T) {
+	t.Run("rules-good", func(t *testing.T) {
+		data, err := os.ReadFile("./testdata/rules.yml")
+		require.NoError(t, err)
+		r, w, err := os.Pipe()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		_, err = w.Write(data)
+		if err != nil {
+			t.Error(err)
+		}
+		w.Close()
+
+		// Restore stdin right after the test.
+		defer func(v *os.File) { os.Stdin = v }(os.Stdin)
+		os.Stdin = r
+
+		exitCode := CheckRules(newLintConfig(lintOptionDuplicateRules, false))
+		require.Equal(t, successExitCode, exitCode, "")
+	})
+
+	t.Run("rules-bad", func(t *testing.T) {
+		data, err := os.ReadFile("./testdata/rules-bad.yml")
+		require.NoError(t, err)
+		r, w, err := os.Pipe()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		_, err = w.Write(data)
+		if err != nil {
+			t.Error(err)
+		}
+		w.Close()
+
+		// Restore stdin right after the test.
+		defer func(v *os.File) { os.Stdin = v }(os.Stdin)
+		os.Stdin = r
+
+		exitCode := CheckRules(newLintConfig(lintOptionDuplicateRules, false))
+		require.Equal(t, failureExitCode, exitCode, "")
+	})
+
+	t.Run("rules-lint-fatal", func(t *testing.T) {
+		data, err := os.ReadFile("./testdata/prometheus-rules.lint.yml")
+		require.NoError(t, err)
+		r, w, err := os.Pipe()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		_, err = w.Write(data)
+		if err != nil {
+			t.Error(err)
+		}
+		w.Close()
+
+		// Restore stdin right after the test.
+		defer func(v *os.File) { os.Stdin = v }(os.Stdin)
+		os.Stdin = r
+
+		exitCode := CheckRules(newLintConfig(lintOptionDuplicateRules, true))
+		require.Equal(t, lintErrExitCode, exitCode, "")
+	})
+}
+
+func TestCheckRulesWithRuleFiles(t *testing.T) {
+	t.Run("rules-good", func(t *testing.T) {
+		exitCode := CheckRules(newLintConfig(lintOptionDuplicateRules, false), "./testdata/rules.yml")
+		require.Equal(t, successExitCode, exitCode, "")
+	})
+
+	t.Run("rules-bad", func(t *testing.T) {
+		exitCode := CheckRules(newLintConfig(lintOptionDuplicateRules, false), "./testdata/rules-bad.yml")
+		require.Equal(t, failureExitCode, exitCode, "")
+	})
+
+	t.Run("rules-lint-fatal", func(t *testing.T) {
+		exitCode := CheckRules(newLintConfig(lintOptionDuplicateRules, true), "./testdata/prometheus-rules.lint.yml")
+		require.Equal(t, lintErrExitCode, exitCode, "")
+	})
+}

cmd/promtool/testdata/rules-bad.yml

@@ -0,0 +1,28 @@
+# This is the rules file.
+
+groups:
+  - name: alerts
+    rules:
+      - alert: InstanceDown
+        expr: up == 0
+        for: 5m
+        labels:
+          severity: page
+        annotations:
+          summary: "Instance {{ $label.foo }} down"
+          description: "{{ $labels.instance }} of job {{ $labels.job }} has been down for more than 5 minutes."
+      - alert: AlwaysFiring
+        expr: 1
+
+  - name: rules
+    rules:
+      - record: job:test:count_over_time1m
+        expr: sum without(instance) (count_over_time(test[1m]))
+
+      # A recording rule that doesn't depend on input series.
+      - record: fixed_data
+        expr: 1
+
+      # Subquery with default resolution test.
+      - record: suquery_interval_test
+        expr: count_over_time(up[5m:])