Skip to content

config: keep local SMTP auth secret overrides - #5448

Merged
SoloJacobs merged 1 commit into
prometheus:mainfrom
sueun-dev:fix-smtp-auth-secret-inheritance
Aug 16, 2026
Merged

config: keep local SMTP auth secret overrides#5448
SoloJacobs merged 1 commit into
prometheus:mainfrom
sueun-dev:fix-smtp-auth-secret-inheritance

Conversation

@sueun-dev

Copy link
Copy Markdown
Contributor

A receiver-level auth_secret should override the global SMTP auth secret defaults. The loader currently merges auth_secret and auth_secret_file independently, though, so a receiver with a local inline secret also inherits global.smtp_auth_secret_file.

That leaves both fields set. getAuthSecret checks the file first, so the receiver's inline secret is not used. If the global path is unavailable, the notification fails while trying to read it; if it is available, CRAM-MD5 uses the global file contents instead of the receiver override.

This restores paired inheritance: copy the global secret fields only when neither receiver field is set, matching the existing password/password-file handling. The global/local SMTP fixture now covers a global secret file with both local inline-secret and local secret-file overrides.

Tested with:

  • go test ./config/ -run '^TestGlobalAndLocalSMTPPassword$' -count=1
  • go test ./config/... ./notify/email/... -count=1
  • go test -race ./config/... ./notify/email/... -count=1
  • go vet ./config/... ./notify/email/...
  • go build ./config/... ./notify/email/... ./cmd/amtool

Pull Request Checklist

  • Please list all open issue(s) discussed with maintainers related to this change
    • None
  • Is this a new Receiver integration?
    • I have already tried to use the Webhook Receiver Integration and third-party integrations before adding this new Receiver Integration
  • Is this a bugfix?
    • I have added tests that can reproduce the bug which pass with this bugfix applied
  • Is this a new feature?
    • I have added tests that test the new feature's functionality
  • Does this change affect performance?
    • I have provided benchmarks comparison that shows performance is improved or is not degraded
    • I have added new benchmarks if required or requested by maintainers
  • Is this a breaking change?
    • My changes do not break the existing cluster messages
    • My changes do not break the existing api
  • I have added/updated the required documentation
    • Not needed: the current docs already say auth_secret and auth_secret_file are mutually exclusive.
  • I have signed-off my commits
  • I will follow best practices for contributing to this project

Which user-facing changes does this PR introduce?

[BUGFIX] Email: Honor a receiver's inline `auth_secret` when a global `smtp_auth_secret_file` is configured.

@sueun-dev
sueun-dev requested a review from a team as a code owner August 13, 2026 10:38
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: be9a2673-08e0-4617-a480-bd1ffee9e705

📥 Commits

Reviewing files that changed from the base of the PR and between 55d0d79 and 1968855.

📒 Files selected for processing (3)
  • config/config.go
  • config/config_test.go
  • config/testdata/conf.smtp-password-global-and-local.yml

📝 Walkthrough

Walkthrough

SMTP receiver authentication secrets now inherit global values only when both receiver-level fields are unset. Tests and test data cover inheritance and mutual clearing between inline secrets and secret files.

Changes

SMTP secret inheritance

Layer / File(s) Summary
Paired SMTP secret resolution
config/config.go, config/config_test.go, config/testdata/conf.smtp-password-global-and-local.yml
SMTP AuthSecret and AuthSecretFile inherit global values together. Local inline or file settings clear the corresponding inherited value. Tests and fixture data cover these cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to 19688

This change makes receiver-level SMTP authentication secrets take precedence over global defaults, with no actionable merge-blocking risk remaining after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description explains the bug, implementation, tests, checklist status, documentation rationale, and release notes.
Title check ✅ Passed The title clearly identifies the configuration area and the primary change to preserve local SMTP authentication secret overrides.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@SoloJacobs SoloJacobs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheMeier Hmm, looks like we broke this one: #4812
Perhaps there are more similar regressions?

Global SMTP auth secret defaults are a mutually exclusive inline/file pair. Merging each field independently can add the global secret file to a receiver that already has a local inline secret. Email resolution checks the file first, so the receiver override is ignored.

Only inherit the pair when neither receiver field is set, matching the password pair. Extend the global/local SMTP fixture to cover both directions.

Signed-off-by: Sueun Cho <sueun.dev@gmail.com>
@SoloJacobs
SoloJacobs force-pushed the fix-smtp-auth-secret-inheritance branch from 1968855 to c4c6ca4 Compare August 16, 2026 19:04
@SoloJacobs
SoloJacobs enabled auto-merge (squash) August 16, 2026 19:05
@SoloJacobs
SoloJacobs merged commit ee6b5f4 into prometheus:main Aug 16, 2026
7 checks passed
@TheMeier

TheMeier commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

@TheMeier Hmm, looks like we broke this one: #4812 Perhaps there are more similar regressions?

Yes very likely. :( I really think we should make a formal abstraction for merging configs. Something like this 7454fd5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants