Uh oh!
There was an error while loading. Please reload this page.
refactor: decompose multi-credential config in test/record.sh - #1004
Merged
Conversation
Extract _get_multi_cred_spec, _load_multi_config_from_file, and _save_multi_config_to_file helpers to eliminate duplicated per-cloud config blocks in try_load_config, save_config, has_credentials, prompt_credentials, and list_clouds. The cloud-to-credential mapping (OVH, UpCloud, Kamatera, AtlanticNet, CloudSigma) is now defined once in _get_multi_cred_spec and consumed by all five functions, making it trivial to add new multi-credential clouds. Agent: complexity-hunter Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
louisgv
approved these changes
Feb 13, 2026
louisgv
left a comment
Collaborator
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Findings
- [MEDIUM] test/record.sh:_save_multi_config_to_file — Uses
evalto expand env var values, but the variable names come from the hardcoded_get_multi_cred_spec()function, not user input. Safe in this context. - [LOW] test/record.sh:has_credentials — Same
evalpattern for credential checking, also sourced from hardcoded spec. Acceptable.
Tests
- bash -n: PASS (verified syntax)
- bun test: N/A (shell script only)
- curl|bash pattern: OK (not applicable — test infrastructure)
- macOS compat: OK (uses printf, ${VAR:-}, no bash 4+ features)
Notes
Clean refactoring that DRYs up multi-credential cloud config handling (OVH, UpCloud, Kamatera, AtlanticNet, CloudSigma). The new _get_multi_cred_spec(), _load_multi_config_from_file(), and _save_multi_config_to_file() helpers eliminate per-cloud case blocks while maintaining the same behavior.
-- security/pr-reviewer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_get_multi_cred_spec()as single source of truth for multi-credential cloud mappings (OVH, UpCloud, Kamatera, AtlanticNet, CloudSigma)_load_multi_config_from_file()and_save_multi_config_to_file()helpers for data-driven JSON config load/savetry_load_config,save_config,has_credentials,prompt_credentials,list_clouds) to use the shared helpers instead of per-cloud copy-paste blocks_get_multi_cred_specTest plan
bash -n test/record.shpasses (no syntax errors)bun testresults unchanged (7365 pass, pre-existing failures only)bash test/mock.shresults unchanged (270 pass, pre-existing failures only)-- refactor/complexity-hunter