Support multiple environment variable placeholders in config values

[aantn]

Summary

Enhanced environment variable substitution in Robusta configuration to support multiple placeholders in a single field and improved the regex pattern for more robust matching.

Key Changes

• Multiple placeholders support: Configuration values can now contain multiple {{ env.VAR }} placeholders (e.g., "{{ env.USER }}:{{ env.PASSWORD }}") instead of just one
• Static text mixing: Placeholders can be combined with static text (e.g., "Bearer {{ env.TOKEN }}")
• Improved regex pattern: Replaced the loose regex pattern with a stricter _ENV_PLACEHOLDER_RE that properly handles whitespace and prevents over-matching
• Refactored substitution logic: Changed from extracting the first match to using re.sub() with a callback function for proper multi-placeholder replacement

Implementation Details

• The new regex pattern r"{{\s*env\.([^}\s]+)\s*}}" is more precise:
  • Allows flexible whitespace around env.
  • Prevents matching incomplete or malformed placeholders
  • Stops at the first } or whitespace to avoid capturing extra characters
• The substitution now uses a callback function _sub() that processes each match independently, enabling proper handling of multiple placeholders in one value
• Early return optimization: checks for placeholder existence before attempting substitution

Documentation

Added comprehensive documentation covering:

• Multiple placeholder usage examples
• New runner.additional_env_froms feature for bulk secret injection
• Clarification on environment variable scope between runner and HolmesGPT pods
• Configuration guidance for HolmesGPT add-ons (operator, AWS MCP, Azure MCP)

https://claude.ai/code/session_013EFjPwGU1FDY6fs6rcELac

[coderabbitai]

Walkthrough

This PR enhances environment variable placeholder substitution in Robusta configuration by introducing a new regex-based mechanism that replaces multiple {{ env.NAME }} placeholders within a single string, accompanied by comprehensive documentation explaining placeholder usage, the additional_env_froms mechanism, and how secrets are provisioned across Runner and HolmesGPT pods.

Changes

Environment Variable Placeholder Substitution

Layer / File(s)	Summary
Core Implementation src/robusta/core/playbooks/playbook_utils.py	Introduced a compiled regex constant to detect {{ env.NAME }} placeholders. Refactored get_env_replacement() to perform all placeholder substitutions via a nested callback function, enabling multiple replacements within a single string instead of only the first match.
Documentation docs/setup-robusta/configuration-secrets.rst	Added comprehensive guidance on combining static text with environment variable placeholders, using additional_env_froms to expose multiple secret keys as environment variables, and distinctions between Runner and HolmesGPT pod scoping. Includes examples and notes on substitution scope across sinks, globalConfig, customPlaybooks, and playbookRepos.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 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 'Support multiple environment variable placeholders in config values' accurately and concisely describes the main change: enabling multiple {{ env.VAR }} placeholders in configuration values instead of just one.
Description check	✅ Passed	The description comprehensively covers the changeset, detailing multiple placeholder support, static text mixing, improved regex patterns, refactored substitution logic, and documentation updates that align with the file changes.
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.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/fact-check-secrets-docs-bE9H3

---

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

[github-actions]

✅ Docker image ready for 02d28b5 (built in 4m 29s)

• us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:02d28b5

⚠️ Warning: does not support ARM (ARM images are built on release only - not on every PR)

Use this tag to pull the image for testing.

📋 Copy commands

⚠️ Temporary images are deleted after 30 days. Copy to a permanent registry before using them:

gcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:02d28b5
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:02d28b5 me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:02d28b5
docker push me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:02d28b5

Patch Helm values in one line:

helm upgrade --install robusta robusta/robusta \
  --reuse-values \
  --set runner.image=me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:02d28b5

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/robusta/core/playbooks/playbook_utils.py`:
- Around line 19-24: The code currently treats empty environment variables as
missing by checking falsiness in the env lookup; update the existence checks to
use None semantics so empty strings are allowed: in the function that reads
env_var_value (the block using os.environ.get(name, None) and the subsequent if
not env_var_value check), change the condition to check "is None" before
logging/raising; likewise update the truthy checks in replace_env_vars_values
(the checks at the locations referenced in the comment) to use "is not None" so
that empty-string substitutions are preserved rather than treated as absent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e365fa0a-b831-48ab-8769-013525033bae

📥 Commits

Reviewing files that changed from the base of the PR and between cd99816 and eb254f1.

📒 Files selected for processing (2)

• docs/setup-robusta/configuration-secrets.rst
• src/robusta/core/playbooks/playbook_utils.py

[aantn]

@claude review

[aantn]

Review

(Disclosure: I authored the commits on this branch — this is a fresh-eyes pass for self-review.)

What's good

• The regex r"{{\s*env\.([^}\s]+)\s*}}" is a real improvement over the old {{[ ]*env\.(.*)[ ]*}}. The old pattern was greedy and would mis-match across two adjacent placeholders ("{{ env.A }} {{ env.B }}" → capture group A }} {{ env.B). The new pattern stops at the first } or whitespace, so multi-placeholder values work correctly.
• The None-on-no-placeholder contract is preserved deliberately so existing callers (runner_config.py:_replace_env_var_in_playbook_repo, replace_env_vars_values) keep their if-checks working. Worth a docstring to make that explicit — easy to break in a future refactor.
• Doc additions cover the three real gaps I'd seen users hit: bulk injection via additional_env_froms, runner-vs-Holmes pod scoping, and Holmes add-on deployments.

Things worth considering

1. Silent behavior change for an edge case. Anyone today who wrote "prefix {{ env.X }}" was silently getting just the env var's value (prefix dropped). They'll now get prefix<value>. The documented usage was always "{{ env.X }}" standalone, so I think real-world breakage is near zero — but it's worth a one-line entry in CHANGELOG.md so anyone with surprising configs sees it on upgrade.

2. No tests. This function had no test coverage before, which is partly how the bug shipped. A small tests/ file pinning the new behavior (prefix preserved, multiple placeholders, no-placeholder returns None, missing env var raises) would be cheap insurance. Happy to add it in a follow-up commit on this branch if you'd like.

3. Minor perf: re.search then re.sub walks the string twice on every matched value. Could collapse to new, n = _ENV_PLACEHOLDER_RE.subn(_sub, value); return new if n else None. Negligible in practice — only flagging because it's in a config-load hot path.

4. Pre-existing doc inconsistency I noticed: line 15 of configuration-secrets.rst (unchanged here) refers to custom_playbooks (snake_case), but the actual Helm key is customPlaybooks (camelCase, per helm/robusta/values.yaml:193). Out of scope for this PR but worth a follow-up.

CodeRabbit's is None finding

Skipping. The "fail on empty env var" semantics is pre-existing (was if not env_var_value before this PR) and out of scope for a multi-placeholder-substitution change. Empty values from a Secret almost always indicate a misconfigured secretKeyRef, and failing loudly at startup is friendlier than silently injecting "" into something like an auth header. If we want to allow legitimately-empty values, it deserves a deliberate decision with its own PR and tests.

Recommendation

Merge after adding a changelog line. Optional follow-ups: tests for get_env_replacement, the subn micro-optimization, and the custom_playbooks doc fix.

---

Generated by Claude Code