Support keystore references for notification URLs

[cwperks]

Summary

• add notification-specific validation for complete ${keystore:<alias>} URL references
• allow Slack, Chime, Microsoft Teams, and webhook models to store an unresolved URL reference
• reject embedded and malformed references while preserving ordinary HTTP and HTTPS URL validation

Motivation

This is a prerequisite for opensearch-project/notifications#1267. Allowing only whole-field references keeps secret URL material out of the notification system index and prevents an authorized configuration editor from redirecting an embedded secret to another host.

The generic validateUrl helper remains unchanged; the reference syntax is scoped to notification configuration models.

Testing

• ./gradlew test
• ./gradlew test --tests org.opensearch.commons.notifications.NotificationConfigReferenceTests ktlint

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Cross-version wire compatibility Slack, Chime, MicrosoftTeams, and Webhook are Writeable types serialized over the OpenSearch binary stream (their StreamInput constructor reads a String url that is then passed through init). After this change, a newer node can persist/send a url like ${keystore:webhook.url}, but any older peer that receives it will fail in its init block because validateUrl will reject the non-HTTP(S) value. This is not guarded by any Version check. Consider gating acceptance of keystore references behind a version constant or ensuring older peers never receive such values. Uncertain: whether these configs are actually round-tripped over the transport layer to mixed-version nodes in practice — worth verifying before shipping. validateUrlOrKeystoreReference(url) Detection heuristic too narrow validateUrlOrKeystoreReference decides whether to enforce the keystore-reference form only when the value contains the literal prefix ${keystore:. A value such as ${ keystore:foo} or ${KEYSTORE:foo} (or with any typo/whitespace) will bypass the reference check and instead be validated as a URL — where it will still fail, but with the misleading "Malformed URL" message rather than "Invalid OpenSearch keystore reference". More importantly, a value like prefix${keystore:foo} triggers the strict branch and is rejected (good), but ${keystore :foo} slips through. Consider matching more loosely (e.g., contains("${") combined with contains("keystore")) to ensure any near-miss is treated as an invalid reference. fun validateUrlOrKeystoreReference(value: String) { if (value.contains(KEYSTORE_REFERENCE_PREFIX)) { require(isKeystoreReference(value)) { "Invalid OpenSearch keystore reference" } } else { validateUrl(value) } }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Disambiguate embedded vs malformed reference errors The contains check on ${keystore: will match embedded references (e.g. https://example.com/${keystore:foo}) and route them to the strict isKeystoreReference check, throwing "Invalid OpenSearch keystore reference" instead of allowing validateUrl to run. While the intent is to reject embedded references, this obscures the error, and more importantly the branch condition is asymmetric with the match: prefer checking startsWith to route only clear reference-shaped values, and reject anything that partially contains the marker with a clearer message. src/main/kotlin/org/opensearch/commons/notifications/NotificationConfigReference.kt [21-27] fun validateUrlOrKeystoreReference(value: String) { - if (value.contains(KEYSTORE_REFERENCE_PREFIX)) { - require(isKeystoreReference(value)) { "Invalid OpenSearch keystore reference" } - } else { - validateUrl(value) + when { + value.startsWith(KEYSTORE_REFERENCE_PREFIX) -> require(isKeystoreReference(value)) { "Invalid OpenSearch keystore reference" } + value.contains(KEYSTORE_REFERENCE_PREFIX) -> throw IllegalArgumentException("Keystore references must not be embedded in URLs") + else -> validateUrl(value) } } Suggestion importance[1-10]: 4 __ Why: The suggestion improves error message clarity by distinguishing embedded references from malformed ones, but the existing behavior already correctly rejects embedded references (as confirmed by the test cases). The improvement is minor and stylistic.	Low
	Tighten keystore alias validation pattern The regex requires at least one character in the alias but permits names starting or ending with ., _, or -, which are not valid OpenSearch keystore setting keys (they must start with a lowercase letter and use dot-separated segments). Consider tightening the pattern to reflect actual keystore key rules, preventing acceptance of aliases like ${keystore:.foo} or ${keystore:-bar}. src/main/kotlin/org/opensearch/commons/notifications/NotificationConfigReference.kt [11] -private val KEYSTORE_REFERENCE_PATTERN = Regex("\\$\\{keystore:([A-Za-z0-9_.-]+)}") +private val KEYSTORE_REFERENCE_PATTERN = Regex("\\$\\{keystore:([a-z][a-z0-9_]*(?:\\.[a-z0-9_]+)*)}") Suggestion importance[1-10]: 3 __ Why: Tightening the regex could be reasonable, but the suggested pattern may be overly restrictive and the actual OpenSearch keystore key rules aren't clearly documented as claimed. The current pattern is permissive but functional.	Low

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.57%. Comparing base (3a69aab) to head (f065f7a).
⚠️ Report is 120 commits behind head on main.

❌ Your project check has failed because the head coverage (70.57%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1007      +/-   ##
============================================
- Coverage     73.97%   70.57%   -3.41%     
- Complexity      916     1149     +233     
============================================
  Files           135      176      +41     
  Lines          6102     7976    +1874     
  Branches        753      958     +205     
============================================
+ Hits           4514     5629    +1115     
- Misses         1253     1948     +695     
- Partials        335      399      +64     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.