feat: Implement context-aware provider architecture (v2.0) #7
Conversation
This commit implements a comprehensive architectural refactor that transforms
supactl from a bifurcated "two-mode CLI" into a unified, context-aware tool
inspired by kubectl.
## Milestone 1: Context-Aware Refactor
### New Provider Interface (internal/provider/)
- Created InstanceProvider interface defining unified contract for all backends
- Implemented RemoteProvider wrapping the existing API client
- Implemented LocalProvider wrapping local Docker management
- Unified Instance type supporting both remote and local fields
### Context-Aware Configuration (internal/auth/config.go)
- Refactored config structure to support multiple contexts
- New format: { "current-context": "...", "contexts": {...} }
- Automatic migration from legacy v1.x format
- Context management methods (Get/Set/Add/Remove/List)
### Unified Commands (cmd/*.go)
- Refactored all commands to use provider interface
- Single command set works with any provider (start, stop, list, etc.)
- Context-aware: behavior determined by active context
- Replaced getAPIClient() with getProvider() factory in root.go
## Milestone 2: kubectl Command Alignment
### Context Management (cmd/config.go)
- config get-contexts: List all contexts
- config use-context: Switch active context
- config current-context: Display current context
- config set-context: Create/update contexts
- config delete-context: Remove contexts
### Resource Commands
- get instances: kubectl-style list command
- describe instance: kubectl-style detailed view
## Benefits
1. **Unified Developer Experience**: One command set for all backends
- Before: `supactl start` (remote) vs `supactl local start` (local)
- After: `supactl start` works for both (context determines backend)
2. **Familiar UX**: kubectl-style commands for K8s users
- Context switching: `supactl config use-context local`
- Resource viewing: `supactl get instances`, `supactl describe instance my-app`
3. **Multi-Backend Support**: Easy to add new providers in future
- Provider interface is extensible
- Factory pattern in getProvider() handles instantiation
4. **Backward Compatibility**:
- Legacy config format auto-migrated to new structure
- Traditional commands (list, status) remain available
- Existing local commands (local add, local start) unchanged
## Testing
- All code compiles successfully
- Provider interface implemented by both RemoteProvider and LocalProvider
- Context configuration supports both local and remote providers
## Documentation
- CLAUDE.md updated with new architecture details
- Provider interface documented
- Context management workflows explained
- kubectl-style commands documented
## Future Work (Milestone 3)
- Declarative management: `supactl apply -f instance.yaml`
- Requires server-side POST /api/v1/apply endpoint implementation
---
Closes #[issue-number] (if applicable)
Summary of ChangesHello @michaelruelas, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant architectural refactor to Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed architectural refactor, unifying the CLI's local and remote modes under a context-aware provider pattern inspired by kubectl. The introduction of the InstanceProvider interface, RemoteProvider, and LocalProvider is a great step towards a more maintainable and extensible codebase. The new kubectl-style commands for context management are a welcome addition for user experience. My review focuses on improving consistency, robustness, and cross-platform compatibility. Specifically, I've suggested replacing hardcoded strings with constants, using tabwriter for consistent output formatting, improving cross-platform IP address detection, and making timestamp parsing more robust. Overall, this is an excellent enhancement to the project.
cmd/config.go
Outdated
| if setContextProvider == "remote" && (setContextServer == "" || setContextAPIKey == "") { | ||
| fmt.Fprintf(os.Stderr, "Error: Remote contexts require --server and --api-key flags\n") | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
The provider type "remote" is hardcoded. It's better to use the provider.ProviderTypeRemote constant defined in internal/provider/provider.go to avoid magic strings and ensure consistency. You'll need to add import "github.com/qubitquilt/supactl/internal/provider" to use it.
| if setContextProvider == "remote" && (setContextServer == "" || setContextAPIKey == "") { | |
| fmt.Fprintf(os.Stderr, "Error: Remote contexts require --server and --api-key flags\n") | |
| os.Exit(1) | |
| } | |
| if setContextProvider == provider.ProviderTypeRemote && (setContextServer == "" || setContextAPIKey == "") { | |
| fmt.Fprintf(os.Stderr, "Error: Remote contexts require --server and --api-key flags\n") | |
| os.Exit(1) | |
| } |
cmd/config.go
Outdated
| if setContextProvider == "remote" { | ||
| ctx.ServerURL = setContextServer | ||
| ctx.APIKey = setContextAPIKey | ||
| } |
There was a problem hiding this comment.
The provider type "remote" is hardcoded. It's better to use the provider.ProviderTypeRemote constant defined in internal/provider/provider.go to avoid magic strings and ensure consistency. You'll need to add import "github.com/qubitquilt/supactl/internal/provider" to use it.
| if setContextProvider == "remote" { | |
| ctx.ServerURL = setContextServer | |
| ctx.APIKey = setContextAPIKey | |
| } | |
| if setContextProvider == provider.ProviderTypeRemote { | |
| ctx.ServerURL = setContextServer | |
| ctx.APIKey = setContextAPIKey | |
| } |
cmd/describe.go
Outdated
| fmt.Printf("Name:\t\t%s\n", instance.Name) | ||
| fmt.Printf("Status:\t\t%s\n", instance.Status) | ||
| fmt.Printf("Studio URL:\t%s\n", instance.StudioURL) | ||
|
|
||
| if instance.APIURL != "" { | ||
| fmt.Printf("API URL:\t%s\n", instance.APIURL) | ||
| } | ||
| if instance.KongURL != "" { | ||
| fmt.Printf("Kong URL:\t%s\n", instance.KongURL) | ||
| } | ||
| if instance.DatabaseURL != "" { | ||
| fmt.Printf("Database URL:\t%s\n", instance.DatabaseURL) | ||
| } | ||
| if instance.Directory != "" { | ||
| fmt.Printf("Directory:\t%s\n", instance.Directory) | ||
| } | ||
| if instance.DBPort != 0 { | ||
| fmt.Printf("DB Port:\t%d\n", instance.DBPort) | ||
| } | ||
| if instance.AnonKey != "" { | ||
| fmt.Printf("Anon Key:\t%s\n", instance.AnonKey) | ||
| } | ||
| if instance.ServiceKey != "" { | ||
| fmt.Printf("Service Key:\t%s\n", instance.ServiceKey) | ||
| } | ||
| if !instance.CreatedAt.IsZero() { | ||
| fmt.Printf("Created:\t%s\n", instance.CreatedAt.Format("2006-01-02 15:04:05")) | ||
| } |
There was a problem hiding this comment.
The output is formatted using tabs (\t), which can lead to misaligned columns if some field names or values are longer than others. For more robust and consistently aligned output, consider using the text/tabwriter package, which is already used in other commands like get and list. You will need to add import "text/tabwriter".
w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
fmt.Fprintf(w, "Name:\t%s\n", instance.Name)
fmt.Fprintf(w, "Status:\t%s\n", instance.Status)
fmt.Fprintf(w, "Studio URL:\t%s\n", instance.StudioURL)
if instance.APIURL != "" {
fmt.Fprintf(w, "API URL:\t%s\n", instance.APIURL)
}
if instance.KongURL != "" {
fmt.Fprintf(w, "Kong URL:\t%s\n", instance.KongURL)
}
if instance.DatabaseURL != "" {
fmt.Fprintf(w, "Database URL:\t%s\n", instance.DatabaseURL)
}
if instance.Directory != "" {
fmt.Fprintf(w, "Directory:\t%s\n", instance.Directory)
}
if instance.DBPort != 0 {
fmt.Fprintf(w, "DB Port:\t%d\n", instance.DBPort)
}
if instance.AnonKey != "" {
fmt.Fprintf(w, "Anon Key:\t%s\n", instance.AnonKey)
}
if instance.ServiceKey != "" {
fmt.Fprintf(w, "Service Key:\t%s\n", instance.ServiceKey)
}
if !instance.CreatedAt.IsZero() {
fmt.Fprintf(w, "Created:\t%s\n", instance.CreatedAt.Format("2006-01-02 15:04:05"))
}
w.Flush()| }, nil | ||
| } | ||
| return nil, fmt.Errorf("failed to read config file: %w", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
The provider type "local" is hardcoded as a string. The internal/provider package defines constants ProviderTypeLocal and ProviderTypeRemote for this purpose. Using these constants throughout this file would improve maintainability and prevent bugs from typos. You'll need to add import "github.com/qubitquilt/supactl/internal/provider" to use them. This applies to all hardcoded "local" and "remote" provider strings in this file.
| Contexts: map[string]*ContextConfig{ | |
| "local": { | |
| Provider: "local", | |
| }, | |
| }, | |
| Contexts: map[string]*ContextConfig{ | |
| "local": { | |
| Provider: provider.ProviderTypeLocal, | |
| }, | |
| }, |
Updated test files to work with the new context-aware configuration structure: - internal/auth/config_test.go: Updated to use SaveLegacyConfig instead of old SaveConfig signature - internal/auth/config_test.go: Updated assertions to check context structure - internal/auth/config_test.go: Added test for local context in IsLoggedIn - internal/auth/config_test.go: Removed unused encoding/json import - cmd/root_test.go: Updated to use SaveLegacyConfig All tests now pass successfully with the new architecture.
…t IP retrieval Use Go's net.InterfaceAddrs() to find the first non-loopback IPv4 address, improving portability and reliability over external command execution.
Use provider constants instead of magic strings for better maintainability and type safety in context validation and setup.
Replace legacy SaveLegacyConfig with new LoadConfig, AddContext, and SaveConfig functions to support context-aware provider architecture. Ensure default and local contexts are properly initialized and set.
Replace direct fmt.Printf calls with tabwriter for better alignment in the describe command output, mimicking kubectl describe style.
Add alternative date format parsing to handle cases where the API returns timestamps not in RFC3339 format, preventing parsing errors.
michaelruelas
deleted the
claude/supactl-architecture-roadmap-01EHW6QEao8sBx76fcZkAd1b
branch
This commit implements a comprehensive architectural refactor that transforms supactl from a bifurcated "two-mode CLI" into a unified, context-aware tool inspired by kubectl.
Milestone 1: Context-Aware Refactor
New Provider Interface (internal/provider/)
Context-Aware Configuration (internal/auth/config.go)
Unified Commands (cmd/*.go)
Milestone 2: kubectl Command Alignment
Context Management (cmd/config.go)
Resource Commands
Benefits
Unified Developer Experience: One command set for all backends
supactl start(remote) vssupactl local start(local)supactl startworks for both (context determines backend)Familiar UX: kubectl-style commands for K8s users
supactl config use-context localsupactl get instances,supactl describe instance my-appMulti-Backend Support: Easy to add new providers in future
Backward Compatibility:
Testing
Documentation
Future Work (Milestone 3)
supactl apply -f instance.yamlCloses #[issue-number] (if applicable)