|
| 1 | +# ADR-005: Sudo Cache Management for Infrastructure Operations |
| 2 | + |
| 3 | +## Status |
| 4 | + |
| 5 | +Accepted |
| 6 | + |
| 7 | +## Context |
| 8 | + |
| 9 | +During infrastructure testing, specifically when running `make test`, users experienced poor UX due to |
| 10 | +sudo password prompts being mixed with other command output. This created several problems: |
| 11 | + |
| 12 | +1. **Mixed Output**: The sudo password prompt appeared in the middle of verbose OpenTofu output, |
| 13 | + making it difficult to notice |
| 14 | +2. **Test Hangs**: Users would miss the password prompt, causing tests to hang indefinitely |
| 15 | +3. **Unclear Timing**: Users didn't know when sudo access would be needed during the test process |
| 16 | +4. **Interrupted Flow**: Password prompts appeared at unpredictable times during infrastructure |
| 17 | + provisioning |
| 18 | + |
| 19 | +### Technical Root Cause |
| 20 | + |
| 21 | +The issue occurred during OpenTofu's `local-exec` provisioner execution in |
| 22 | +`infrastructure/terraform/main.tf`: |
| 23 | + |
| 24 | +```hcl |
| 25 | +# Fix permissions after creation |
| 26 | +provisioner "local-exec" { |
| 27 | + command = "${path.module}/../scripts/fix-volume-permissions.sh" |
| 28 | +} |
| 29 | +``` |
| 30 | + |
| 31 | +This script runs `sudo` commands for libvirt volume permission management, but the password prompt |
| 32 | +was buried in OpenTofu's verbose output. |
| 33 | + |
| 34 | +## Decision |
| 35 | + |
| 36 | +We chose **Option 1: Pre-authorize sudo with timeout and clear user messaging**. |
| 37 | + |
| 38 | +### Implemented Solution |
| 39 | + |
| 40 | +1. **Sudo Cache Management Functions** in `scripts/shell-utils.sh`: |
| 41 | + |
| 42 | + - `is_sudo_cached()` - Check if sudo credentials are cached |
| 43 | + - `ensure_sudo_cached(description)` - Warn user and cache sudo credentials |
| 44 | + - `run_with_sudo(description, command)` - Run command with pre-cached sudo |
| 45 | + - `clear_sudo_cache()` - Clear sudo cache for testing |
| 46 | + |
| 47 | +2. **Proactive Sudo Preparation**: |
| 48 | + |
| 49 | + - Cache sudo credentials before infrastructure operations begin |
| 50 | + - Clear user messaging about when and why sudo is needed |
| 51 | + - Use harmless `sudo -v` command to cache without executing privileged operations |
| 52 | + |
| 53 | +3. **Integration Points**: |
| 54 | + - `tests/test-e2e.sh`: Prepare sudo cache before infrastructure provisioning |
| 55 | + - `infrastructure/scripts/provision-infrastructure.sh`: Cache sudo before `tofu apply` |
| 56 | + - `infrastructure/scripts/fix-volume-permissions.sh`: Use cached sudo for operations |
| 57 | + |
| 58 | +### User Experience Improvement |
| 59 | + |
| 60 | +**Before:** |
| 61 | + |
| 62 | +```bash |
| 63 | +make test |
| 64 | +# ... lots of OpenTofu output ... |
| 65 | +libvirt_volume.base_image (local-exec): Fixing libvirt volume permissions... |
| 66 | +[sudo] password for user: # <- Hidden in output, easy to miss |
| 67 | +``` |
| 68 | +
|
| 69 | +**After:** |
| 70 | +
|
| 71 | +```bash |
| 72 | +make test |
| 73 | +⚠️ SUDO PREPARATION |
| 74 | +Infrastructure provisioning requires administrator privileges |
| 75 | +[sudo] password for user: # <- Clear, upfront prompt |
| 76 | +✓ Administrator privileges confirmed and cached |
| 77 | +# ... rest runs without interruption ... |
| 78 | +``` |
| 79 | +
|
| 80 | +## Alternatives Considered |
| 81 | +
|
| 82 | +### Option 1: Pre-authorize sudo with timeout ⭐ (CHOSEN) |
| 83 | +
|
| 84 | +- **Pros**: Safe, minimal changes, clear UX, leverages existing sudo timeout |
| 85 | +- **Cons**: Still requires password entry once |
| 86 | +
|
| 87 | +### Option 2: Passwordless sudo configuration |
| 88 | +
|
| 89 | +- **Pros**: No password prompts during tests |
| 90 | +- **Cons**: Security risk, requires system configuration changes, complex setup |
| 91 | +
|
| 92 | +### Option 3: Replace local-exec with null_resource |
| 93 | +
|
| 94 | +- **Pros**: Better output control |
| 95 | +- **Cons**: Still needs sudo password, more complex Terraform |
| 96 | +
|
| 97 | +### Option 4: Move permission fixes to cloud-init |
| 98 | +
|
| 99 | +- **Pros**: No host sudo needed |
| 100 | +- **Cons**: Complex implementation, may not solve all permission issues |
| 101 | +
|
| 102 | +### Option 5: Enhanced messaging only |
| 103 | +
|
| 104 | +- **Pros**: Simple implementation |
| 105 | +- **Cons**: Doesn't solve the core mixing problem |
| 106 | +
|
| 107 | +### Option 6: Use polkit/pkexec |
| 108 | +
|
| 109 | +- **Pros**: GUI prompts, better UX |
| 110 | +- **Cons**: Complex setup, environment dependencies |
| 111 | +
|
| 112 | +### Option 7: Automated passwordless sudo setup |
| 113 | +
|
| 114 | +- **Pros**: One-time setup eliminates problem |
| 115 | +- **Cons**: Security implications, system configuration complexity |
| 116 | +
|
| 117 | +## Rationale |
| 118 | +
|
| 119 | +Option 1 was chosen because it: |
| 120 | +
|
| 121 | +1. **Maintains Security**: Uses standard sudo timeout without permanent passwordless access |
| 122 | +2. **Minimal Risk**: Uses safe `sudo -v` command that doesn't execute privileged operations |
| 123 | +3. **Clear UX**: Users know exactly when and why password is needed |
| 124 | +4. **Simple Implementation**: Leverages existing sudo cache mechanism (~15 minutes) |
| 125 | +5. **Backwards Compatible**: Doesn't require system configuration changes |
| 126 | +6. **Universal**: Works across different Linux distributions and environments |
| 127 | +
|
| 128 | +## Implementation Details |
| 129 | +
|
| 130 | +### Core Functions (`scripts/shell-utils.sh`) |
| 131 | +
|
| 132 | +```bash |
| 133 | +# Check if sudo credentials are cached |
| 134 | +is_sudo_cached() { |
| 135 | + sudo -n true 2>/dev/null |
| 136 | +} |
| 137 | +
|
| 138 | +# Warn user and ensure sudo is cached |
| 139 | +ensure_sudo_cached() { |
| 140 | + local operation_description="${1:-the operation}" |
| 141 | +
|
| 142 | + if is_sudo_cached; then |
| 143 | + return 0 |
| 144 | + fi |
| 145 | +
|
| 146 | + log_warning "The next step requires administrator privileges" |
| 147 | + log_info "You may be prompted for your password to ${operation_description}" |
| 148 | +
|
| 149 | + # Use harmless sudo command to cache credentials |
| 150 | + if sudo -v; then |
| 151 | + log_success "Administrator privileges confirmed" |
| 152 | + return 0 |
| 153 | + else |
| 154 | + log_error "Failed to obtain administrator privileges" |
| 155 | + return 1 |
| 156 | + fi |
| 157 | +} |
| 158 | +``` |
| 159 | +
|
| 160 | +### Integration Pattern |
| 161 | +
|
| 162 | +```bash |
| 163 | +# Before any infrastructure operation that needs sudo |
| 164 | +if ! ensure_sudo_cached "provision libvirt infrastructure"; then |
| 165 | + log_error "Cannot proceed without administrator privileges" |
| 166 | + exit 1 |
| 167 | +fi |
| 168 | +
|
| 169 | +# Now run operations that need sudo - no prompts expected |
| 170 | +sudo chown -R libvirt-qemu:libvirt /var/lib/libvirt/images/ |
| 171 | +``` |
| 172 | +
|
| 173 | +## Consequences |
| 174 | +
|
| 175 | +### Positive |
| 176 | +
|
| 177 | +- **Better UX**: Clear, predictable password prompts |
| 178 | +- **No Mixed Output**: Password prompt happens before verbose operations |
| 179 | +- **Faster Tests**: No hanging due to missed prompts |
| 180 | +- **Security Maintained**: Uses standard sudo timeout mechanism |
| 181 | +- **Universal**: Works in all environments without special setup |
| 182 | +
|
| 183 | +### Negative |
| 184 | +
|
| 185 | +- **Still Requires Password**: Users must enter password once per test session |
| 186 | +- **Cache Dependency**: Relies on system sudo timeout (usually 15 minutes) |
| 187 | +- **Additional Code**: Added complexity in shell utilities |
| 188 | +
|
| 189 | +### Neutral |
| 190 | +
|
| 191 | +- **Test Duration**: No impact on test execution time |
| 192 | +- **Security Posture**: Maintains existing security model |
| 193 | +- **Maintenance**: Minimal ongoing maintenance required |
| 194 | +
|
| 195 | +## Monitoring |
| 196 | +
|
| 197 | +Success of this decision can be measured by: |
| 198 | +
|
| 199 | +1. **Reduced Support Issues**: Fewer reports of hanging tests or missed prompts |
| 200 | +2. **Contributor Feedback**: Improved developer experience feedback |
| 201 | +3. **Test Reliability**: More consistent test execution without manual intervention |
| 202 | +
|
| 203 | +## Related Decisions |
| 204 | +
|
| 205 | +- [ADR-001: Makefile Location](001-makefile-location.md) - Central automation interface |
| 206 | +- [ADR-002: Docker for All Services](002-docker-for-all-services.md) - Service architecture |
| 207 | +
|
| 208 | +## References |
| 209 | +
|
| 210 | +- Original issue discussion with password prompt mixing |
| 211 | +- Shell utilities implementation in `scripts/shell-utils.sh` |
| 212 | +- Integration testing guide documentation |
| 213 | +- Sudo cache timeout documentation: `man sudo` |
0 commit comments