feat: add DigitalOcean key CRUD commands - #53
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds three DigitalOcean SSH key console commands, new traits for DigitalOcean initialization and key helpers/validation, a BaseDigitalOceanService for centralized API client management, refactors DigitalOcean services to extend it, and registers the commands in the Symfony application. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as KeyAddDigitalOceanCommand
participant Trait as DigitalOceanCommandTrait
participant Env as EnvService
participant IO as IOService
participant DOService as DigitalOceanService
participant API as DigitalOcean API
User->>CLI: run key:add:digitalocean
CLI->>Trait: initializeDigitalOceanAPI()
Trait->>Env: read DIGITALOCEAN_API_TOKEN / DO_API_TOKEN
Trait->>IO: show spinner "Initializing DigitalOcean API..."
Trait->>DOService: initializeAPI() (private)
DOService->>API: construct & authenticate client
API-->>DOService: authenticated client
DOService-->>Trait: return Client
Trait->>IO: hide spinner, return SUCCESS
CLI->>IO: prompt for public key path and name
User-->>CLI: enter path/name
CLI->>CLI: validate path & name (KeyValidationTrait / KeyHelpersTrait)
CLI->>IO: show spinner "Uploading key..."
CLI->>API: uploadKey(path, name)
API-->>CLI: success + key id
CLI->>IO: hide spinner, show success + usage hint
CLI-->>User: exit SUCCESS
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Services/DigitalOcean/DigitalOceanKeyService.php (1)
57-74: Remove string cast; reconsider 404 detection approach.The parameter cast is valid—the library accepts
intdirectly. However, the proposedgetCode() === 404check is unreliable without catching the specific HTTP exception type. The codebase currently uses message-based detection uniformly across both KeyService and DropletService; either keep this pattern for consistency or properly catch the HTTP client's typed exception (e.g., Guzzle'sClientExceptionwithgetResponse()->getStatusCode()).- $keyApi = $client->key(); - $keyApi->remove((string) $keyId); + $keyApi = $client->key(); + $keyApi->remove($keyId); } catch (\Throwable $e) { // Check if 404 (already deleted) - silently succeed $message = strtolower($e->getMessage());
🧹 Nitpick comments (8)
app/Services/DigitalOceanService.php (3)
71-92: Wire subservices only after successful auth.Today, account/key/droplet services are set before verifyAuthentication(). If auth fails, they remain wired with an unusable client. Move setAPI calls to after a successful verification.
Apply minimal diff:
@@ public function initialize(string $token): void - $this->setToken($token); - $this->initializeAPI(); - $this->verifyAuthentication(); + $this->setToken($token); + $this->initializeAPI(); + $this->verifyAuthentication(); + // Wire subservices only after successful auth + $api = $this->initializeAPI(); // idempotent + $this->account->setAPI($api); + $this->key->setAPI($api); + $this->droplet->setAPI($api);@@ public function initializeAPI(): Client - $this->account->setAPI($this->api); - $this->key->setAPI($this->api); - $this->droplet->setAPI($this->api);
71-92: Prefer marking initializeAPI() as internal (or reduce to protected).If external access is required, annotate with a DocBlock “@internal” to discourage direct command usage and keep initialization through initialize().
56-62: Minimize token residency.Optionally null out $this->token after client creation to reduce in‑memory exposure; reauth isn’t needed unless setToken() is called again.
app/Traits/DigitalOceanCommandTrait.php (1)
24-57: Minor DX: trim token and centralize env var keys.
- Trim the token before emptiness checks to avoid whitespace issues.
- Extract ['DIGITALOCEAN_API_TOKEN','DO_API_TOKEN'] to a private const for reuse/readability.
app/Services/DigitalOcean/DigitalOceanDropletService.php (1)
168-185: Prefer status code/typed exception over message text for 404.Mirror the approach suggested in KeyService: check $e->getCode() === 404 (or catch the library’s NotFound exception) instead of parsing error strings.
Diff:
- } catch (\Throwable $e) { - // Check if 404 (already deleted) - silently succeed - $message = strtolower($e->getMessage()); - if (str_contains($message, '404') || str_contains($message, 'not found')) { + } catch (\Throwable $e) { + if ((int) $e->getCode() === 404) { return; } // Other errors - throw throw new \RuntimeException("Failed to destroy droplet: {$e->getMessage()}", 0, $e); }app/Console/Key/KeyListDigitalOceanCommand.php (1)
71-73: Consider adding a header for better readability.The key listing is functional but lacks a header row. Adding a simple header would improve readability, especially with many keys.
Apply this diff to add a header:
+ $this->io->writeln(''); + $this->io->writeln(' <fg=cyan>ID</> - <fg=cyan>Description</>'); + $this->io->writeln(''); + foreach ($keys as $keyId => $description) { $this->io->writeln(" <fg=cyan>{$keyId}</> - {$description}"); }app/Services/DigitalOcean/DigitalOceanAccountService.php (1)
119-119: Minor docblock/method name inconsistency.The docblocks say "Get available" while the method names are
getUserVpcsandgetUserSshKeys. While the "available" terminology is accurate for filtering, the "user" prefix indicates resources belonging to the user's account. This minor inconsistency is acceptable, but for perfect alignment, consider either keeping both as "user" or both as "available" in future refactors.Also applies to: 145-145
app/Console/Key/KeyDeleteDigitalOceanCommand.php (1)
95-96: Redundant null coalescing operator.Line 96:
getOption('force')for aVALUE_NONEoption always returns a boolean, so?? falseis redundant. However, this is a minor style point and doesn't affect functionality.Apply this diff:
- /** @var bool $forceSkip */ - $forceSkip = $input->getOption('force') ?? false; + /** @var bool $forceSkip */ + $forceSkip = (bool) $input->getOption('force');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/Console/Key/KeyAddDigitalOceanCommand.php(1 hunks)app/Console/Key/KeyDeleteDigitalOceanCommand.php(1 hunks)app/Console/Key/KeyListDigitalOceanCommand.php(1 hunks)app/Contracts/BaseCommand.php(1 hunks)app/Services/DigitalOcean/BaseDigitalOceanService.php(1 hunks)app/Services/DigitalOcean/DigitalOceanAccountService.php(3 hunks)app/Services/DigitalOcean/DigitalOceanDropletService.php(1 hunks)app/Services/DigitalOcean/DigitalOceanKeyService.php(1 hunks)app/Services/DigitalOceanService.php(4 hunks)app/SymfonyApp.php(2 hunks)app/Traits/DigitalOceanCommandTrait.php(1 hunks)app/Traits/KeyHelpersTrait.php(1 hunks)app/Traits/KeyValidationTrait.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/00-main.mdc)
**/*.php: Eliminate single-use methods: inline if a method is called only once
Cache computed values: initialize expensive calculations in the constructor
Avoid method call overhead: prefer direct property access when appropriate
**/*.php: Adhere to PSR-12, enable strict_types, and leverage PHP 8.x features (union types, match, attributes, readonly)
Use explicit return types, including generic-like annotations where applicable (e.g., Collection<int, User>)
Prefer Symfony components (e.g., Filesystem, Process) over native PHP functions for testability
Always import classes via use statements; only use root FQDNs for core exceptions (e.g., \InvalidArgumentException, \RuntimeException). Do not use inline FQDNs for non-root namespaces
Create objects via $container->build(ClassName::class) everywhere except DTOs, value objects, and pure data structures
Use minimalist DocBlocks documenting description, parameters, and return types for classes and functions
Follow the specified comment structure with section headers/subheaders and spacing; remove obsolete comments with removed code
Files:
app/Contracts/BaseCommand.phpapp/Traits/KeyHelpersTrait.phpapp/Traits/KeyValidationTrait.phpapp/Services/DigitalOcean/DigitalOceanKeyService.phpapp/Services/DigitalOcean/DigitalOceanDropletService.phpapp/Services/DigitalOcean/DigitalOceanAccountService.phpapp/Services/DigitalOcean/BaseDigitalOceanService.phpapp/Console/Key/KeyListDigitalOceanCommand.phpapp/SymfonyApp.phpapp/Console/Key/KeyAddDigitalOceanCommand.phpapp/Traits/DigitalOceanCommandTrait.phpapp/Console/Key/KeyDeleteDigitalOceanCommand.phpapp/Services/DigitalOceanService.php
**/*Command.php
📄 CodeRabbit inference engine (.cursor/rules/01-architecture.mdc)
**/*Command.php: Commands handle user interaction (I/O), orchestration, styling, error formatting, and prompts using SymfonyStyle
Commands must not contain business logic; delegate to Services
Commands must not invoke other commands (no proxy commands)
Files:
app/Contracts/BaseCommand.phpapp/Console/Key/KeyListDigitalOceanCommand.phpapp/Console/Key/KeyAddDigitalOceanCommand.phpapp/Console/Key/KeyDeleteDigitalOceanCommand.php
**/*{Command,Service}.php
📄 CodeRabbit inference engine (.cursor/rules/01-architecture.mdc)
All dependencies should be expressed in constructor signatures; avoid circular dependencies
Files:
app/Contracts/BaseCommand.phpapp/Services/DigitalOcean/DigitalOceanKeyService.phpapp/Services/DigitalOcean/DigitalOceanDropletService.phpapp/Services/DigitalOcean/DigitalOceanAccountService.phpapp/Services/DigitalOcean/BaseDigitalOceanService.phpapp/Console/Key/KeyListDigitalOceanCommand.phpapp/Console/Key/KeyAddDigitalOceanCommand.phpapp/Console/Key/KeyDeleteDigitalOceanCommand.phpapp/Services/DigitalOceanService.php
**/BaseCommand.php
📄 CodeRabbit inference engine (.cursor/rules/03-commands.mdc)
**/BaseCommand.php: If an IO helper is missing, add it to BaseCommand with modern styling instead of using Symfony IO directly.
BaseCommand handles shared initialization/configuration/orchestration only; do not implement individual I/O operations here.
Files:
app/Contracts/BaseCommand.php
**/*ValidationTrait.php
📄 CodeRabbit inference engine (.cursor/rules/03-commands.mdc)
**/*ValidationTrait.php: Validation methods for prompts/options must accept mixed and return ?string error or null (do not throw exceptions).
Naming: use validateInput() for prompt/option validators returning ?string; use validate() to throw exceptions for heavy I/O validations.
Files:
app/Traits/KeyValidationTrait.php
**/*Service.php
📄 CodeRabbit inference engine (.cursor/rules/01-architecture.mdc)
**/*Service.php: Services must perform no console I/O and should accept/return plain PHP types
Services are dependency-injected via constructor and encapsulate business logic, external APIs, and file operations
Stateful services should use lazy loading and explicit initialization methods (e.g., load(), initialize()) and document requirements
Files:
app/Services/DigitalOcean/DigitalOceanKeyService.phpapp/Services/DigitalOcean/DigitalOceanDropletService.phpapp/Services/DigitalOcean/DigitalOceanAccountService.phpapp/Services/DigitalOcean/BaseDigitalOceanService.phpapp/Services/DigitalOceanService.php
**/SymfonyApp.php
📄 CodeRabbit inference engine (.cursor/rules/01-architecture.mdc)
Register console commands in SymfonyApp.php
Files:
app/SymfonyApp.php
🧬 Code graph analysis (10)
app/Traits/KeyHelpersTrait.php (1)
app/Services/IOService.php (6)
IOService(30-583)writeln(463-469)warning(490-493)getOptionOrPrompt(84-132)promptSelect(304-322)error(498-501)
app/Traits/KeyValidationTrait.php (2)
app/Traits/KeyHelpersTrait.php (1)
expandKeyPath(41-53)app/Services/FilesystemService.php (2)
exists(41-44)readFile(51-54)
app/Services/DigitalOcean/DigitalOceanKeyService.php (2)
app/Services/DigitalOcean/BaseDigitalOceanService.php (1)
BaseDigitalOceanService(14-39)app/Services/FilesystemService.php (1)
FilesystemService(27-106)
app/Services/DigitalOcean/DigitalOceanDropletService.php (1)
app/Services/DigitalOcean/BaseDigitalOceanService.php (1)
BaseDigitalOceanService(14-39)
app/Services/DigitalOcean/DigitalOceanAccountService.php (1)
app/Services/DigitalOcean/BaseDigitalOceanService.php (1)
BaseDigitalOceanService(14-39)
app/Console/Key/KeyListDigitalOceanCommand.php (6)
app/Contracts/BaseCommand.php (2)
BaseCommand(29-158)execute(136-157)app/Console/Key/KeyAddDigitalOceanCommand.php (2)
AsCommand(21-128)execute(48-127)app/Console/Key/KeyDeleteDigitalOceanCommand.php (2)
AsCommand(19-159)execute(46-158)app/Services/IOService.php (6)
hr(517-523)h1(506-512)promptSpin(439-452)error(498-501)writeln(463-469)warning(490-493)app/Traits/DigitalOceanCommandTrait.php (1)
initializeDigitalOceanAPI(24-57)app/Services/DigitalOcean/DigitalOceanAccountService.php (1)
getUserSshKeys(149-167)
app/Console/Key/KeyAddDigitalOceanCommand.php (6)
app/Contracts/BaseCommand.php (3)
BaseCommand(29-158)configure(68-85)execute(136-157)app/Services/IOService.php (9)
hr(517-523)h1(506-512)getValidatedOptionOrPrompt(159-182)promptText(200-218)promptSpin(439-452)success(482-485)writeln(463-469)error(498-501)showCommandHint(530-566)app/Traits/DigitalOceanCommandTrait.php (1)
initializeDigitalOceanAPI(24-57)app/Traits/KeyValidationTrait.php (2)
validateKeyPathInput(25-72)validateKeyNameInput(81-98)app/Traits/KeyHelpersTrait.php (1)
expandKeyPath(41-53)app/Services/DigitalOcean/DigitalOceanKeyService.php (1)
uploadKey(31-46)
app/Traits/DigitalOceanCommandTrait.php (3)
app/Services/IOService.php (3)
promptSpin(439-452)error(498-501)writeln(463-469)app/Contracts/BaseCommand.php (1)
initialize(97-127)app/Services/DigitalOceanService.php (1)
initialize(44-49)
app/Console/Key/KeyDeleteDigitalOceanCommand.php (6)
app/Contracts/BaseCommand.php (3)
BaseCommand(29-158)configure(68-85)execute(136-157)app/Services/IOService.php (11)
hr(517-523)h1(506-512)error(498-501)writeln(463-469)promptText(200-218)getOptionOrPrompt(84-132)promptConfirm(260-276)warning(490-493)promptSpin(439-452)success(482-485)showCommandHint(530-566)app/Traits/DigitalOceanCommandTrait.php (1)
initializeDigitalOceanAPI(24-57)app/Services/DigitalOcean/DigitalOceanAccountService.php (1)
getUserSshKeys(149-167)app/Traits/KeyHelpersTrait.php (1)
selectKey(63-97)app/Services/DigitalOcean/DigitalOceanKeyService.php (1)
deleteKey(57-74)
app/Services/DigitalOceanService.php (1)
app/Services/DigitalOcean/BaseDigitalOceanService.php (1)
setAPI(21-24)
🔇 Additional comments (8)
app/Contracts/BaseCommand.php (1)
55-56: LGTM — comment clarification is accurate.app/SymfonyApp.php (1)
8-10: LGTM — commands imported and registered.Also applies to: 126-129
app/Services/DigitalOcean/BaseDigitalOceanService.php (1)
14-38: LGTM — centralized API client management is clear and safe.app/Services/DigitalOcean/DigitalOceanDropletService.php (2)
136-156: No changes needed—networks structure confirmed.The code correctly handles the actual structure:
DropletEntity->networksis a flat array of Network objects where both v4 and v6 API arrays are merged, with version property set on each entry. The current iteration over$droplet->networkschecking->typeand->versionproperties is already appropriate.
53-66: Verify the exact parameter order of Droplet API create() in v5.0.0 against your environment.The web search confirms that
toin0u/digitalocean-v2supports the parameters in your code (backups, ipv6, vpc_uuid, ssh_keys, user_data, monitoring, volumes), but public documentation does not expose the exact method signature with parameter order. The code passes 11 arguments positionally:name, region, size, image, backups, ipv6, vpc_uuid, ssh_keys, user_data, monitoring, volumesSince parameter order for optional positional arguments is critical and can silently fail if misaligned, inspect the library's
Dropletclass in yourvendor/toin0u/digitalocean-v2directory to confirm this order matches thecreate()method signature in v5.0.0.app/Services/DigitalOcean/DigitalOceanAccountService.php (1)
16-16: LGTM! Clean refactoring to shared base class.The refactoring to extend
BaseDigitalOceanServiceeliminates code duplication across DigitalOcean services while preserving the public API.app/Console/Key/KeyAddDigitalOceanCommand.php (1)
48-127: LGTM! Well-structured command with proper validation.The command follows best practices:
- Delegates business logic to services
- Uses trait-based validation and helpers
- Provides clear error messages and success feedback
- Handles both interactive and non-interactive modes
app/Traits/KeyHelpersTrait.php (1)
63-97: LGTM! Robust key selection with proper validation.The
selectKeymethod properly handles:
- Empty key lists (returns success, not an error)
- Interactive and non-interactive modes
- Validation of selected key existence (critical for
--keyoption)
| $this->io->showCommandHint('key:delete:digitalocean', [ | ||
| 'key' => (string) $keyId, | ||
| 'yes' => $confirmed, | ||
| 'force' => true, | ||
| ]); |
There was a problem hiding this comment.
Command hint always shows --force regardless of actual usage.
Line 154 hardcodes 'force' => true in the command hint, even when the user didn't use the --force flag. This could mislead users into thinking they used --force when they went through typed confirmation.
Apply this diff to show the actual force flag state:
$this->io->showCommandHint('key:delete:digitalocean', [
'key' => (string) $keyId,
'yes' => $confirmed,
- 'force' => true,
+ 'force' => $forceSkip,
]);🤖 Prompt for AI Agents
In app/Console/Key/KeyDeleteDigitalOceanCommand.php around lines 151 to 155, the
command hint hardcodes 'force' => true which inaccurately shows --force even
when not used; replace the hardcoded true with the actual force flag state (e.g.
use the command's option or the local $confirmed/$force variable) so the hint
reflects whether --force was provided.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Services/DigitalOceanService.php (1)
56-62: Inline single-use method.The
setToken()method is called only once frominitialize()(line 48). Per the coding guideline to "eliminate single-use methods: inline if a method is called only once," this should be inlined directly intoinitialize()to reduce method call overhead.As per coding guidelines.
Apply this diff:
public function initialize(string $token): void { - $this->setToken($token); + $this->token = $token; + // Reset client every time a new token is set + $this->api = null; $this->initializeAPI(); $this->verifyAuthentication(); } - - /** - * Set a DigitalOcean API token. - */ - private function setToken(string $token): void - { - $this->token = $token; - - // Reset client every time a new token is set - $this->api = null; - }
🧹 Nitpick comments (1)
app/Services/DigitalOceanService.php (1)
97-107: MakeverifyAuthentication()private and use direct property access to eliminate redundant method call overhead.The method is only called from
initialize()(line 50) afterinitializeAPI()has already set$this->api(line 49). No external calls exist. Change it to private and access$this->apidirectly instead of callinginitializeAPI()again on line 99, removing unnecessary method invocation overhead per the coding guideline.- public function verifyAuthentication(): void + private function verifyAuthentication(): void { - $api = $this->initializeAPI(); - try { // Use account endpoint to verify token validity - $api->account()->getUserInformation(); + $this->api->account()->getUserInformation(); } catch (\Throwable $e) { throw new \RuntimeException('Failed to authenticate with DigitalOcean API: ' . $e->getMessage(), 0, $e); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Services/DigitalOceanService.php(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/00-main.mdc)
**/*.php: Eliminate single-use methods: inline if a method is called only once
Cache computed values: initialize expensive calculations in the constructor
Avoid method call overhead: prefer direct property access when appropriate
**/*.php: Adhere to PSR-12, enable strict_types, and leverage PHP 8.x features (union types, match, attributes, readonly)
Use explicit return types, including generic-like annotations where applicable (e.g., Collection<int, User>)
Prefer Symfony components (e.g., Filesystem, Process) over native PHP functions for testability
Always import classes via use statements; only use root FQDNs for core exceptions (e.g., \InvalidArgumentException, \RuntimeException). Do not use inline FQDNs for non-root namespaces
Create objects via $container->build(ClassName::class) everywhere except DTOs, value objects, and pure data structures
Use minimalist DocBlocks documenting description, parameters, and return types for classes and functions
Follow the specified comment structure with section headers/subheaders and spacing; remove obsolete comments with removed code
Files:
app/Services/DigitalOceanService.php
**/*Service.php
📄 CodeRabbit inference engine (.cursor/rules/01-architecture.mdc)
**/*Service.php: Services must perform no console I/O and should accept/return plain PHP types
Services are dependency-injected via constructor and encapsulate business logic, external APIs, and file operations
Stateful services should use lazy loading and explicit initialization methods (e.g., load(), initialize()) and document requirements
Files:
app/Services/DigitalOceanService.php
**/*{Command,Service}.php
📄 CodeRabbit inference engine (.cursor/rules/01-architecture.mdc)
All dependencies should be expressed in constructor signatures; avoid circular dependencies
Files:
app/Services/DigitalOceanService.php
🧬 Code graph analysis (1)
app/Services/DigitalOceanService.php (1)
app/Services/DigitalOcean/BaseDigitalOceanService.php (1)
setAPI(21-24)
🔇 Additional comments (1)
app/Services/DigitalOceanService.php (1)
46-51: LGTM!The
initialize()method provides a clean, well-documented entry point for API setup. The orchestration of token setup, API initialization, and authentication verification is clear and follows the stated requirement that it "must be called before making any API calls."
1 participant
Summary by CodeRabbit