-
Notifications
You must be signed in to change notification settings - Fork 122
Self discovery mode #718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Self discovery mode #718
Conversation
WalkthroughThis pull request introduces auto-discovery functionality that detects cloud provider configurations from environment variables and runtime settings. The feature adds a new AutoDiscovery mechanism spanning multiple files, integrates provider discovery into the runner's initialization path, and updates inventory creation to handle graceful provider initialization failures. Changes
Sequence DiagramsequenceDiagram
participant Runner
participant AutoDiscovery
participant ProviderDiscoverers as K8s/AWS/GCP/Azure
participant Inventory
participant Providers
Runner->>Runner: New(options) called with AutoDiscovery enabled
alt AutoDiscovery enabled
Runner->>AutoDiscovery: NewAutoDiscovery(options)
AutoDiscovery->>ProviderDiscoverers: discoverKubernetes()
ProviderDiscoverers-->>AutoDiscovery: OptionBlock (k8s config)
AutoDiscovery->>ProviderDiscoverers: discoverAWS()
ProviderDiscoverers-->>AutoDiscovery: OptionBlock (AWS credentials)
AutoDiscovery->>ProviderDiscoverers: discoverGCP()
ProviderDiscoverers-->>AutoDiscovery: OptionBlock (GCP config)
AutoDiscovery->>ProviderDiscoverers: discoverAzure()
ProviderDiscoverers-->>AutoDiscovery: OptionBlock (Azure identity)
AutoDiscovery-->>Runner: Discovered provider configurations
else
Runner->>Runner: Load provider config from file (existing path)
end
Runner->>Inventory: NewWithOptions(discoveredConfig, graceful=true, verbose)
Inventory->>Providers: Create each provider
alt Provider creation succeeds
Providers-->>Inventory: Provider instance
else Provider creation fails
Inventory->>Inventory: Record failure, log, continue (graceful mode)
end
Inventory-->>Runner: Initialized Inventory with available providers
Runner->>Inventory: Enumerate()
Inventory->>Providers: Run()
Providers-->>Inventory: Results
Inventory-->>Runner: Enumeration complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/runner/autodiscovery.go (1)
70-92: Consider cross-platform KUBECONFIG path separator.Line 80 splits
KUBECONFIGon:, but Windows uses;as the path separator. Consider usingfilepath.ListSeparatorfor cross-platform compatibility.- if strings.Contains(kubeconfigPath, ":") { - kubeconfigPath = strings.Split(kubeconfigPath, ":")[0] + if strings.Contains(kubeconfigPath, string(filepath.ListSeparator)) { + kubeconfigPath = strings.Split(kubeconfigPath, string(filepath.ListSeparator))[0] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
cmd/cloudlist/main.go(1 hunks)internal/runner/autodiscovery.go(1 hunks)internal/runner/options.go(2 hunks)internal/runner/runner.go(4 hunks)pkg/inventory/inventory.go(3 hunks)pkg/providers/aws/aws.go(2 hunks)pkg/providers/k8s/kubernetes.go(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
pkg/providers/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/providers/**/*.go: All providers must implement the schema.Provider interface with methods: Name(), ID(), Resources(ctx), Services()
For complex providers, keep service-specific logic in separate files (e.g., instances.go, dns.go, route53.go, s3.go, eks.go) and return *schema.Resources merged by the main Resources()
Parse provider configuration via block.GetMetadata(key); support environment variables using $VAR syntax; return &schema.ErrNoSuchKey{Name: key} for missing required config
Respect service filtering: Services() must list supported services, and providers should honor -s filters when gathering resources
Always pass and honor context.Context in Resources() and long-running API calls to support cancellation
Do not manually deduplicate resources; use Resources.Append() to rely on schema’s ResourceDeduplicator
Each emitted resource must have at least one of IP or DNS populated; empty resources are invalid
Treat provider config id as a user-defined identifier for filtering, not a cloud resource ID
Files:
pkg/providers/k8s/kubernetes.gopkg/providers/aws/aws.go
pkg/providers/*/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Each provider directory must include a main file named .go defining New(block schema.OptionBlock) and Resources(ctx) orchestration
Files:
pkg/providers/k8s/kubernetes.gopkg/providers/aws/aws.go
pkg/inventory/inventory.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/inventory/inventory.go: Register every new provider in nameToProvider() using a case mapping from provider name to implementation
Keep service names consistent: entries in each provider’s Services() must match the inventory Providers map
When adding a provider, also update the Providers map with the provider and its supported services
Files:
pkg/inventory/inventory.go
🧬 Code graph analysis (4)
pkg/providers/aws/aws.go (1)
pkg/schema/schema.go (1)
ErrNoSuchKey(166-168)
internal/runner/options.go (1)
internal/runner/autodiscovery.go (1)
AutoDiscovery(19-21)
pkg/inventory/inventory.go (2)
internal/runner/options.go (1)
Options(22-39)pkg/schema/schema.go (1)
Options(176-176)
internal/runner/autodiscovery.go (2)
internal/runner/options.go (1)
Options(22-39)pkg/schema/schema.go (1)
OptionBlock(195-195)
🔇 Additional comments (25)
cmd/cloudlist/main.go (1)
14-16: LGTM!The nil check prevents a potential panic when auto-discovery returns no providers. The early return is appropriate.
internal/runner/options.go (2)
38-38: LGTM!The AutoDiscovery field is properly documented and integrates cleanly with the existing Options structure.
81-81: LGTM!The flag definition is clear and follows the existing pattern with both long and short forms.
pkg/providers/k8s/kubernetes.go (3)
31-31: LGTM!The constant follows the existing naming pattern for configuration options.
39-43: LGTM!The validation logic correctly checks for all three configuration options and provides a helpful error message.
57-61: LGTM!The in-cluster configuration path is correctly implemented using
rest.InClusterConfig()with proper error wrapping. This enables Kubernetes provider discovery when running inside a pod.pkg/providers/aws/aws.go (2)
49-69: LGTM!The credential handling correctly makes credentials optional when using IMDS or ECS task roles, allowing the AWS SDK to use its default credential chain. The error handling properly returns
&schema.ErrNoSuchKeywhen required credentials are missing in non-metadata scenarios.
147-152: LGTM!Only setting credentials when both AccessKey and SecretKey are non-empty allows the AWS SDK to fall back to its default credential chain (IMDS, ECS task role, environment variables, credentials file). This is the correct approach for auto-discovery mode.
pkg/inventory/inventory.go (2)
38-40: LGTM!Delegating to NewWithOptions maintains backward compatibility while enabling the new graceful failure behavior.
42-79: LGTM!The graceful failure handling is well-implemented. Failed providers are tracked and logged appropriately based on the verbose setting, allowing enumeration to continue with successfully initialized providers. This is essential for auto-discovery mode where some providers may not be available.
internal/runner/runner.go (3)
27-53: LGTM!The auto-discovery integration is well-structured. The code appropriately bypasses provider config file reading when auto-discovery is enabled, provides helpful hints when no providers are discovered, and gracefully returns nil to signal no providers available.
64-66: LGTM!Correctly prevents default providers from being added when auto-discovery is active, allowing only discovered providers to be used.
107-125: LGTM!The graceful failure mode for auto-discovery is correctly implemented. Using
NewWithOptionswith graceful failure allows partial success, and the check for empty providers prevents proceeding with no valid providers. This provides a better user experience in auto-discovery scenarios.internal/runner/autodiscovery.go (12)
18-26: LGTM!The AutoDiscovery struct and constructor are simple and well-defined.
28-68: LGTM!The discovery orchestration logic is clean and well-structured. Using
getProvidersToCheckallows filtering by user-specified providers or defaulting to all supported providers.
94-102: LGTM!The in-cluster mode detection correctly checks for the service account token file and returns appropriate configuration for Kubernetes pods.
104-119: LGTM!The kubeconfig file discovery logic appropriately checks for file existence before returning the configuration.
121-177: LGTM!The AWS discovery implements a sensible priority order: IMDS → ECS credentials → environment variables → credentials file. The verbose logging about AWS profiles and credentials files helps users understand why discovery might not succeed.
179-234: LGTM!The IMDS detection correctly implements IMDSv2 with token-based authentication first, then falls back to IMDSv1. The 2-second timeout is appropriate for metadata service calls. Both IMDSv2 and IMDSv1 paths properly validate role availability before returning configuration.
236-259: LGTM!The IMDSv2 role validation correctly uses the token header and checks for a non-empty response.
261-279: LGTM!The ECS credentials detection correctly checks both
AWS_CONTAINER_CREDENTIALS_RELATIVE_URIandAWS_CONTAINER_CREDENTIALS_FULL_URIenvironment variables, covering different ECS configuration scenarios.
325-367: LGTM!The GCP metadata service detection correctly checks for the service account token endpoint with proper headers and validates the JSON response structure. The 2-second timeout is appropriate.
369-421: LGTM!The Azure discovery implements a comprehensive priority order: Managed Identity → environment variables → Azure CLI. The verbose logging helps users understand configuration requirements.
423-523: LGTM!The Azure Managed Identity detection is comprehensive, covering App Service/Functions MSI, Container Instance MSI, and VM/AKS IMDS. Each path correctly validates the response and extracts the subscription ID when available. The HTTP requests properly use context with timeout and close response bodies.
281-323: ADC fallback for empty GCP service account key is already implemented.The
registerWithOptions()function inpkg/providers/gcp/auth.go(lines 187-200) explicitly handles empty credentials: whenlen(serviceAccountKey) == 0(which occurs whengcp_service_account_keyis an empty string from the metadata service), the code falls back togoogle.FindDefaultCredentials(), which uses Application Default Credentials (ADC). This behavior is documented in comments atpkg/providers/gcp/gcp.golines 190-192 and implemented as designed. The concern raised in the review is already addressed by the existing codebase.
This PR adds support for auto discovery. With the -ad flag, you no longer need to provide a provider config manually—it automatically detects environment variables or IAM attachments on VMs/pods and uses the appropriate authentication method.
Testing checklist:
Summary by CodeRabbit
Release Notes
New Features
--auto-discoveryflag to enable provider auto-detection without requiring explicit configuration files.Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.