Support direct Weaviate key rotation - #90
Conversation
📝 WalkthroughWalkthroughThe Weaviate staging inventory now references the staging Vault secret and instance name. The playbook derives a primary API key, accepts only that key, and disables runtime configuration application. ChangesWeaviate operations
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@playbooks/weaviate/weaviate.yml`:
- Around line 19-30: Update the weaviate_primary_api_key resolution to use the
inline api_key only when it is defined and non-empty for var_instance_name;
otherwise fall back to the existing Vault lookup. Add validation before
rendering weaviate_accepted_api_keys to assert that the resolved primary key is
present, ensuring unresolved or empty keys fail closed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ae00a311-e72c-4aac-8b15-b74e767256e0
📒 Files selected for processing (2)
playbooks/weaviate/OWNERSHIP.mdplaybooks/weaviate/weaviate.yml
| weaviate_primary_api_key: >- | ||
| {{ | ||
| weaviate_inline_secrets[var_instance_name].api_key | ||
| if weaviate_inline_secrets is defined | ||
| else lookup( | ||
| 'hashi_vault', | ||
| 'kv/data/itg/services/services_ase/{0}'.format(var_instance_name) | ||
| ).get('api_key') | ||
| }} | ||
| # Rotation is a direct cutover: the compromised key is never rendered. | ||
| weaviate_accepted_api_keys: | ||
| - "{{ weaviate_primary_api_key }}" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,200p'
echo "== target file =="
if [ -f playbooks/weaviate/weaviate.yml ]; then
wc -l playbooks/weaviate/weaviate.yml
cat -n playbooks/weaviate/weaviate.yml
else
echo "missing playbooks/weaviate/weaviate.yml"
fi
echo "== search relevant symbols =="
rg -n "weaviate_inline_secrets|weaviate_primary_api_key|weaviate_accepted_api_keys|weaviate-test|var_instance_name|hashi_vault" -S .Repository: ls1intum/artemis-ansible
Length of output: 35558
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ownership docs =="
cat -n playbooks/weaviate/OWNERSHIP.md
echo "== group_vars weaviate files =="
for f in group_vars/artemistests_weaviate.yml group_vars/artemis_staging1.yml group_vars/artemis_staging2.yml group_vars/artemis_production.yml hosts; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,180p' "$f" | cat -n
else
echo "missing $f"
fi
done
echo "== ansible lookup/conditional probe =="
python3 - <<'PY'
import re
text = open("playbooks/weaviate/weaviate.yml", encoding="utf-8").read()
expr = re.search(
r'weaviate_primary_api_key:\s*>\-?\n\s*\{\{\s*\n\s*weaviate_inline_secrets\[var_instance_name\]\.api_key\s*\n\s*if\s+weaviate_inline_secrets\s+is\s+defined',
text,
re.S
)
print("conditional_defined_inline_branch_present=", bool(expr))
# Model the Jinja-ish two-branch ternary semantics for key lookup, not running Ansible.
def pick(var_instance_name, secrets=None):
if secrets is not None:
return secrets.get(var_instance_name, {}).get("api_key")
# fallback path would be Vault; model this as KeyError for missing Vault value.
raise KeyError(f"kv/data/itg/services/services_ase/{var_instance_name}")
cases = {
"defined_with_instance_key": ("weaviate-test", {"weaviate-test": {"api_key": "inline"}}),
"defined_without_instance_key": ("weaviate-test", {"other": {"api_key": "x"}}),
"defined_empty": ("weaviate-test", {}),
"staging_uses_test_key": ("weaviate-test", {"weaviate-test": {"api_key": "shared"}}),
}
for name, (key, secrets) in cases.items():
try:
out = pick(key, secrets)
except KeyError as e:
print(f"{name}: fallback_path_triggered")
else:
print(f"{name}: inline_key_used={out!r}")
PY
echo "== role files if present =="
find . -type d -path '*weaviate*' | sed -n '1,80p'Repository: ls1intum/artemis-ansible
Length of output: 19140
Fail closed when the inline replacement key is unavailable.
Current weaviate_staging hosts map var_instance_name to weaviate-test, so the explicit staging sharing note already covers that. The missing case is an empty/partial inline map for the selected instance: weaviate_inline_secrets is defined still selects the inline branch and renders an unresolved key instead of falling back to Vault. Only use the inline value when weaviate_inline_secrets[var_instance_name].api_key is non-empty, and assert the primary key before rendering weaviate_accepted_api_keys.
🤖 Prompt for 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.
In `@playbooks/weaviate/weaviate.yml` around lines 19 - 30, Update the
weaviate_primary_api_key resolution to use the inline api_key only when it is
defined and non-empty for var_instance_name; otherwise fall back to the existing
Vault lookup. Add validation before rendering weaviate_accepted_api_keys to
assert that the resolved primary key is present, ensuring unresolved or empty
keys fail closed.
b7d084f to
fb30ba2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
playbooks/weaviate/weaviate.yml (1)
19-28: 🗄️ Data Integrity & Integration | 🟠 MajorFail closed when the resolved key is missing or empty.
.get('api_key')can yieldNoneor an empty string, but the value is still rendered as the sole accepted key. Add a pre-role assertion that the resolved key is a non-empty string so rotation aborts before producing an invalid authentication configuration.🤖 Prompt for 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. In `@playbooks/weaviate/weaviate.yml` around lines 19 - 28, Update the pre-role setup around weaviate_primary_api_key to assert that the resolved value is a non-empty string before populating weaviate_accepted_api_keys. Make the assertion fail and abort rotation when the Vault lookup returns None or an empty value, while preserving the existing accepted-key configuration for valid keys.
🤖 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 `@playbooks/weaviate/weaviate.yml`:
- Around line 19-25: Update the variable assignment for weaviate_primary_api_key
to explicitly select the protected offline replacement input when offline
recovery is active, otherwise retain the existing Vault lookup. Validate the
selected key before passing it to the Weaviate role, rejecting missing or empty
values while preserving the normal Vault-backed path.
---
Duplicate comments:
In `@playbooks/weaviate/weaviate.yml`:
- Around line 19-28: Update the pre-role setup around weaviate_primary_api_key
to assert that the resolved value is a non-empty string before populating
weaviate_accepted_api_keys. Make the assertion fail and abort rotation when the
Vault lookup returns None or an empty value, while preserving the existing
accepted-key configuration for valid keys.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0c79926-1565-490f-a5be-392e4f739410
📒 Files selected for processing (1)
playbooks/weaviate/weaviate.yml
| weaviate_primary_api_key: >- | ||
| {{ | ||
| lookup( | ||
| 'hashi_vault', | ||
| 'kv/data/itg/services/services_ase/{0}'.format(var_instance_name) | ||
| ).get('api_key') | ||
| }} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Honor the documented offline recovery source.
This expression always reads the key from Vault; it never consumes the protected offline replacement input described in the PR objective. During an offline recovery rotation, the playbook can therefore configure the wrong key and prevent the intended cutover. Add explicit source selection for the offline value versus Vault, then validate the selected key before passing it to the role.
🤖 Prompt for 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.
In `@playbooks/weaviate/weaviate.yml` around lines 19 - 25, Update the variable
assignment for weaviate_primary_api_key to explicitly select the protected
offline replacement input when offline recovery is active, otherwise retain the
existing Vault lookup. Validate the selected key before passing it to the
Weaviate role, rejecting missing or empty values while preserving the normal
Vault-backed path.
1 participant
Motivation and Context
Give the production, test, and staging Weaviate hosts independent replacement
credentials.
Related: ls1intum/artemis-ansible-collection#231
Description
weaviate-stagingVault mapping and use it for Artemisstaging1/staging2.
Steps for Deployment
Weaviate prod, test, and staging are deployed. The Artemis staging1/staging2
consumer configuration remains for the Artemis owner.