| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
- Add cmd/bcloud directory structure with main.go and command files - Add cobra and viper dependencies for CLI framework - Implement bcloud <refId> <verb> command structure - Support YAML credential configuration in ~/.bcloud/credentials.yaml - Use YAML key as ref_id by default with optional explicit override - Implement YAML output format for all commands - Add core commands: create, terminate, list, get, types - Integrate with existing CloudClient interfaces and provider implementations - Support LambdaLabs provider with extensible factory pattern Co-Authored-By: Alec Fong <alecsanf@usc.edu>
Co-Authored-By: Alec Fong <alecsanf@usc.edu>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Sorry, something went wrong.
|
I don't like how we are doing factory, config etc can we do something closer to this design? package creds
import (
"encoding/json"
"fmt"
"gopkg.in/yaml.v3"
)
// ---------- Core interfaces & registry ----------
type ProviderCredential interface {
GetCloudProviderID() string
}
var providerRegistry = map[string]func() ProviderCredential{}
func RegisterProvider(id string, factory func() ProviderCredential) {
providerRegistry[id] = factory
}
// ---------- Wrapper type (provider-discriminated union) ----------
type CredentialEntry struct {
// Persisted "provider" key in files
Provider string `json:"provider" yaml:"provider"`
// In-memory typed value; excluded from default serialization
Value ProviderCredential `json:"-" yaml:"-"`
}
// Common decode from a generic map (used by JSON & YAML)
func (c *CredentialEntry) decodeFromMap(m map[string]any) error {
rawProv, ok := m["provider"]
if !ok {
return fmt.Errorf("missing 'provider'")
}
provider, ok := rawProv.(string)
if !ok || provider == "" {
return fmt.Errorf("invalid 'provider'")
}
factory, ok := providerRegistry[provider]
if !ok {
return fmt.Errorf("unknown provider: %s", provider)
}
cred := factory()
// Reuse JSON to map→struct unmarshal; unknown keys are ignored by struct
b, err := json.Marshal(m)
if err != nil {
return err
}
if err := json.Unmarshal(b, cred); err != nil {
return err
}
c.Provider = provider
c.Value = cred
return nil
}
// Common encode to a generic map (used by JSON & YAML)
func (c CredentialEntry) encodeToMap() (map[string]any, error) {
if c.Value == nil {
return nil, fmt.Errorf("nil credential Value")
}
b, err := json.Marshal(c.Value) // serialize provider-specific fields
if err != nil {
return nil, err
}
out := map[string]any{}
if err := json.Unmarshal(b, &out); err != nil {
return nil, err
}
out["provider"] = c.Value.GetCloudProviderID()
return out, nil
}
// ---------- JSON round-trip ----------
func (c *CredentialEntry) UnmarshalJSON(b []byte) error {
m := map[string]any{}
if err := json.Unmarshal(b, &m); err != nil {
return err
}
return c.decodeFromMap(m)
}
func (c CredentialEntry) MarshalJSON() ([]byte, error) {
m, err := c.encodeToMap()
if err != nil {
return nil, err
}
return json.Marshal(m)
}
// ---------- YAML round-trip ----------
func (c *CredentialEntry) UnmarshalYAML(n *yaml.Node) error {
m := map[string]any{}
if err := n.Decode(&m); err != nil {
return err
}
return c.decodeFromMap(m)
}
func (c CredentialEntry) MarshalYAML() (interface{}, error) {
m, err := c.encodeToMap()
if err != nil {
return nil, err
}
return m, nil // let yaml encode the map
}
// ---------- File container ----------
type CredentialFile struct {
Credentials map[string]CredentialEntry `json:"credentials" yaml:"credentials"`
}
// ---------- Provider implementations this would be in the respective provider packages ----------
// Lambda Labs
type LambdaLabsCredential struct {
APIKey string `json:"api_key" yaml:"api_key"`
DefaultLocation string `json:"default_location" yaml:"default_location"`
}
func (l *LambdaLabsCredential) GetCloudProviderID() string { return "lambdalabs" } // use provider id const
// Nebius
type NebiusCredential struct {
ServiceAccountJSON string `json:"service_account_json" yaml:"service_account_json"`
DefaultLocation string `json:"default_location" yaml:"default_location"`
RefID string `json:"ref_id,omitempty" yaml:"ref_id,omitempty"`
}
func (n *NebiusCredential) GetCloudProviderID() string { return "nebius" } // use provider id const
// Register at init (or from each provider package's init)
func init() {
RegisterProvider("lambdalabs", func() ProviderCredential { return &LambdaLabsCredential{} }) // use provider id const
RegisterProvider("nebius", func() ProviderCredential { return &NebiusCredential{} }) // use provider id const
}
|
Sorry, something went wrong.
|
lets test this method as well |
Sorry, something went wrong.
…eedback - Replace custom ProviderCredential with existing CloudCredential interface - Add DefaultLocationProvider interface for CLI-specific functionality - Update factory pattern to work with CloudCredential interface - Maintain key-as-ref_id pattern with improved architecture - All lint checks and tests pass Co-Authored-By: Alec Fong <alecsanf@usc.edu>
| } | ||
| cred := factory() | ||
|
|
||
| if _, hasRefID := m["ref_id"]; !hasRefID { |
There was a problem hiding this comment.
lets write a test that says all registered providers must have a ref_id in struct
Sorry, something went wrong.
There was a problem hiding this comment.
we don't need to recreate the credentials here this exactly logic lies in internal/lambda/v1... lets just register them. this file can probably removed and registration can happen in config.go
Sorry, something went wrong.
There was a problem hiding this comment.
i think this file can be deleted
Sorry, something went wrong.
…K implementations - Delete cmd/bcloud/providers/factory.go as requested - Remove cmd/bcloud/config/providers.go and move registration to config.go - Use existing LambdaLabsCredential and NebiusCredential from SDK with wrapper types - Add CLI-specific DefaultLocationProvider interface for default_location field - Add test ensuring all registered providers have RefID field in struct - Update all command files to work with simplified credential system - All lint checks and tests pass Co-Authored-By: Alec Fong <alecsanf@usc.edu>
…sing - Test LoadConfig with various scenarios: success, missing files, invalid YAML - Test credential parsing edge cases: missing provider, invalid provider, unknown provider - Test key-as-ref_id pattern with both explicit and implicit ref_id values - Test JSON/YAML marshaling and unmarshaling for credential entries - Test provider registry functionality and wrapper type validation - Test error handling for malformed configurations and nil values - Test default settings application and file path resolution - All tests use isolated temp directories and proper cleanup - Fixed lint issues: error checking for os.Setenv/os.Chdir, file permissions, formatting Co-Authored-By: Alec Fong <alecsanf@usc.edu>
| var providerRegistry = map[string]func() v1.CloudCredential{} | ||
|
|
||
| var providerIDToRegistryKey = map[string]string{ | ||
| "lambda-labs": "lambdalabs", |
There was a problem hiding this comment.
these should be the same (provider id and registry key) strings lets refactor
Sorry, something went wrong.
|
|
||
| if _, hasRefID := m["ref_id"]; !hasRefID { | ||
| m["ref_id"] = yamlKey | ||
| if _, hasRefID := m["RefID"]; !hasRefID { |
There was a problem hiding this comment.
lets go in to the provider credential and use tags to change them to ref_id api_key etc.
Sorry, something went wrong.
- Add JSON tags to LambdaLabsCredential and NebiusCredential structs - Use lowercase field names: ref_id, api_key, service_account_key, project_id - Unify Lambda Labs provider ID from 'lambda-labs' to 'lambdalabs' to match registry key - Remove providerIDToRegistryKey mapping from config.go (no longer needed) - Add provider constants (ProviderLambdaLabs, ProviderNebius) to eliminate goconst lint warnings - Update all tests to use new lowercase field names and provider constants - All tests pass and lint checks pass Co-Authored-By: Alec Fong <alecsanf@usc.edu>
| Back | FazBrowse Home | New Git URL |
Add comprehensive testing and implement GitHub feedback for CLI credential system
Summary
This PR adds extensive testing to the bcloud CLI configuration system and implements feedback to standardize credential field naming and provider IDs. The changes ensure robust error handling and edge case coverage for credential loading while maintaining backward compatibility patterns.
Key Changes:
Review & Testing Checklist for Human
Critical Items (3):
Important Items (2):
Diagram
%%{ init : { "theme" : "default" }}%% graph TD subgraph "CLI System" A["cmd/bcloud/config/config.go"]:::major-edit B["cmd/bcloud/config/config_test.go"]:::major-edit C["cmd/bcloud/commands/create.go"]:::context end subgraph "SDK Provider Structs" D["internal/lambdalabs/v1/client.go"]:::minor-edit E["internal/nebius/v1/client.go"]:::minor-edit end subgraph "Core SDK" F["pkg/v1/client.go CloudCredential interface"]:::context end A --> F A --> D A --> E B --> A C --> A D --> F E --> F subgraph Legend L1["Major Edit"]:::major-edit L2["Minor Edit"]:::minor-edit L3["Context/No Edit"]:::context end classDef major-edit fill:#90EE90 classDef minor-edit fill:#87CEEB classDef context fill:#FFFFFFNotes
Testing Coverage Added:
Breaking Changes:
Session Info: