Skip to content

config: check for empty cluster TLS client config#5126

Merged
SoloJacobs merged 1 commit intoprometheus:mainfrom
TheMeier:issues/5110
Mar 28, 2026
Merged

config: check for empty cluster TLS client config#5126
SoloJacobs merged 1 commit intoprometheus:mainfrom
TheMeier:issues/5110

Conversation

@TheMeier
Copy link
Copy Markdown
Contributor

@TheMeier TheMeier commented Mar 27, 2026

Pull Request Checklist

Please check all the applicable boxes.

Which user-facing changes does this PR introduce?

[FIX] CONFIG: Improve validation of cluster mTLS config

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced TLS configuration validation to provide clearer error messages when required configuration entries are missing.

@TheMeier TheMeier requested a review from a team as a code owner March 27, 2026 20:13
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 60c9baa0-db1a-4983-9e7b-cb46b9174dc8

📥 Commits

Reviewing files that changed from the base of the PR and between 19e792f and d7ba1c8.

📒 Files selected for processing (2)
  • cluster/tls_config.go
  • cluster/tls_transport_test.go

📝 Walkthrough

Walkthrough

Added explicit validation in TLS configuration parsing to check if tls_client_config is present, returning an error instead of allowing null pointer dereference. Corresponding test case validates the error handling for missing configuration entries.

Changes

Cohort / File(s) Summary
TLS Configuration Validation
cluster/tls_config.go
Added nil check for tls_client_config after YAML unmarshalling to prevent null pointer dereference, returning explicit error message when the entry is missing.
TLS Transport Tests
cluster/tls_transport_test.go
Added test case for testdata/tls_config_with_null_client.yml to verify proper error handling when tls_client_config is absent from the configuration file.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 A null that lurked in config deep,
Made the daemon crash in sleep.
A check! A guard! A test so keen,
Now validation stands between.
No more segfaults shall arise,
When config's missing—we'll advise! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding validation for empty cluster TLS client config, which directly addresses the issue.
Description check ✅ Passed The description is substantially complete, following the template with applicable checklist items marked, linked issue specified, bugfix confirmed with tests, and release notes provided.
Linked Issues check ✅ Passed The code changes directly address issue #5110 by adding validation that checks for nil tls_client_config and returns an explicit error instead of allowing a nil pointer dereference that caused the segfault.
Out of Scope Changes check ✅ Passed All changes are in-scope: modifications to tls_config.go add the required validation check, and test additions in tls_transport_test.go directly test the new validation behavior required to fix the segfault.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Fixes: prometheus#5110
Signed-off-by: Christoph Maser <christoph.maser+github@gmail.com>
@SoloJacobs SoloJacobs merged commit 799132f into prometheus:main Mar 28, 2026
6 checks passed
SoloJacobs added a commit to SoloJacobs/alertmanager that referenced this pull request Apr 6, 2026
* [CHANGE] `go get github.com/prometheus/alertmanager/ui` will now fail as compiled UI assets are no longer checked into the repository. Downstream builds that rely on these assets being present in the source tree must now build the UI from source. prometheus#5113
* [CHANGE] The '--enable-feature=auto-gomaxprocs' option is deprecated and will be removed in v0.33. This flag currently has no effect and can be safely removed from any startup scripts. prometheus#5090
* [CHANGE] Update internal function signatures across multiple packages. This affects any project that integrates `Alertmanager` code.
* [ENHANCEMENT] Add static asset caching. prometheus#5113
* [ENHANCEMENT] Reduce memory allocations through pre-sizing collections and batch allocation. prometheus#5020
* [ENHANCEMENT] Replace help with documentation in navigation bar. prometheus#4943
* [ENHANCEMENT] docs(ha): Update high availability documentation. prometheus#5136
* [ENHANCEMENT] docs: Add `auth_secret_file` for smtp in document. prometheus#5036
* [ENHANCEMENT] docs: Add description for global `telegram_bot_token`. prometheus#5114
* [ENHANCEMENT] docs: Add note about notifier timeouts. prometheus#5077
* [ENHANCEMENT] docs: Fix `force_implicit_tls` config field name. prometheus#5030
* [ENHANCEMENT] docs: Link community supported integrations. prometheus#4978
* [ENHANCEMENT] docs: Remove duplicate header. prometheus#5034
* [ENHANCEMENT] docs: Update mutual tls reference in high availability documentation. prometheus#5120
* [ENHANCEMENT] tracing: Use noop spans when tracing disabled. prometheus#5118
* [FEATURE] Add silence annotations. prometheus#4965
* [FEATURE] Add silence logging option. prometheus#4163
* [FEATURE] Add support for multiple matcher set silences. prometheus#4957
* [FEATURE] Add the reason for notifying in dedup stage. prometheus#4971
* [FEATURE] mattermost: Flatten attachments into top-level config. prometheus#5009
* [FEATURE] mattermost: Support global webhook url. prometheus#4998
* [FEATURE] slack: Add default color from template. prometheus#5014
* [FEATURE] slack: Allow receiver to edit existing messages. prometheus#5007
* [FEATURE] template: Add dict, map and append functions. prometheus#5093
* [FEATURE] webhook: Add full payload templating support for notifier. prometheus#5011
* [BUGFIX] config: Check for empty cluster tls client config. prometheus#5126
* [BUGFIX] config: Don't crash upon reading empty config for notifier. prometheus#4979
* [BUGFIX] config: Fix ipv6 address handling in hostport.string(). prometheus#5040
* [BUGFIX] mattermost: Omit empty text field in notifications. prometheus#4985
* [BUGFIX] telegram: Send fallback message when notification exceeds character limit. prometheus#5074
* [BUGFIX] ui: Fix escaping for matcher values with quotes. prometheus#4862
* [BUGFIX] ui: Handle special chars in silence regex-matchers. prometheus#4942
* [BUGFIX] ui: Support utf-8 label names in matchers. prometheus#5089

Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
@SoloJacobs SoloJacobs mentioned this pull request Apr 6, 2026
ultrotter pushed a commit that referenced this pull request Apr 7, 2026
* [CHANGE] `go get github.com/prometheus/alertmanager/ui` will now fail as compiled UI assets are no longer checked into the repository. Downstream builds that rely on these assets being present in the source tree must now build the UI from source. #5113
* [CHANGE] The '--enable-feature=auto-gomaxprocs' option is deprecated and will be removed in v0.33. This flag currently has no effect and can be safely removed from any startup scripts. #5090
* [CHANGE] Update internal function signatures across multiple packages. This affects any project that integrates `Alertmanager` code.
* [ENHANCEMENT] Add static asset caching. #5113
* [ENHANCEMENT] Reduce memory allocations through pre-sizing collections and batch allocation. #5020
* [ENHANCEMENT] Replace help with documentation in navigation bar. #4943
* [ENHANCEMENT] docs(ha): Update high availability documentation. #5136
* [ENHANCEMENT] docs: Add `auth_secret_file` for smtp in document. #5036
* [ENHANCEMENT] docs: Add description for global `telegram_bot_token`. #5114
* [ENHANCEMENT] docs: Add note about notifier timeouts. #5077
* [ENHANCEMENT] docs: Fix `force_implicit_tls` config field name. #5030
* [ENHANCEMENT] docs: Link community supported integrations. #4978
* [ENHANCEMENT] docs: Remove duplicate header. #5034
* [ENHANCEMENT] docs: Update mutual tls reference in high availability documentation. #5120
* [ENHANCEMENT] tracing: Use noop spans when tracing disabled. #5118
* [FEATURE] Add silence annotations. #4965
* [FEATURE] Add silence logging option. #4163
* [FEATURE] Add support for multiple matcher set silences. #4957
* [FEATURE] Add the reason for notifying in dedup stage. #4971
* [FEATURE] mattermost: Flatten attachments into top-level config. #5009
* [FEATURE] mattermost: Support global webhook url. #4998
* [FEATURE] slack: Add default color from template. #5014
* [FEATURE] slack: Allow receiver to edit existing messages. #5007
* [FEATURE] template: Add dict, map and append functions. #5093
* [FEATURE] webhook: Add full payload templating support for notifier. #5011
* [BUGFIX] config: Check for empty cluster tls client config. #5126
* [BUGFIX] config: Don't crash upon reading empty config for notifier. #4979
* [BUGFIX] config: Fix ipv6 address handling in hostport.string(). #5040
* [BUGFIX] mattermost: Omit empty text field in notifications. #4985
* [BUGFIX] telegram: Send fallback message when notification exceeds character limit. #5074
* [BUGFIX] ui: Fix escaping for matcher values with quotes. #4862
* [BUGFIX] ui: Handle special chars in silence regex-matchers. #4942
* [BUGFIX] ui: Support utf-8 label names in matchers. #5089

Signed-off-by: Solomon Jacobs <solomonjacobs@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

segfault when loading file for cluster.tls-config

2 participants