feat: add promtool validation for golden files by Arpit529Srivastava · Pull Request #8216 · prometheus-operator/prometheus-operator

Arpit529Srivastava · 2025-12-26T22:05:39Z

Description

This PR adds promtool validation for golden files to catch invalid Prometheus configurations early. currently unit tests only compare generated configs against expected golden files (string comparison) but don't validate if the config is actually valid according to Prometheus schema.
This would have caught bugs like #7196 where fallback_scrape_protocol was incorrectly placed in the global config section.

Closes: #7201

If you're contributing for the first-time, check our contribution guidelines.

Type of change

What type of changes does your code introduce to the Prometheus operator? Put an x in the box that apply.

CHANGE (fix or feature that would cause existing functionality to not work as expected)
FEATURE (non-breaking change which adds functionality)
BUGFIX (non-breaking change which fixes an issue)
ENHANCEMENT (non-breaking change which improves existing functionality)
NONE (if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)

Verification

Please check the Prometheus-Operator testing guidelines for recommendations about automated tests.

# Run locally (requires promtool installed)
go test -v -tags=promtool ./pkg/prometheus/ -run TestPromtoolGoldenFiles

Changelog entry

[FEATURE] Add promtool validation for golden files to catch invalid Prometheus configurations early. (#7201)

Arpit529Srivastava · 2025-12-26T22:25:08Z

cc: @slashpai
the ci failure for promtool is expected as it is catching the issue. some golden files are test fixtures with intentionally invalid data.
should i skip known test fixtures or should these golden files be fixed?

slashpai · 2026-01-05T08:59:30Z

cc: @slashpai the ci failure for promtool is expected as it is catching the issue. some golden files are test fixtures with intentionally invalid data. should i skip known test fixtures or should these golden files be fixed?

Hi @Arpit529Srivastava 👋🏻

Thank you for the PR, I will take a look at this today

Since some of the work is done in #7381 can you add @heliapb as Co-Authored By to the PR commit?

Arpit529Srivastava · 2026-01-05T09:36:49Z

Hi @Arpit529Srivastava 👋🏻

Thank you for the PR, I will take a look at this today

Since some of the work is done in #7381 can you add @heliapb as Co-Authored By to the PR commit?

sure.

simonpasquier · 2026-01-05T09:47:19Z

Can we add this to the Makefile so the check is also performed locally? E.g. running make tests-unit or make test-long should have a dependency which verifies the validity of the golden files.

Arpit529Srivastava · 2026-01-05T10:54:43Z

@simonpasquier , i'll add a test-promtool target. should it be a dependency of test-long or test-unit?

slashpai · 2026-01-05T10:29:59Z

.github/workflows/promtool.yaml

+  push:
+    branches:
+      - 'release-*'
+      - 'master'


nit: let's remove master branch as we don't use it

slashpai · 2026-01-05T10:39:32Z

pkg/prometheus/promtool_test.go

+	}
+
+	for _, file := range goldenFiles {
+		t.Run(filepath.Base(file), func(t *testing.T) {


Since each golden file validation is independent, lets add t.Parallel() after the Run()

slashpai · 2026-01-05T10:43:40Z

pkg/prometheus/promtool_test.go

+				return
+			}
+
+			tmpFile, err := os.CreateTemp("", "promtool-*.yaml")


Instead of manually doing this we can use testing package's TempDir which will do autoclean up as well

tmpDir := t.TempDir() tmpFile := filepath.Join(tmpDir, "config.yaml") if err := os.WriteFile(tmpFile, content, 0644); err != nil { t.Fatalf("failed to write temp file: %v", err) } cmd := exec.Command("promtool", "check", "config", tmpFile) output, err := cmd.CombinedOutput() if err != nil { t.Errorf("promtool validation failed:\n%s", string(output)) }

slashpai · 2026-01-05T10:48:50Z

pkg/prometheus/promtool_test.go

+			}
+
+			// Skip partial configs that are not complete Prometheus configs.
+			if !bytes.HasPrefix(bytes.TrimSpace(content), []byte("global:")) {


t.Skipf already causes test to skip

Suggested change

if !bytes.HasPrefix(bytes.TrimSpace(content), []byte("global:")) {

if !bytes.HasPrefix(bytes.TrimSpace(content), []byte("global:")) {

t.Skipf("skipping partial config: %s", file)

}

slashpai · 2026-01-05T10:54:47Z

.github/workflows/promtool.yaml

+        check-latest: true
+    - name: Install promtool
+      run: |
+        PROMETHEUS_VERSION="3.0.0"


We may need a step to extract latest from compatibility matrix

prometheus-operator/pkg/operator/defaults.go

Line 42 in 8b72a71

DefaultPrometheusVersion = PrometheusCompatibilityMatrix[len(PrometheusCompatibilityMatrix)-1]

Arpit529Srivastava · 2026-01-05T11:33:04Z

@slashpai made the requested changes. PTAL
let me know if i miss anything.
Thank you.

slashpai · 2026-01-05T11:37:34Z

.github/workflows/promtool.yaml

+        check-latest: true
+    - name: Install promtool
+      run: |
+        # Extract latest Prometheus version from compatibility matrix


The steps in this could go as a Makefile target something say test-goldenfiles so we can utilize it locally also and in actions can call the make command directly

done with the changes.

Arpit529Srivastava · 2026-01-05T11:52:06Z

@slashpai PTAL

simonpasquier · 2026-01-05T12:11:06Z

Makefile


+.PHONY: test-goldenfiles
+test-goldenfiles:
+	@command -v promtool >/dev/null 2>&1 || { \


I would follow the same pattern we have for other "tools" and add github.com/prometheus/prometheus/cmd/promtool to scripts/tools.go.

simonpasquier · 2026-01-05T12:12:18Z

Makefile

+		sudo mv "prometheus-$${VERSION_NUM}.linux-amd64/promtool" /usr/local/bin/; \
+		rm -rf prometheus-*; \
+	}
+	go test -v -tags=promtool ./pkg/prometheus/ -run TestPromtoolGoldenFiles


can we simply run promtool check config pkg/prometheus/testdata/*?

i’ve added the test-goldenfiles makefile target and updated tools.go as requested. about removing promtool_test.go: running promtool on all files fails for partial configs. i can drop the test file and instead use grep in the makefile to only run it on valid configs (starting with global:). does that sound better than keeping the test?

What if we add a marker comment to the golden file to signal that it's not a Prometheus config file?

#skip-promtool-verification

Another option is to move the few problematic files to a sub-directory under testdata/.

We also have a few tests calling generateK8SSDConfig() which could be updated to call GenerateServerConfiguration() instead.

Arpit529Srivastava · 2026-01-05T15:59:35Z

@slashpai @simonpasquier
using skip markers breaks existing golden tests and fixing the tests would require significant refactoring to make partial configs valid. moving the partial configs is the cleanest approach. i’ll move the 12 partial config files to pkg/prometheus/testdata/partial/ and update their references in promcfg_test.go.
thank u

Arpit529Srivastava · 2026-01-06T16:33:08Z

@slashpai @simonpasquier

added promtool to scripts/tools.go
replaced the test with promtool check config in the Makefile
moved partial configs to testdata/partial/

let me know if anything else needs changes.
thank you.

slashpai · 2026-01-09T07:13:37Z

Makefile

 	./scripts/update-golden-files.sh

+.PHONY: test-goldenfiles
+test-goldenfiles: $(TOOLS_BIN_DIR)/promtool


Lets add PROMTOOL_BINARY=$(TOOLS_BIN_DIR)/promtool after

prometheus-operator/Makefile

Line 49 in 3bff673

PROMLINTER_BINARY=$(TOOLS_BIN_DIR)/promlinter

to follow approach as other tools and include it in

prometheus-operator/Makefile

Line 54 in 3bff673

TOOLING=$(CONTROLLER_GEN_BINARY) $(GOBINDATA_BINARY) $(JB_BINARY) $(GOJSONTOYAML_BINARY) $(JSONNET_BINARY) $(JSONNETFMT_BINARY) $(SHELLCHECK_BINARY) $(PROMLINTER_BINARY) $(GOLANGCILINTER_BINARY) $(MDOX_BINARY) $(API_DOC_GEN_BINARY) $(GOLANGCIKUBEAPILINTER_BINARY)

slashpai · 2026-01-09T07:14:42Z

Makefile

 	./scripts/update-golden-files.sh

+.PHONY: test-goldenfiles
+test-goldenfiles: $(TOOLS_BIN_DIR)/promtool


Suggested change

test-goldenfiles: $(TOOLS_BIN_DIR)/promtool

test-goldenfiles: $(PROMTOOL_BINARY)

slashpai · 2026-01-09T07:15:55Z

Makefile

+.PHONY: test-goldenfiles
+test-goldenfiles: $(TOOLS_BIN_DIR)/promtool
+	@echo "Validating golden files with promtool..."
+	-@$(TOOLS_BIN_DIR)/promtool check config pkg/prometheus/testdata/*.golden 2>&1 | tee /dev/stderr | grep -c "SUCCESS" | xargs -I {} echo "{} golden files passed promtool validation"


Suggested change

-@$(TOOLS_BIN_DIR)/promtool check config pkg/prometheus/testdata/*.golden 2>&1 | tee /dev/stderr | grep -c "SUCCESS" | xargs -I {} echo "{} golden files passed promtool validation"

-$(PROMTOOL_BINARY) check config pkg/prometheus/testdata/*.golden 2>&1 | tee /dev/stderr | grep -c "SUCCESS" | xargs -I {} echo "{} golden files passed promtool validation"

slashpai · 2026-01-09T07:19:04Z

.github/workflows/promtool.yaml

+jobs:
+  changed-files:
+    uses: ./.github/workflows/changed-files.yaml
+  promtool-check:


we can add this as new job check-goldenfiles in existing workflow checks.yaml

prometheus-operator/.github/workflows/checks.yaml

Line 103 in 3bff673

slashpai · 2026-01-09T07:22:38Z

Can you also add about the new make file target make test-goldenfiles in TESTING.md explaining its use and partial configs as well.

slashpai · 2026-01-09T07:24:36Z

Makefile

 test-unit-update-golden:
 	./scripts/update-golden-files.sh

+.PHONY: test-goldenfiles


Since only testing prometheus golden files here may be name it as test-prometheus-goldenfiles and later when other packages are added create test-goldenfiles as aggregate target

addressed all the requested changes

Arpit529Srivastava · 2026-01-09T07:59:07Z

@slashpai PTAL.
Thank you for addressing.

slashpai · 2026-01-09T10:41:15Z

Makefile

+	-$(PROMTOOL_BINARY) check config pkg/prometheus/testdata/*.golden 2>&1 | tee /dev/stderr | grep -c "SUCCESS" | xargs -I {} echo "{} golden files passed promtool validation"
+	@echo "Note: Some failures are expected for version-specific or intentionally invalid test fixtures"
+
+$(PROMTOOL_BINARY): scripts/go.mod scripts/go.sum scripts/tools.go


nit: do we need this?

I am not sure other tools works without it right?

you're right - it's not needed.
removed it.

slashpai

lgtm

I would have @simonpasquier also take a look

Arpit529Srivastava · 2026-01-12T17:17:28Z

I'd prefer to enforce strict validation first (as I wrote earlier I already spotted a bug about gce_sd_configs). To avoid the PR growing too much, I'd be ok if you fix any issue found by promtool in separate PRs which we can merge before this one.

For partials, I'd also recommend that we update the existing tests to build full configurations which could be passed to promtool. You can also submit individual PRs to migrate the tests first.

Again thanks for the work, I think that it's very valuable!

thanks for the guidance @simonpasquier @slashpai!

here’s my plan:

bug fixes

pr1: fix the gce_sd_config → gce_sd_configs bug

test migrations (convert partials to full configs)

pr2: alertmanager config tests (~10 files)
pr3: sd config tests (k8s, azure, etc.) (~30 files)
pr4: remotewrite config tests (~10 files)
pr5: servicemonitor / podmonitor tests (~15 files)
pr6: remaining tests (~15 files)

final

this pr: enable strict validation once the above are merged

i’ll start with the gce_sd_configs fix and send it shortly.

Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com>

Arpit529Srivastava · 2026-01-28T19:55:46Z

@slashpai @heliapb @simonpasquier when you get a chance, could you do a final review? From my side it looks like the promtool issue is resolved.
thanks :)

slashpai · 2026-01-29T00:33:26Z

Good work on this
lgtm

Arpit529Srivastava · 2026-01-30T04:47:17Z

@simonpasquier a gentle ping, thanks :)

simonpasquier · 2026-01-29T16:42:35Z

TESTING.md

+make test-prometheus-goldenfiles
+```
+
+**Note:** Files in `pkg/prometheus/testdata/input/` and `pkg/prometheus/testdata/legacy-versions/` are excluded from validation as they contain configuration fragments or version-specific test data.


(nit) maybe have a more generic stance since we have more than these 2 directories (e.g. testdata subdirectories aren't checked because they contain either configuration fragments or legacy configurations).

simonpasquier · 2026-01-30T07:54:34Z

Makefile

+test-prometheus-goldenfiles: $(TOOLS_BIN_DIR)/promtool
+	$(TOOLS_BIN_DIR)/promtool check config --syntax-only pkg/prometheus/testdata/*.golden
+
+$(TOOLS_BIN_DIR)/promtool: $(TOOLS_BIN_DIR)


can we use the same mechanism as we have for other tools (e.g. add the command to scripts/tools.go)?

adding promtool to tools.go will significantly increase the dependency footprint in go.mod (roughly 100+ extra packages). cuz prometheus pulls in a wide range of service discovery clients and integrations for multiple cloud providers like aws, azure, gcp, digital ocean, hetzner and others.

that's why i was initially hesitant to add it to tool.go and went with the binary download approach instead.

should i go ahead and add it to tools.go for consistency with the other tools, or would you prefer to stick with the binary download approach? or if i am missing anything 🤔

the additional dependencies will only show up in tools/go.mod which is acceptable.

okay, done.

Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com>

Arpit529Srivastava · 2026-01-30T21:57:50Z

@slashpai @simonpasquier final review and then ig we are good to go?..
thanks for reviewing :)

simonpasquier

thanks!

) * feat: add promtool validation for golden files Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com> * fix: add goldenfiles and testdata to cspell dictionary Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com> * fix: clarify note on excluded files in golden file validation Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com> * refactor: use tools.go pattern for promtool Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com> --------- Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com>

Arpit529Srivastava requested a review from a team as a code owner December 26, 2025 22:05

pull-request-size bot added the size/L label Dec 26, 2025

Arpit529Srivastava force-pushed the fix/promtool-7201 branch from 87dcc59 to 94d92d1 Compare January 5, 2026 09:38

slashpai reviewed Jan 5, 2026

View reviewed changes

Arpit529Srivastava force-pushed the fix/promtool-7201 branch from 94d92d1 to 54ed4a2 Compare January 5, 2026 11:24

pull-request-size bot added size/XXL and removed size/L labels Jan 5, 2026

Arpit529Srivastava force-pushed the fix/promtool-7201 branch from 54ed4a2 to 7aa477d Compare January 5, 2026 11:27

pull-request-size bot added size/L and removed size/XXL labels Jan 5, 2026

slashpai reviewed Jan 5, 2026

View reviewed changes

Arpit529Srivastava force-pushed the fix/promtool-7201 branch from 7aa477d to 4e1d149 Compare January 5, 2026 11:50

simonpasquier reviewed Jan 5, 2026

View reviewed changes

Arpit529Srivastava requested a review from slashpai January 9, 2026 06:35

slashpai reviewed Jan 9, 2026

View reviewed changes

Arpit529Srivastava requested a review from slashpai January 9, 2026 08:54

slashpai reviewed Jan 9, 2026

View reviewed changes

slashpai previously approved these changes Jan 9, 2026

View reviewed changes

feat: add promtool validation for golden files

505a3fd

Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com>

Arpit529Srivastava force-pushed the fix/promtool-7201 branch from cf2ba79 to 505a3fd Compare January 28, 2026 19:03

pull-request-size bot added size/M and removed size/L labels Jan 28, 2026

fix: add goldenfiles and testdata to cspell dictionary

b8133a3

Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com>

slashpai previously approved these changes Jan 29, 2026

View reviewed changes

simonpasquier reviewed Jan 30, 2026

View reviewed changes

fix: clarify note on excluded files in golden file validation

07d103d

Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com>

Arpit529Srivastava dismissed slashpai’s stale review via 07d103d January 30, 2026 09:11

Arpit529Srivastava requested review from simonpasquier and slashpai January 30, 2026 14:15

refactor: use tools.go pattern for promtool

7acc182

Signed-off-by: arpit529srivastava <arpitsrivastava529@gmail.com>

pull-request-size bot added size/L and removed size/M labels Jan 30, 2026

simonpasquier approved these changes Feb 2, 2026

View reviewed changes

simonpasquier merged commit 47c8b93 into prometheus-operator:main Feb 2, 2026
23 checks passed

Arpit529Srivastava deleted the fix/promtool-7201 branch February 2, 2026 15:17

	test-goldenfiles: $(TOOLS_BIN_DIR)/promtool
	test-goldenfiles: $(PROMTOOL_BINARY)

	-@$(TOOLS_BIN_DIR)/promtool check config pkg/prometheus/testdata/*.golden 2>&1 \| tee /dev/stderr \| grep -c "SUCCESS" \| xargs -I {} echo "{} golden files passed promtool validation"
	-$(PROMTOOL_BINARY) check config pkg/prometheus/testdata/*.golden 2>&1 \| tee /dev/stderr \| grep -c "SUCCESS" \| xargs -I {} echo "{} golden files passed promtool validation"

Conversation

Arpit529Srivastava commented Dec 26, 2025 • edited by slashpai Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Verification

Changelog entry

Uh oh!

Arpit529Srivastava commented Dec 26, 2025

Uh oh!

slashpai commented Jan 5, 2026

Uh oh!

Arpit529Srivastava commented Jan 5, 2026

Uh oh!

simonpasquier commented Jan 5, 2026

Uh oh!

Arpit529Srivastava commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Arpit529Srivastava commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Arpit529Srivastava commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Arpit529Srivastava commented Jan 5, 2026

Uh oh!

Arpit529Srivastava commented Jan 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slashpai commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Arpit529Srivastava commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slashpai left a comment

Choose a reason for hiding this comment

Uh oh!

Arpit529Srivastava commented Jan 12, 2026

bug fixes

test migrations (convert partials to full configs)

final

Uh oh!

Arpit529Srivastava commented Jan 28, 2026

Uh oh!

Arpit529Srivastava commented Dec 26, 2025 •

edited by slashpai

Loading