diff --git a/cmd/root.go b/cmd/root.go index 2ca49fddae..a4a6af6605 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -28,6 +28,8 @@ import ( "github.com/cloudposse/atmos/internal/tui/templates" "github.com/cloudposse/atmos/internal/tui/templates/term" cfg "github.com/cloudposse/atmos/pkg/config" + // Import adapters to register them with the config package. + _ "github.com/cloudposse/atmos/pkg/config/adapters" "github.com/cloudposse/atmos/pkg/data" "github.com/cloudposse/atmos/pkg/filesystem" "github.com/cloudposse/atmos/pkg/flags" diff --git a/docs/prd/import-adapter-registry.md b/docs/prd/import-adapter-registry.md new file mode 100644 index 0000000000..9332f421fc --- /dev/null +++ b/docs/prd/import-adapter-registry.md @@ -0,0 +1,779 @@ +# Import Adapter Registry Pattern + +## Overview + +This document describes the import adapter registry pattern for Atmos, which provides a modular, extensible architecture for custom import schemes that require **transformation** (like `terragrunt://` for HCL→YAML conversion, or `mock://` for testing). Standard go-getter schemes continue to work through the existing infrastructure. + +## Problem Statement + +### Current State + +Atmos uses [go-getter](https://github.com/hashicorp/go-getter) for downloading remote imports, which supports many schemes: +- `http://`, `https://` - HTTP(S) downloads +- `git::` - Git repositories +- `s3::` - Amazon S3 +- `gcs::` - Google Cloud Storage +- `oci://` - OCI registries +- And many more (file://, hg::, scp://, sftp://, etc.) + +**However**, the current implementation has two issues: + +```go +// pkg/config/imports.go - Current implementation ONLY checks http/https +func isRemoteImport(importPath string) bool { + return strings.HasPrefix(importPath, "http://") || strings.HasPrefix(importPath, "https://") +} + +// downloadRemoteConfig() DOES use go-getter, but isRemoteImport() blocks other schemes! +``` + +**Issue 1:** The `isRemoteImport()` function only routes `http://` and `https://` to go-getter, even though `downloadRemoteConfig()` already uses go-getter which supports many more schemes. + +**Issue 2:** Some use cases need **transformation** before the content becomes valid YAML: +- `terragrunt://` - Parse HCL (terragrunt.hcl) and convert to YAML +- `mock://` - Generate synthetic YAML for testing without external dependencies + +### Challenges + +1. **Underutilized go-getter** - go-getter supports many schemes but only http/https are routed to it +2. **No transformation support** - Can't handle schemes that need content transformation (HCL→YAML) +3. **No testing support** - No mock scheme for testing imports without external dependencies +4. **Migration friction** - Users migrating from Terragrunt cannot reference existing terragrunt.hcl files + +### Key Requirements + +1. **Fix go-getter routing** - Route all go-getter schemes (git::, s3::, gcs::, oci://, etc.) properly +2. **Support import adapters** - Enable `terragrunt://`, `mock://` for content transformation +3. **Maintain backward compatibility** - Existing local and HTTP/HTTPS imports must work unchanged +4. **Support testing** - Mock adapter for unit tests without external dependencies +5. **Preserve recursive imports** - Nested imports continue to work across all schemes + +## Functional Requirements + +### FR-1: Import Adapter Interface + +| ID | Requirement | +|----|-------------| +| FR-1.1 | The system SHALL provide an `ImportAdapter` interface with `Schemes()` and `Resolve()` methods | +| FR-1.2 | The `Schemes()` method SHALL return URL schemes/prefixes the adapter handles (e.g., `["http://", "https://"]`) | +| FR-1.3 | The `Resolve()` method SHALL accept context, import path, base path, temp directory, currentDepth, and maxDepth parameters | +| FR-1.4 | The `Resolve()` method SHALL return a list of resolved file paths and any errors | +| FR-1.5 | Import adapters SHALL be able to generate, transform, or fetch content in custom ways | + +### FR-2: Import Adapter Registry + +| ID | Requirement | +|----|-------------| +| FR-2.1 | The system SHALL provide a global registry for import adapters | +| FR-2.2 | The registry SHALL support registration via `RegisterImportAdapter()` function | +| FR-2.3 | The registry SHALL prevent duplicate registration of the same scheme | +| FR-2.4 | The registry SHALL normalize schemes to lowercase for case-insensitive matching | +| FR-2.5 | The registry SHALL be thread-safe using appropriate synchronization | +| FR-2.6 | The registry SHALL provide `GetImportAdapter()` to retrieve adapters by scheme | +| FR-2.7 | The registry SHALL provide `ResetImportAdapterRegistry()` for testing purposes | +| FR-2.8 | Adapters SHALL self-register via `init()` functions | + +### FR-3: Scheme Detection and Routing + +| ID | Requirement | +|----|-------------| +| FR-3.1 | The system SHALL check for registered import adapters before go-getter routing | +| FR-3.2 | The system SHALL route all go-getter schemes (http, https, git::, s3::, gcs::, oci://, etc.) to go-getter | +| FR-3.3 | The system SHALL fall back to local filesystem for paths without recognized schemes | +| FR-3.4 | The `extractScheme()` function SHALL extract the scheme from paths containing "://" | +| FR-3.5 | The `extractScheme()` function SHALL return empty string for paths without schemes | + +### FR-4: Mock Import Adapter + +| ID | Requirement | +|----|-------------| +| FR-4.1 | The system SHALL provide a `MockAdapter` for testing purposes | +| FR-4.2 | The mock adapter SHALL handle the `mock://` scheme | +| FR-4.3 | `mock://empty` SHALL generate an empty YAML configuration | +| FR-4.4 | `mock://error` SHALL return a simulated error for testing error handling | +| FR-4.5 | `mock://nested` SHALL generate a configuration with nested mock:// imports | +| FR-4.6 | `mock://` SHALL generate a configuration with `mock_path` and `mock_source` vars | +| FR-4.7 | The mock adapter SHALL support custom data injection via `MockData` field | +| FR-4.8 | The mock adapter SHALL write generated YAML to the temp directory | +| FR-4.9 | The mock adapter SHALL use unique filenames to prevent collisions | + +### FR-5: go-getter Integration + +| ID | Requirement | +|----|-------------| +| FR-5.1 | The `isRemoteImport()` function SHALL recognize all go-getter schemes | +| FR-5.2 | Supported go-getter schemes SHALL include: http://, https://, git::, git@, s3::, s3://, gcs::, gcs://, oci://, file://, hg::, scp://, sftp://, github.com/, bitbucket.org/ | +| FR-5.3 | The `isRemoteImport()` function SHALL return false for import adapter schemes | +| FR-5.4 | The existing `downloadRemoteConfig()` function SHALL continue to use go-getter | + +### FR-6: Backward Compatibility + +| ID | Requirement | +|----|-------------| +| FR-6.1 | Existing local file imports SHALL continue to work unchanged | +| FR-6.2 | Existing HTTP/HTTPS imports SHALL continue to work unchanged | +| FR-6.3 | Nested/recursive imports SHALL continue to work across all schemes | +| FR-6.4 | The `ResolvedPaths` struct SHALL support a new `ADAPTER` import type | + +## Solution: Two-Layer Architecture + +### Design Philosophy + +Rather than replacing go-getter, we **embrace** it for standard downloads and add a **transformation layer** for schemes that need content conversion: + +1. **Layer 1: go-getter** - Handles all standard download schemes (http, https, git::, s3::, gcs::, oci://, etc.) +2. **Layer 2: Transformation Adapters** - Handles schemes that need content transformation before becoming valid YAML + +### Architecture Overview + +```text +┌─────────────────────────────────────────────────────────────┐ +│ Import Resolution │ +├─────────────────────────────────────────────────────────────┤ +│ │ +│ importPath │ +│ │ │ +│ ▼ │ +│ ┌────────────────────────────────────────────┐ │ +│ │ Scheme Detection & Routing │ │ +│ └────────────────────────────────────────────┘ │ +│ │ │ │ +│ │ Transformation Schemes │ Standard Schemes │ +│ │ (terragrunt://, mock://) │ (everything else) │ +│ ▼ ▼ │ +│ ┌─────────────────┐ ┌──────────────────────┐ │ +│ │ Transformation │ │ go-getter │ │ +│ │ Adapter Registry│ │ (existing) │ │ +│ │ │ │ │ │ +│ │ - mock:// │ │ - http(s):// │ │ +│ │ - terragrunt:// │ │ - git:: │ │ +│ │ (future) │ │ - s3:: │ │ +│ │ │ │ - gcs:: │ │ +│ │ │ │ - oci:// │ │ +│ │ │ │ - file:// │ │ +│ │ │ │ - local paths │ │ +│ └─────────────────┘ └──────────────────────┘ │ +│ │ │ │ +│ │ Generate YAML │ Download YAML │ +│ ▼ ▼ │ +│ ┌────────────────────────────────────────────┐ │ +│ │ Merge Configuration │ │ +│ └────────────────────────────────────────────┘ │ +│ │ +└─────────────────────────────────────────────────────────────┘ +``` + +### Key Design Principles + +1. **go-getter first** - Standard schemes continue using go-getter (our existing investment) +2. **Transformation adapters for special cases** - Only schemes needing transformation use adapters +3. **Minimal surface area** - Small, focused adapter interface +4. **Backward compatibility** - Existing imports work without changes +5. **Testability** - Mock adapter enables testing without external dependencies + +## Implementation + +### 1. Fix isRemoteImport() to Properly Route go-getter Schemes + +The first and most important change is fixing the scheme detection to route all go-getter schemes properly: + +```go +// pkg/config/imports.go - UPDATED + +// isRemoteImport determines if the import path should use go-getter. +// This includes all schemes that go-getter supports. +func isRemoteImport(importPath string) bool { + // Check for import adapters first (they handle their own schemes). + if hasImportAdapter(importPath) { + return false + } + + // go-getter schemes (not exhaustive, go-getter handles detection). + goGetterPrefixes := []string{ + "http://", + "https://", + "git::", + "git@", // SSH git + "s3::", + "s3://", + "gcs::", + "gcs://", + "oci://", + "file://", + "hg::", + "scp://", + "sftp://", + "github.com/", // GitHub shorthand + "bitbucket.org/", // Bitbucket shorthand + } + + lowered := strings.ToLower(importPath) + for _, prefix := range goGetterPrefixes { + if strings.HasPrefix(lowered, prefix) { + return true + } + } + + return false +} +``` + +### 2. Import Adapter Interface (for mock://, terragrunt://) + +Import adapters handle custom schemes that need special processing (content transformation, synthetic generation, etc.): + +```go +// pkg/config/import_adapter.go +package config + +import ( + "context" +) + +// ImportAdapter handles custom import schemes that need special processing. +// Unlike go-getter schemes (which download YAML directly), import adapters +// can generate, transform, or fetch content in custom ways. +// +// Examples: +// - mock:// - Generates synthetic YAML for testing +// - terragrunt:// - Transforms HCL to YAML (future) +type ImportAdapter interface { + // Schemes returns URL schemes/prefixes this adapter handles. + // Return empty slice for default adapter (LocalAdapter). + // Examples: ["http://", "https://", "git::"] or ["mock://"] + Schemes() []string + + // Resolve processes an import path and returns YAML file paths. + // + // Parameters: + // - ctx: Context for cancellation and deadlines + // - importPath: The full import path (e.g., "mock://component/vpc") + // - basePath: The base path for resolving relative references + // - tempDir: Temporary directory for generated files + // - currentDepth: Current recursion depth for nested imports + // - maxDepth: Maximum allowed recursion depth + // + // Returns: + // - []ResolvedPaths: List of YAML file paths to merge + // - error: Any error encountered during resolution + Resolve( + ctx context.Context, + importPath string, + basePath string, + tempDir string, + currentDepth int, + maxDepth int, + ) ([]ResolvedPaths, error) +} +``` + +### 3. Import Adapter Registry + +Small registry for custom import adapters: + +```go +// pkg/config/import_adapter_registry.go +package config + +import ( + "fmt" + "strings" + "sync" +) + +var ( + importAdapters = &ImportAdapterRegistry{ + adapters: make(map[string]ImportAdapter), + } +) + +// ImportAdapterRegistry manages import adapter registration. +type ImportAdapterRegistry struct { + mu sync.RWMutex + adapters map[string]ImportAdapter +} + +// RegisterImportAdapter adds an import adapter to the registry. +func RegisterImportAdapter(adapter ImportAdapter) error { + importAdapters.mu.Lock() + defer importAdapters.mu.Unlock() + + if adapter == nil { + return fmt.Errorf("import adapter cannot be nil") + } + + scheme := strings.ToLower(adapter.Scheme()) + if scheme == "" { + return fmt.Errorf("import adapter scheme cannot be empty") + } + + if _, exists := importAdapters.adapters[scheme]; exists { + return fmt.Errorf("import adapter for scheme %q already registered", scheme) + } + + importAdapters.adapters[scheme] = adapter + return nil +} + +// GetImportAdapter returns an import adapter by scheme. +func GetImportAdapter(scheme string) (ImportAdapter, bool) { + importAdapters.mu.RLock() + defer importAdapters.mu.RUnlock() + + adapter, ok := importAdapters.adapters[strings.ToLower(scheme)] + return adapter, ok +} + +// hasImportAdapter checks if there's a registered import adapter for the path. +func hasImportAdapter(importPath string) bool { + scheme := extractScheme(importPath) + _, ok := GetImportAdapter(scheme) + return ok +} + +// extractScheme extracts the URL scheme from an import path. +func extractScheme(importPath string) string { + if idx := strings.Index(importPath, "://"); idx > 0 { + return strings.ToLower(importPath[:idx]) + } + return "" +} + +// ResetImportAdapterRegistry clears the registry (for testing only). +func ResetImportAdapterRegistry() { + importAdapters.mu.Lock() + defer importAdapters.mu.Unlock() + importAdapters.adapters = make(map[string]ImportAdapter) +} +``` + +### 4. Mock Import Adapter (For Testing) + +```go +// pkg/config/adapters/mock_adapter.go +package adapters + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/cloudposse/atmos/pkg/config" +) + +// MockAdapter generates synthetic YAML for testing. +// This enables unit tests without external dependencies. +// +// Usage: +// - mock://empty → Empty YAML config +// - mock://error → Returns error (test error handling) +// - mock://nested → Config with nested mock:// imports +// - mock://custom/path → Config with mock_path set to "custom/path" +// +// Tests can inject custom data via MockData field. +type MockAdapter struct { + // MockData allows tests to inject custom YAML content. + // Key is the path after "mock://", value is YAML content. + MockData map[string]string +} + +func init() { + if err := config.RegisterImportAdapter(&MockAdapter{}); err != nil { + panic(fmt.Sprintf("failed to register mock adapter: %v", err)) + } +} + +func (m *MockAdapter) Scheme() string { + return "mock" +} + +func (m *MockAdapter) Resolve( + ctx context.Context, + importPath string, + basePath string, + tempDir string, +) ([]config.ResolvedPaths, error) { + mockPath := strings.TrimPrefix(importPath, "mock://") + + switch mockPath { + case "error": + return nil, fmt.Errorf("mock error: simulated import failure") + case "empty": + return m.writeConfig(importPath, tempDir, "# Empty mock configuration\n") + case "nested": + return m.writeConfig(importPath, tempDir, `# Nested mock configuration +import: + - mock://component/base + +vars: + nested: true + level: 1 +`) + default: + // Check for injected mock data. + if m.MockData != nil { + if content, ok := m.MockData[mockPath]; ok { + return m.writeConfig(importPath, tempDir, content) + } + } + + // Generate default mock config. + content := fmt.Sprintf(`# Mock configuration for %s +vars: + mock_path: "%s" + mock_source: "mock_adapter" +`, mockPath, mockPath) + return m.writeConfig(importPath, tempDir, content) + } +} + +func (m *MockAdapter) writeConfig(importPath, tempDir, content string) ([]config.ResolvedPaths, error) { + fileName := fmt.Sprintf("mock-import-%d.yaml", time.Now().UnixNano()) + filePath := filepath.Join(tempDir, fileName) + + if err := os.WriteFile(filePath, []byte(content), 0o644); err != nil { + return nil, fmt.Errorf("failed to write mock config: %w", err) + } + + return []config.ResolvedPaths{ + { + FilePath: filePath, + ImportPaths: importPath, + ImportType: config.ADAPTER, + }, + }, nil +} +``` + +### 5. Updated imports.go Integration + +```go +// pkg/config/imports.go - Key changes + +type importTypes int + +const ( + LOCAL importTypes = 0 + REMOTE importTypes = 1 + ADAPTER importTypes = 2 // For import adapters (mock://, terragrunt://) +) + +func processImports(basePath string, importPaths []string, tempDir string, currentDepth, maxDepth int) (resolvedPaths []ResolvedPaths, err error) { + // ... existing validation ... + + ctx := context.Background() + + for _, importPath := range importPaths { + if importPath == "" { + continue + } + + var paths []ResolvedPaths + var err error + + // Check for import adapter first. + if adapter, ok := GetImportAdapter(extractScheme(importPath)); ok { + paths, err = adapter.Resolve(ctx, importPath, basePath, tempDir) + } else if isRemoteImport(importPath) { + // Use go-getter for remote imports. + paths, err = processRemoteImport(basePath, importPath, tempDir, currentDepth, maxDepth) + } else { + // Local filesystem import. + paths, err = processLocalImport(basePath, importPath, tempDir, currentDepth, maxDepth) + } + + if err != nil { + log.Debug("failed to resolve import", "path", importPath, "error", err) + continue + } + + resolvedPaths = append(resolvedPaths, paths...) + } + + return resolvedPaths, nil +} +``` + +## Use Cases + +### Use Case 1: Mock Adapter for Testing + +```go +// pkg/config/imports_test.go +func TestImportResolution(t *testing.T) { + // Inject custom mock data for tests. + mockAdapter := &adapters.MockAdapter{ + MockData: map[string]string{ + "base/defaults": `vars: + cidr: "10.0.0.0/16" + environment: test`, + }, + } + config.ResetImportAdapterRegistry() + config.RegisterImportAdapter(mockAdapter) + + // Test import resolution. + paths, err := processImports(basePath, []string{"mock://base/defaults"}, tempDir, 1, 25) + assert.NoError(t, err) + assert.Len(t, paths, 1) + assert.Equal(t, config.ADAPTER, paths[0].ImportType) +} +``` + +### Use Case 2: Nested Mock Imports + +```yaml +# Importing "mock://nested" generates config with further imports: +# import: +# - mock://component/base +# vars: +# nested: true + +# This enables testing complex import chains without external dependencies. +``` + +### Use Case 3: All go-getter Schemes Now Work + +With the fixed `isRemoteImport()`: + +```yaml +import: + # All of these now work (they already were supposed to!) + - git::https://github.com/acme/configs//stacks/catalog/vpc?ref=v1.0.0 + - s3::https://s3.amazonaws.com/acme-configs/stacks/catalog/vpc.yaml + - gcs::https://storage.googleapis.com/acme-configs/stacks/catalog/eks.yaml + - oci://registry.example.com/configs:v1.0.0 + - github.com/acme/configs//stacks/defaults +``` + +### Use Case 4: Future Terragrunt Adapter (Phase 2) + +```yaml +# stacks/migration.yaml - During Terragrunt migration +import: + # Transform Terragrunt HCL to YAML on-the-fly + - terragrunt://legacy/prod/vpc/terragrunt.hcl + + # Mix with native Atmos imports + - _defaults/globals +``` + +## Testing Strategy + +### 1. Import Adapter Registry Tests + +```go +// pkg/config/import_adapter_registry_test.go + +func TestRegisterImportAdapter(t *testing.T) { + ResetImportAdapterRegistry() + + adapter := &MockAdapter{} + err := RegisterImportAdapter(adapter) + assert.NoError(t, err) +} + +func TestExtractScheme(t *testing.T) { + tests := []struct { + path string + scheme string + }{ + {"mock://test", "mock"}, + {"terragrunt://path/to/file", "terragrunt"}, + {"http://example.com", "http"}, + {"/absolute/path", ""}, // No scheme + {"relative/path", ""}, // No scheme + } + + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + assert.Equal(t, tt.scheme, extractScheme(tt.path)) + }) + } +} +``` + +### 2. Mock Adapter Tests + +```go +// pkg/config/adapters/mock_adapter_test.go + +func TestMockAdapterResolve(t *testing.T) { + adapter := &MockAdapter{} + tempDir := t.TempDir() + + tests := []struct { + name string + importPath string + wantErr bool + }{ + {"empty config", "mock://empty", false}, + {"error simulation", "mock://error", true}, + {"nested config", "mock://nested", false}, + {"custom path", "mock://component/vpc", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + paths, err := adapter.Resolve(context.Background(), tt.importPath, "/base", tempDir) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Len(t, paths, 1) + } + }) + } +} +``` + +### 3. isRemoteImport Tests + +```go +func TestIsRemoteImport(t *testing.T) { + tests := []struct { + path string + expected bool + }{ + // go-getter schemes + {"http://example.com/config.yaml", true}, + {"https://example.com/config.yaml", true}, + {"git::https://github.com/org/repo//path", true}, + {"s3::https://s3.amazonaws.com/bucket/key", true}, + {"gcs::https://storage.googleapis.com/bucket/key", true}, + {"oci://registry.example.com/image:tag", true}, + {"github.com/org/repo//path", true}, + + // Local paths + {"/absolute/path", false}, + {"relative/path", false}, + {"./current/path", false}, + + // Import adapters (not go-getter) + {"mock://test", false}, + {"terragrunt://path", false}, + } + + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + assert.Equal(t, tt.expected, isRemoteImport(tt.path)) + }) + } +} +``` + +## Implementation Plan + +### Phase 1: Fix go-getter Routing + Mock Adapter + +**Goal:** Route all go-getter schemes properly and add mock:// for testing. + +**Tasks:** +1. Update `isRemoteImport()` to recognize all go-getter schemes +2. Create `pkg/config/import_adapter.go` with `TransformationAdapter` interface +3. Create `pkg/config/import_adapter_registry.go` with registry +4. Implement `MockAdapter` in `pkg/config/adapters/mock_adapter.go` +5. Update `processImports()` to check transformation adapters first +6. Add comprehensive tests + +**Files to modify:** +- `pkg/config/imports.go` - Fix `isRemoteImport()`, add transformation adapter check +- `pkg/config/import_adapter.go` - New: TransformationAdapter interface +- `pkg/config/import_adapter_registry.go` - New: Registry functions +- `pkg/config/adapters/mock_adapter.go` - New: Mock adapter implementation +- `pkg/config/adapters/mock_adapter_test.go` - New: Mock adapter tests +- `pkg/config/imports_test.go` - Add isRemoteImport and integration tests + +**Success Criteria:** +- ✅ All go-getter schemes (git::, s3::, gcs::, oci://, etc.) work for imports +- ✅ `mock://` adapter generates synthetic YAML for testing +- ✅ Existing local/http imports unchanged +- ✅ 90%+ test coverage on new code + +### Phase 2: Terragrunt Adapter (Future) + +**Goal:** Transform terragrunt.hcl to YAML for migration support. + +**Tasks:** +1. Create `TerragruntAdapter` that parses HCL +2. Transform Terragrunt `inputs` → Atmos `vars` +3. Transform `include` → `import` +4. Handle `dependency` blocks + +### Phase 3: Additional Import Adapters (Future) + +Potential future adapters: +- `consul://` - Fetch and transform Consul KV to YAML +- `vault://` - Fetch and transform Vault secrets to YAML +- Custom format converters + +**Note:** s3://, git::, gcs::, oci:// already work via go-getter - no adapters needed. + +## Benefits + +### Immediate Benefits (Phase 1) + +1. ✅ **Full go-getter support** - All documented schemes now work (git::, s3::, oci://, etc.) +2. ✅ **Testability** - Mock adapter enables testing without external dependencies +3. ✅ **Minimal changes** - Small, focused transformation adapter layer +4. ✅ **Backward compatibility** - Leverages existing go-getter investment + +### Future Benefits (Phase 2+) + +5. ✅ **Migration support** - Terragrunt users can reference existing HCL files +6. ✅ **Format flexibility** - Can add converters for other config formats + +## Risks & Mitigation + +| Risk | Probability | Impact | Mitigation | +|------|------------|--------|------------| +| Breaking existing imports | Low | High | Comprehensive backward compatibility tests | +| go-getter scheme detection wrong | Low | Medium | Test all documented schemes | +| Mock adapter generates invalid YAML | Low | Low | Validate generated YAML in tests | + +## Success Criteria + +### Phase 1 +- ✅ `isRemoteImport()` returns true for all go-getter schemes +- ✅ `mock://` adapter generates valid YAML +- ✅ Nested mock imports work (mock://nested) +- ✅ All existing tests pass +- ✅ 90%+ test coverage on new code + +## FAQ + +### Q: Why not replace go-getter with adapters for everything? + +**A:** go-getter is battle-tested and supports many schemes (git::, s3::, gcs::, oci://, etc.). We have significant investment in it. Import adapters are only for schemes that need special processing (content transformation, synthetic generation), not for standard downloads. + +### Q: Why was s3://, git::, etc. not working before? + +**A:** Bug in `isRemoteImport()` - it only checked for `http://` and `https://` prefixes, even though `downloadRemoteConfig()` uses go-getter which supports many more schemes. + +### Q: How does the mock adapter help testing? + +**A:** It generates synthetic YAML without external files or network. Tests can inject custom content via `MockData` field. Special paths like `mock://error` simulate failures. + +### Q: What's the routing order? + +**A:** +1. Check for import adapter (mock://, terragrunt://) +2. Check if go-getter scheme (http, https, git::, s3::, oci://, etc.) +3. Fall back to local filesystem + +## References + +- [Command Registry Pattern PRD](command-registry-pattern.md) +- [Component Registry Pattern PRD](component-registry-pattern.md) +- [Store Registry Implementation](../../pkg/store/registry.go) +- [Current imports.go](../../pkg/config/imports.go) +- [Terragrunt Migration Guide](../../website/docs/migration/terragrunt.mdx) + +## Changelog + +| Version | Date | Changes | +|---------|------|---------| +| 1.0 | 2025-12-18 | Initial PRD with mock adapter implementation | diff --git a/errors/errors.go b/errors/errors.go index f6ec94a483..67b6a2724b 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -572,6 +572,9 @@ var ( ErrNoFileMatchPattern = errors.New("no files matching patterns found") ErrMaxImportDepth = errors.New("maximum import depth reached") ErrNoValidAbsolutePaths = errors.New("no valid absolute paths found") + ErrDownloadRemoteConfig = errors.New("failed to download remote config") + ErrMockImportFailure = errors.New("mock error: simulated import failure") + ErrProcessNestedImports = errors.New("failed to process nested imports") // Profiler-related errors. ErrProfilerStart = errors.New("profiler start failed") diff --git a/examples/quick-start-advanced/Dockerfile b/examples/quick-start-advanced/Dockerfile index c55eebbb4e..1e1d89867d 100644 --- a/examples/quick-start-advanced/Dockerfile +++ b/examples/quick-start-advanced/Dockerfile @@ -6,7 +6,7 @@ ARG GEODESIC_OS=debian # https://atmos.tools/ # https://github.com/cloudposse/atmos # https://github.com/cloudposse/atmos/releases -ARG ATMOS_VERSION=1.203.0 +ARG ATMOS_VERSION=1.204.0 # Terraform: https://github.com/hashicorp/terraform/releases ARG TF_VERSION=1.5.7 diff --git a/pkg/config/adapters/adapters_test.go b/pkg/config/adapters/adapters_test.go new file mode 100644 index 0000000000..c21bea0673 --- /dev/null +++ b/pkg/config/adapters/adapters_test.go @@ -0,0 +1,639 @@ +package adapters + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + + errUtils "github.com/cloudposse/atmos/errors" + "github.com/cloudposse/atmos/pkg/config" +) + +// TestGoGetterAdapter_Schemes tests that GoGetterAdapter returns expected schemes. +func TestGoGetterAdapter_Schemes(t *testing.T) { + adapter := &GoGetterAdapter{} + schemes := adapter.Schemes() + + assert.Contains(t, schemes, "http://") + assert.Contains(t, schemes, "https://") + assert.Contains(t, schemes, "git::") + assert.Contains(t, schemes, "s3::") + assert.Contains(t, schemes, "oci://") +} + +// TestGoGetterAdapter_DownloadError tests error handling in GoGetterAdapter. +func TestGoGetterAdapter_DownloadError(t *testing.T) { + tempDir := t.TempDir() + + adapter := &GoGetterAdapter{} + ctx := context.Background() + + _, err := adapter.Resolve(ctx, "https://nonexistent.invalid/config.yaml", tempDir, tempDir, 1, 10) + assert.Error(t, err) // Download should fail +} + +// TestGoGetterAdapter_ReadConfigError tests viper read error handling. +func TestGoGetterAdapter_ReadConfigError(t *testing.T) { + tempDir := t.TempDir() + + // Create a local file with invalid YAML to simulate download success but read failure. + invalidYAML := `invalid: [yaml: content` + invalidPath := filepath.Join(tempDir, "invalid.yaml") + err := os.WriteFile(invalidPath, []byte(invalidYAML), 0o644) + assert.NoError(t, err) + + // Simulate what the adapter does with a malformed file. + v := viper.New() + v.SetConfigFile(invalidPath) + err = v.ReadInConfig() + assert.Error(t, err) // Should fail to read invalid YAML +} + +// TestLocalAdapter_Schemes tests that LocalAdapter returns nil (default fallback). +func TestLocalAdapter_Schemes(t *testing.T) { + adapter := &LocalAdapter{} + schemes := adapter.Schemes() + + assert.Nil(t, schemes) // LocalAdapter is the default fallback. +} + +// TestLocalAdapter_EmptyImportPath tests error path for empty import path. +func TestLocalAdapter_EmptyImportPath(t *testing.T) { + tempDir := t.TempDir() + + adapter := &LocalAdapter{} + ctx := context.Background() + + _, err := adapter.Resolve(ctx, "", tempDir, tempDir, 1, 10) + assert.Error(t, err) + assert.ErrorIs(t, err, errUtils.ErrImportPathRequired) +} + +// TestLocalAdapter_SearchConfigError tests error path for non-existent path. +func TestLocalAdapter_SearchConfigError(t *testing.T) { + tempDir := t.TempDir() + + nonExistentPath := filepath.Join(tempDir, "nonexistent", "config.yaml") + + adapter := &LocalAdapter{} + ctx := context.Background() + + _, err := adapter.Resolve(ctx, nonExistentPath, tempDir, tempDir, 1, 10) + assert.Error(t, err) + assert.ErrorIs(t, err, errUtils.ErrResolveLocal) +} + +// TestLocalAdapter_ReadConfigError tests error path for invalid YAML. +func TestLocalAdapter_ReadConfigError(t *testing.T) { + tempDir := t.TempDir() + + // Create invalid YAML file. + invalidContent := `invalid: [yaml: content` + invalidPath := filepath.Join(tempDir, "invalid.yaml") + err := os.WriteFile(invalidPath, []byte(invalidContent), 0o644) + assert.NoError(t, err) + + adapter := &LocalAdapter{} + ctx := context.Background() + + paths, err := adapter.Resolve(ctx, "invalid.yaml", tempDir, tempDir, 1, 10) + // Should not error but should skip files that fail to load. + assert.NoError(t, err) + _ = paths +} + +// TestLocalAdapter_ValidFile tests successful resolution of a valid file. +func TestLocalAdapter_ValidFile(t *testing.T) { + tempDir := t.TempDir() + + // Create a valid YAML file. + validContent := `settings: + key: value +` + validPath := filepath.Join(tempDir, "valid.yaml") + err := os.WriteFile(validPath, []byte(validContent), 0o644) + assert.NoError(t, err) + + adapter := &LocalAdapter{} + ctx := context.Background() + + paths, err := adapter.Resolve(ctx, "valid.yaml", tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.NotEmpty(t, paths) + assert.Equal(t, validPath, paths[0].FilePath) + assert.Equal(t, config.LOCAL, paths[0].ImportType) +} + +// TestMockAdapter_Schemes tests that MockAdapter returns the mock:// scheme. +func TestMockAdapter_Schemes(t *testing.T) { + adapter := &MockAdapter{} + schemes := adapter.Schemes() + + assert.Equal(t, []string{"mock://"}, schemes) +} + +// TestMockAdapter_Empty tests mock://empty path. +func TestMockAdapter_Empty(t *testing.T) { + tempDir := t.TempDir() + adapter := &MockAdapter{} + ctx := context.Background() + + paths, err := adapter.Resolve(ctx, "mock://empty", tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.Len(t, paths, 1) + assert.Equal(t, config.ADAPTER, paths[0].ImportType) +} + +// TestMockAdapter_Error tests mock://error path. +func TestMockAdapter_Error(t *testing.T) { + tempDir := t.TempDir() + adapter := &MockAdapter{} + ctx := context.Background() + + _, err := adapter.Resolve(ctx, "mock://error", tempDir, tempDir, 1, 10) + assert.Error(t, err) + assert.Contains(t, err.Error(), "mock error") +} + +// TestMockAdapter_CustomData tests custom mock data injection. +func TestMockAdapter_CustomData(t *testing.T) { + tempDir := t.TempDir() + adapter := &MockAdapter{ + MockData: map[string]string{ + "custom/path": "custom_key: custom_value\n", + }, + } + ctx := context.Background() + + paths, err := adapter.Resolve(ctx, "mock://custom/path", tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.Len(t, paths, 1) + + // Verify the file contains the custom content. + content, err := os.ReadFile(paths[0].FilePath) + assert.NoError(t, err) + assert.Equal(t, "custom_key: custom_value\n", string(content)) +} + +// TestMockAdapter_DefaultPath tests default mock path handling. +func TestMockAdapter_DefaultPath(t *testing.T) { + tempDir := t.TempDir() + adapter := &MockAdapter{} + ctx := context.Background() + + paths, err := adapter.Resolve(ctx, "mock://some/path", tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.Len(t, paths, 1) + + // Verify the file contains default mock config. + content, err := os.ReadFile(paths[0].FilePath) + assert.NoError(t, err) + assert.Contains(t, string(content), "mock_path: \"some/path\"") +} + +// TestFindImportAdapter tests adapter registry routing. +func TestFindImportAdapter(t *testing.T) { + tests := []struct { + name string + path string + isLocalFallback bool + }{ + {"http URL", "http://example.com/config.yaml", false}, + {"https URL", "https://example.com/config.yaml", false}, + {"git URL", "git::https://github.com/user/repo.git", false}, + {"s3 URL", "s3://bucket/path/config.yaml", false}, + {"mock URL", "mock://test", false}, + {"local path", "local/path/config.yaml", true}, + {"absolute path", "/absolute/path/config.yaml", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + adapter := config.FindImportAdapter(tt.path) + assert.NotNil(t, adapter) + if tt.isLocalFallback { + // Local paths should get LocalAdapter (default fallback). + schemes := adapter.Schemes() + assert.Nil(t, schemes) // LocalAdapter returns nil for schemes. + } + }) + } +} + +// TestGlobalMockAdapter tests the global mock adapter singleton. +func TestGlobalMockAdapter(t *testing.T) { + // Clear any existing mock data. + ClearMockData() + + // Set new mock data. + SetMockData(map[string]string{ + "test/path": "test: data\n", + }) + + adapter := GetGlobalMockAdapter() + assert.NotNil(t, adapter) + assert.Equal(t, "test: data\n", adapter.MockData["test/path"]) + + // Clear and verify. + ClearMockData() + assert.Empty(t, adapter.MockData) +} + +// TestLocalAdapter_AbsolutePath tests resolution of absolute paths. +func TestLocalAdapter_AbsolutePath(t *testing.T) { + tempDir := t.TempDir() + + // Create a valid YAML file. + validContent := `settings: + key: value +` + validPath := filepath.Join(tempDir, "absolute.yaml") + err := os.WriteFile(validPath, []byte(validContent), 0o644) + assert.NoError(t, err) + + adapter := &LocalAdapter{} + ctx := context.Background() + + // Use the absolute path directly. + paths, err := adapter.Resolve(ctx, validPath, tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.NotEmpty(t, paths) + assert.Equal(t, validPath, paths[0].FilePath) +} + +// TestLocalAdapter_OutsideBasePath tests paths outside the base directory. +func TestLocalAdapter_OutsideBasePath(t *testing.T) { + parentDir := t.TempDir() + subDir := filepath.Join(parentDir, "subdir") + err := os.Mkdir(subDir, 0o755) + assert.NoError(t, err) + + // Create a config in parent directory. + parentConfig := filepath.Join(parentDir, "parent.yaml") + err = os.WriteFile(parentConfig, []byte("key: value\n"), 0o644) + assert.NoError(t, err) + + adapter := &LocalAdapter{} + ctx := context.Background() + + // Import from subdir pointing to parent. + paths, err := adapter.Resolve(ctx, "../parent.yaml", subDir, parentDir, 1, 10) + assert.NoError(t, err) + assert.NotEmpty(t, paths) + assert.Equal(t, parentConfig, paths[0].FilePath) +} + +// TestLocalAdapter_NestedImports tests processing of nested imports. +func TestLocalAdapter_NestedImports(t *testing.T) { + tempDir := t.TempDir() + + // Create nested config file. + nestedContent := `settings: + nested: true +` + nestedPath := filepath.Join(tempDir, "nested.yaml") + err := os.WriteFile(nestedPath, []byte(nestedContent), 0o644) + assert.NoError(t, err) + + // Create main config with import. + mainContent := `import: + - nested.yaml +settings: + main: true +` + mainPath := filepath.Join(tempDir, "main.yaml") + err = os.WriteFile(mainPath, []byte(mainContent), 0o644) + assert.NoError(t, err) + + // Setup test adapters to avoid circular import. + config.ResetImportAdapterRegistry() + config.SetDefaultAdapter(&LocalAdapter{}) + + adapter := &LocalAdapter{} + ctx := context.Background() + + paths, err := adapter.Resolve(ctx, "main.yaml", tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.GreaterOrEqual(t, len(paths), 1) // At least the main file. +} + +// TestLocalAdapter_NestedImportsWithCustomBasePath tests nested imports with custom base_path. +func TestLocalAdapter_NestedImportsWithCustomBasePath(t *testing.T) { + tempDir := t.TempDir() + subDir := filepath.Join(tempDir, "configs") + err := os.Mkdir(subDir, 0o755) + assert.NoError(t, err) + + // Create nested config in subdirectory. + nestedContent := `settings: + nested: true +` + nestedPath := filepath.Join(subDir, "nested.yaml") + err = os.WriteFile(nestedPath, []byte(nestedContent), 0o644) + assert.NoError(t, err) + + // Create main config with import and custom base_path. + mainContent := `base_path: ./configs +import: + - nested.yaml +settings: + main: true +` + mainPath := filepath.Join(tempDir, "main.yaml") + err = os.WriteFile(mainPath, []byte(mainContent), 0o644) + assert.NoError(t, err) + + // Setup test adapters. + config.ResetImportAdapterRegistry() + config.SetDefaultAdapter(&LocalAdapter{}) + + adapter := &LocalAdapter{} + ctx := context.Background() + + paths, err := adapter.Resolve(ctx, "main.yaml", tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.NotEmpty(t, paths) +} + +// TestLocalAdapter_NoNestedImports tests file without nested imports. +func TestLocalAdapter_NoNestedImports(t *testing.T) { + tempDir := t.TempDir() + + // Create config without imports. + content := `settings: + key: value +` + configPath := filepath.Join(tempDir, "config.yaml") + err := os.WriteFile(configPath, []byte(content), 0o644) + assert.NoError(t, err) + + config.ResetImportAdapterRegistry() + config.SetDefaultAdapter(&LocalAdapter{}) + + adapter := &LocalAdapter{} + ctx := context.Background() + + paths, err := adapter.Resolve(ctx, "config.yaml", tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.Len(t, paths, 1) +} + +// TestGoGetterAdapter_AllSchemes tests that all expected schemes are present. +func TestGoGetterAdapter_AllSchemes(t *testing.T) { + adapter := &GoGetterAdapter{} + schemes := adapter.Schemes() + + expectedSchemes := []string{ + "http://", "https://", + "git::", "git@", + "s3::", "s3://", + "gcs::", "gcs://", + "oci://", + "file://", + "hg::", + "scp://", "sftp://", + "github.com/", "bitbucket.org/", + } + + for _, expected := range expectedSchemes { + assert.Contains(t, schemes, expected, "Should contain scheme: %s", expected) + } +} + +// TestDownloadRemoteConfig_NilContext tests downloadRemoteConfig with nil context. +func TestDownloadRemoteConfig_NilContext(t *testing.T) { + tempDir := t.TempDir() + + // Test with TODO context - should work with default timeout. + // Use an invalid URL to trigger error (we're testing the context handling). + _, err := downloadRemoteConfig(context.TODO(), "https://nonexistent.invalid/config.yaml", tempDir) + assert.Error(t, err) // Should fail but not panic. +} + +// TestDownloadRemoteConfig_ValidContext tests downloadRemoteConfig with valid context. +func TestDownloadRemoteConfig_ValidContext(t *testing.T) { + tempDir := t.TempDir() + ctx := context.Background() + + // Use an invalid URL to trigger error. + _, err := downloadRemoteConfig(ctx, "https://nonexistent.invalid/config.yaml", tempDir) + assert.Error(t, err) +} + +// TestMockAdapter_NestedImports tests mock adapter with nested imports. +func TestMockAdapter_NestedImports(t *testing.T) { + tempDir := t.TempDir() + + // Setup adapters. + config.ResetImportAdapterRegistry() + mockAdapter := &MockAdapter{ + MockData: map[string]string{ + "parent": `import: + - mock://child +settings: + parent: true +`, + "child": `settings: + child: true +`, + }, + } + config.RegisterImportAdapter(mockAdapter) + config.SetDefaultAdapter(&LocalAdapter{}) + + ctx := context.Background() + + paths, err := mockAdapter.Resolve(ctx, "mock://parent", tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.NotEmpty(t, paths) +} + +// TestMockAdapter_WriteError tests mock adapter when temp directory doesn't exist. +func TestMockAdapter_WriteError(t *testing.T) { + adapter := &MockAdapter{} + ctx := context.Background() + + // Use non-existent temp directory. + _, err := adapter.Resolve(ctx, "mock://test", "/nonexistent/temp/dir", "/base", 1, 10) + assert.Error(t, err) +} + +// TestGoGetterAdapter_ResolveWithLocalFile tests Resolve with a valid local file. +func TestGoGetterAdapter_ResolveWithLocalFile(t *testing.T) { + tempDir := t.TempDir() + + // Create a valid local YAML config file. + validContent := `settings: + key: value +` + validPath := filepath.Join(tempDir, "config.yaml") + err := os.WriteFile(validPath, []byte(validContent), 0o644) + assert.NoError(t, err) + + // Setup adapters. + config.ResetImportAdapterRegistry() + config.SetDefaultAdapter(&LocalAdapter{}) + + adapter := &GoGetterAdapter{} + ctx := context.Background() + + // Use file:// URL to download local file. + fileURL := "file://" + validPath + paths, err := adapter.Resolve(ctx, fileURL, tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.NotEmpty(t, paths) + assert.Equal(t, config.REMOTE, paths[0].ImportType) +} + +// TestGoGetterAdapter_ResolveWithInvalidYAML tests Resolve with invalid YAML content. +func TestGoGetterAdapter_ResolveWithInvalidYAML(t *testing.T) { + tempDir := t.TempDir() + + // Create an invalid YAML config file. + invalidContent := `invalid: [yaml: content` + invalidPath := filepath.Join(tempDir, "invalid.yaml") + err := os.WriteFile(invalidPath, []byte(invalidContent), 0o644) + assert.NoError(t, err) + + adapter := &GoGetterAdapter{} + ctx := context.Background() + + // Use file:// URL to download the invalid file. + fileURL := "file://" + invalidPath + _, err = adapter.Resolve(ctx, fileURL, tempDir, tempDir, 1, 10) + assert.Error(t, err) // Should fail when reading invalid YAML. +} + +// TestGoGetterAdapter_ResolveWithNestedImports tests Resolve with nested imports. +func TestGoGetterAdapter_ResolveWithNestedImports(t *testing.T) { + tempDir := t.TempDir() + + // Create nested config file. + nestedContent := `settings: + nested: true +` + nestedPath := filepath.Join(tempDir, "nested.yaml") + err := os.WriteFile(nestedPath, []byte(nestedContent), 0o644) + assert.NoError(t, err) + + // Create parent config with import. + parentContent := `import: + - nested.yaml +settings: + parent: true +` + parentPath := filepath.Join(tempDir, "parent.yaml") + err = os.WriteFile(parentPath, []byte(parentContent), 0o644) + assert.NoError(t, err) + + // Setup adapters. + config.ResetImportAdapterRegistry() + config.SetDefaultAdapter(&LocalAdapter{}) + + adapter := &GoGetterAdapter{} + ctx := context.Background() + + // Use file:// URL to download parent file. + fileURL := "file://" + parentPath + paths, err := adapter.Resolve(ctx, fileURL, tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.GreaterOrEqual(t, len(paths), 1) // At least the parent file. +} + +// TestGoGetterAdapter_ResolveWithCustomBasePath tests Resolve with custom base_path. +func TestGoGetterAdapter_ResolveWithCustomBasePath(t *testing.T) { + tempDir := t.TempDir() + subDir := filepath.Join(tempDir, "subdir") + err := os.Mkdir(subDir, 0o755) + assert.NoError(t, err) + + // Create nested config in subdirectory. + nestedContent := `settings: + nested: true +` + nestedPath := filepath.Join(subDir, "nested.yaml") + err = os.WriteFile(nestedPath, []byte(nestedContent), 0o644) + assert.NoError(t, err) + + // Create parent config with custom base_path. + parentContent := `base_path: ` + subDir + ` +import: + - nested.yaml +settings: + parent: true +` + parentPath := filepath.Join(tempDir, "parent.yaml") + err = os.WriteFile(parentPath, []byte(parentContent), 0o644) + assert.NoError(t, err) + + // Setup adapters. + config.ResetImportAdapterRegistry() + config.SetDefaultAdapter(&LocalAdapter{}) + + adapter := &GoGetterAdapter{} + ctx := context.Background() + + // Use file:// URL to download parent file. + fileURL := "file://" + parentPath + paths, err := adapter.Resolve(ctx, fileURL, tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.NotEmpty(t, paths) +} + +// TestGoGetterAdapter_ResolveNoImports tests Resolve with config that has no imports. +func TestGoGetterAdapter_ResolveNoImports(t *testing.T) { + tempDir := t.TempDir() + + // Create a config without imports. + content := `settings: + key: value +components: + terraform: + vpc: {} +` + configPath := filepath.Join(tempDir, "no-imports.yaml") + err := os.WriteFile(configPath, []byte(content), 0o644) + assert.NoError(t, err) + + // Setup adapters. + config.ResetImportAdapterRegistry() + config.SetDefaultAdapter(&LocalAdapter{}) + + adapter := &GoGetterAdapter{} + ctx := context.Background() + + fileURL := "file://" + configPath + paths, err := adapter.Resolve(ctx, fileURL, tempDir, tempDir, 1, 10) + assert.NoError(t, err) + assert.Len(t, paths, 1) // Only the downloaded file. + assert.Equal(t, config.REMOTE, paths[0].ImportType) +} + +// TestDownloadRemoteConfig_LocalFile tests downloading a local file via file:// URL. +func TestDownloadRemoteConfig_LocalFile(t *testing.T) { + tempDir := t.TempDir() + destDir := t.TempDir() + + // Create a source file. + content := `key: value` + sourcePath := filepath.Join(tempDir, "source.yaml") + err := os.WriteFile(sourcePath, []byte(content), 0o644) + assert.NoError(t, err) + + ctx := context.Background() + fileURL := "file://" + sourcePath + + // Download the file. + resultPath, err := downloadRemoteConfig(ctx, fileURL, destDir) + assert.NoError(t, err) + assert.NotEmpty(t, resultPath) + + // Verify content was downloaded. + downloadedContent, err := os.ReadFile(resultPath) + assert.NoError(t, err) + assert.Equal(t, content, string(downloadedContent)) +} diff --git a/pkg/config/adapters/gogetter_adapter.go b/pkg/config/adapters/gogetter_adapter.go new file mode 100644 index 0000000000..3c9414feb9 --- /dev/null +++ b/pkg/config/adapters/gogetter_adapter.go @@ -0,0 +1,134 @@ +package adapters + +import ( + "context" + "fmt" + "path/filepath" + "time" + + "github.com/hashicorp/go-getter" + "github.com/spf13/viper" + + errUtils "github.com/cloudposse/atmos/errors" + "github.com/cloudposse/atmos/pkg/config" + log "github.com/cloudposse/atmos/pkg/logger" + "github.com/cloudposse/atmos/pkg/perf" +) + +// GoGetterAdapter handles remote imports using HashiCorp's go-getter. +// It supports a wide variety of URL schemes for downloading remote configurations. +type GoGetterAdapter struct{} + +// Schemes returns all URL schemes/prefixes handled by go-getter. +func (g *GoGetterAdapter) Schemes() []string { + return []string{ + // HTTP/HTTPS. + "http://", + "https://", + // Git. + "git::", + "git@", + // Cloud storage. + "s3::", + "s3://", + "gcs::", + "gcs://", + // OCI registries. + "oci://", + // Local file URLs. + "file://", + // Mercurial. + "hg::", + // SSH-based protocols. + "scp://", + "sftp://", + // Shorthand for popular hosts. + "github.com/", + "bitbucket.org/", + } +} + +// Resolve downloads a remote configuration and processes any nested imports. +// +//nolint:revive // argument-limit: matches ImportAdapter interface signature. +func (g *GoGetterAdapter) Resolve( + ctx context.Context, + importPath string, + basePath string, + tempDir string, + currentDepth int, + maxDepth int, +) ([]config.ResolvedPaths, error) { + defer perf.Track(nil, "adapters.GoGetterAdapter.Resolve")() + + // Download the remote configuration. + tempFile, err := downloadRemoteConfig(ctx, importPath, tempDir) + if err != nil { + log.Debug("failed to download remote config", "path", importPath, "error", err) + return nil, err + } + + // Read the downloaded configuration. + v := viper.New() + v.SetConfigFile(tempFile) + err = v.ReadInConfig() + if err != nil { + log.Debug("failed to read remote config", "path", importPath, "error", err) + return nil, fmt.Errorf(errUtils.ErrWrapFormat, errUtils.ErrReadConfig, err) + } + + resolvedPaths := make([]config.ResolvedPaths, 0) + resolvedPaths = append(resolvedPaths, config.ResolvedPaths{ + FilePath: tempFile, + ImportPaths: importPath, + ImportType: config.REMOTE, + }) + + // Process nested imports from the remote file. + imports := v.GetStringSlice("import") + importBasePath := v.GetString("base_path") + if importBasePath == "" { + importBasePath = basePath + } + + if len(imports) > 0 { + nestedPaths, err := config.ProcessImportsFromAdapter(importBasePath, imports, tempDir, currentDepth+1, maxDepth) + if err != nil { + log.Debug("failed to process nested imports", "import", importPath, "err", err) + return nil, fmt.Errorf(errUtils.ErrWrapFormat, errUtils.ErrProcessNestedImports, err) + } + resolvedPaths = append(resolvedPaths, nestedPaths...) + } + + return resolvedPaths, nil +} + +// downloadRemoteConfig downloads a configuration file from a remote URL. +func downloadRemoteConfig(ctx context.Context, url string, tempDir string) (string, error) { + defer perf.Track(nil, "adapters.downloadRemoteConfig")() + + // Generate unique filename for the temp file. + fileName := fmt.Sprintf("atmos-import-%d.yaml", time.Now().UnixNano()) + tempFile := filepath.Join(tempDir, fileName) + + // Use context with timeout if none provided. + if ctx == nil { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + } + + client := &getter.Client{ + Ctx: ctx, + Src: url, + Dst: tempFile, + Mode: getter.ClientModeFile, + } + + err := client.Get() + if err != nil { + return "", fmt.Errorf(errUtils.ErrWrapFormat, errUtils.ErrDownloadRemoteConfig, err) + } + + return tempFile, nil +} diff --git a/pkg/config/adapters/init.go b/pkg/config/adapters/init.go new file mode 100644 index 0000000000..94a1c677fc --- /dev/null +++ b/pkg/config/adapters/init.go @@ -0,0 +1,24 @@ +// Package adapters contains import adapter implementations for the config package. +// Import this package to register all built-in adapters. +package adapters + +import ( + "github.com/cloudposse/atmos/pkg/config" +) + +func init() { + // Set the function pointer to register adapters lazily. + config.SetBuiltinAdaptersInitializer(registerBuiltinAdapters) +} + +// registerBuiltinAdapters registers all built-in import adapters. +func registerBuiltinAdapters() { + // Register GoGetterAdapter for remote imports. + config.RegisterImportAdapter(&GoGetterAdapter{}) + + // Register MockAdapter for testing (using the global singleton). + config.RegisterImportAdapter(GetGlobalMockAdapter()) + + // Register LocalAdapter as the default fallback. + config.SetDefaultAdapter(&LocalAdapter{}) +} diff --git a/pkg/config/adapters/local_adapter.go b/pkg/config/adapters/local_adapter.go new file mode 100644 index 0000000000..2c3c500785 --- /dev/null +++ b/pkg/config/adapters/local_adapter.go @@ -0,0 +1,103 @@ +package adapters + +import ( + "context" + "path/filepath" + "strings" + + "github.com/spf13/viper" + + errUtils "github.com/cloudposse/atmos/errors" + "github.com/cloudposse/atmos/pkg/config" + log "github.com/cloudposse/atmos/pkg/logger" + "github.com/cloudposse/atmos/pkg/perf" +) + +// LocalAdapter handles local filesystem imports. +// It is registered as the default adapter and handles paths without URL schemes. +type LocalAdapter struct{} + +// Schemes returns nil because LocalAdapter is the default fallback. +// It handles any path that doesn't match other adapters' schemes. +func (l *LocalAdapter) Schemes() []string { + return nil +} + +// Resolve processes a local filesystem import path. +// +//nolint:revive // argument-limit: matches ImportAdapter interface signature. +func (l *LocalAdapter) Resolve( + _ context.Context, importPath, basePath, tempDir string, currentDepth, maxDepth int, +) ([]config.ResolvedPaths, error) { + defer perf.Track(nil, "adapters.LocalAdapter.Resolve")() + + if importPath == "" { + return nil, errUtils.ErrImportPathRequired + } + + resolvedPath := l.resolveImportPath(importPath, basePath) + paths, err := config.SearchAtmosConfig(resolvedPath) + if err != nil { + log.Debug("failed to resolve local import path", "path", importPath, "err", err) + return nil, errUtils.ErrResolveLocal + } + + resolvedPaths := make([]config.ResolvedPaths, 0) + for _, path := range paths { + v := viper.New() + v.SetConfigFile(path) + v.SetConfigType("yaml") + if err := v.ReadInConfig(); err != nil { + log.Debug("failed to load local config", "path", path, "error", err) + continue + } + + resolvedPaths = append(resolvedPaths, config.ResolvedPaths{ + FilePath: path, ImportPaths: importPath, ImportType: config.LOCAL, + }) + + nestedPaths := l.processNestedImports(v, nestedImportParams{ + basePath: basePath, tempDir: tempDir, currentDepth: currentDepth, maxDepth: maxDepth, path: path, + }) + resolvedPaths = append(resolvedPaths, nestedPaths...) + } + + return resolvedPaths, nil +} + +// resolveImportPath converts a relative import path to absolute. +func (l *LocalAdapter) resolveImportPath(importPath, basePath string) string { + resolvedPath := importPath + if !filepath.IsAbs(importPath) { + resolvedPath = filepath.Join(basePath, importPath) + } + if !strings.HasPrefix(filepath.Clean(resolvedPath), filepath.Clean(basePath)) { + log.Trace("Import path is outside of base directory", "importPath", resolvedPath, "basePath", basePath) + } + return resolvedPath +} + +// nestedImportParams holds parameters for processing nested imports. +type nestedImportParams struct { + basePath, tempDir string + currentDepth, maxDepth int + path string +} + +// processNestedImports handles nested import statements in a config file. +func (l *LocalAdapter) processNestedImports(v *viper.Viper, p nestedImportParams) []config.ResolvedPaths { + imports := v.GetStringSlice("import") + if len(imports) == 0 { + return nil + } + importBasePath := v.GetString("base_path") + if importBasePath == "" { + importBasePath = p.basePath + } + nestedPaths, err := config.ProcessImportsFromAdapter(importBasePath, imports, p.tempDir, p.currentDepth+1, p.maxDepth) + if err != nil { + log.Debug("failed to process nested imports from", "path", p.path, "error", err) + return nil + } + return nestedPaths +} diff --git a/pkg/config/adapters/mock_adapter.go b/pkg/config/adapters/mock_adapter.go new file mode 100644 index 0000000000..558dcf110c --- /dev/null +++ b/pkg/config/adapters/mock_adapter.go @@ -0,0 +1,140 @@ +package adapters + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + errUtils "github.com/cloudposse/atmos/errors" + "github.com/cloudposse/atmos/pkg/config" + "github.com/cloudposse/atmos/pkg/perf" +) + +// mockConfigFilePerms is the file permission for mock config files. +const mockConfigFilePerms = 0o600 + +// MockAdapter generates synthetic YAML for testing. +// This enables unit tests without external dependencies. +// +// Usage: +// - mock://empty → Empty YAML config +// - mock://error → Returns error (test error handling) +// - mock://nested → Config with nested mock:// imports +// - mock://custom/path → Config with mock_path set to "custom/path" +// +// Tests can inject custom data via MockData field. +type MockAdapter struct { + // MockData allows tests to inject custom YAML content. + // Key is the path after "mock://", value is YAML content. + MockData map[string]string +} + +// globalMockAdapter is the singleton instance used for registration. +// Tests can replace MockData via SetMockData(). +var globalMockAdapter = &MockAdapter{ + MockData: make(map[string]string), +} + +// GetGlobalMockAdapter returns the global mock adapter instance. +// This is used by the init.go to register the singleton. +func GetGlobalMockAdapter() *MockAdapter { + return globalMockAdapter +} + +// SetMockData sets the mock data for the global mock adapter. +// This is intended for testing. +func SetMockData(data map[string]string) { + globalMockAdapter.MockData = data +} + +// ClearMockData clears all mock data. +// This is intended for testing cleanup. +func ClearMockData() { + globalMockAdapter.MockData = make(map[string]string) +} + +// Schemes returns the mock:// scheme. +func (m *MockAdapter) Schemes() []string { + return []string{"mock://"} +} + +// Resolve generates synthetic YAML based on the mock path. +// +//nolint:revive // argument-limit: matches ImportAdapter interface signature. +func (m *MockAdapter) Resolve( + ctx context.Context, + importPath string, + basePath string, + tempDir string, + currentDepth int, + maxDepth int, +) ([]config.ResolvedPaths, error) { + defer perf.Track(nil, "adapters.MockAdapter.Resolve")() + + mockPath := strings.TrimPrefix(importPath, "mock://") + + switch mockPath { + case "error": + return nil, errUtils.ErrMockImportFailure + + case "empty": + return m.writeConfig(importPath, tempDir, "# Empty mock configuration\n") + + case "nested": + content := `# Nested mock configuration +import: + - mock://component/base + +vars: + nested: true + level: 1 +` + paths, err := m.writeConfig(importPath, tempDir, content) + if err != nil { + return nil, err + } + + // Process nested imports. + nestedPaths, err := config.ProcessImportsFromAdapter(basePath, []string{"mock://component/base"}, tempDir, currentDepth+1, maxDepth) + if err != nil { + return paths, nil // Return what we have, log error. + } + return append(paths, nestedPaths...), nil + + default: + // Check for injected mock data. + if m.MockData != nil { + if content, ok := m.MockData[mockPath]; ok { + return m.writeConfig(importPath, tempDir, content) + } + } + + // Generate default mock config. + content := fmt.Sprintf(`# Mock configuration for %s +vars: + mock_path: "%s" + mock_source: "mock_adapter" +`, mockPath, mockPath) + return m.writeConfig(importPath, tempDir, content) + } +} + +func (m *MockAdapter) writeConfig(importPath, tempDir, content string) ([]config.ResolvedPaths, error) { + fileName := fmt.Sprintf("mock-import-%d.yaml", time.Now().UnixNano()) + filePath := filepath.Join(tempDir, fileName) + + if err := os.WriteFile(filePath, []byte(content), mockConfigFilePerms); err != nil { + return nil, fmt.Errorf("failed to write mock config: %w", err) + } + + return []config.ResolvedPaths{ + { + FilePath: filePath, + ImportPaths: importPath, + ImportType: config.ADAPTER, + }, + }, nil +} diff --git a/pkg/config/command_merge_core_test.go b/pkg/config/command_merge_core_test.go index 3bf8fcb9af..3f84c5ba78 100644 --- a/pkg/config/command_merge_core_test.go +++ b/pkg/config/command_merge_core_test.go @@ -15,6 +15,8 @@ import ( // ensuring that commands from imported configurations are properly merged // with local commands, and that local commands can override imported ones. func TestCommandMergeCore(t *testing.T) { + setupTestAdapters() + tests := []struct { name string setupFiles map[string]string diff --git a/pkg/config/command_merging_behavior_test.go b/pkg/config/command_merging_behavior_test.go index 6a2cf2c372..a27daa9d5d 100644 --- a/pkg/config/command_merging_behavior_test.go +++ b/pkg/config/command_merging_behavior_test.go @@ -17,6 +17,8 @@ import ( // 2. Local config can add more commands. // 3. If local has a duplicate name, local should win. func TestCommandMergingBehavior(t *testing.T) { + setupTestAdapters() + tests := []struct { name string setupFiles map[string]string @@ -332,6 +334,8 @@ commands: // TestCommandStructurePreservation verifies that complex command structures are preserved through merging. func TestCommandStructurePreservation(t *testing.T) { + setupTestAdapters() + // Create temp directory. tempDir := t.TempDir() diff --git a/pkg/config/import_adapter.go b/pkg/config/import_adapter.go new file mode 100644 index 0000000000..080cfb512b --- /dev/null +++ b/pkg/config/import_adapter.go @@ -0,0 +1,50 @@ +package config + +import ( + "context" +) + +// ImportAdapter handles import resolution for specific URL schemes. +// All import handling goes through the adapter registry - there are no special cases. +// +// Built-in adapters: +// - GoGetterAdapter: http://, https://, git::, s3::, oci://, github.com/, etc. +// - LocalAdapter: Local filesystem paths (default fallback) +// - MockAdapter: mock:// scheme for testing +// +// Future adapters: +// - TerragruntAdapter: terragrunt:// for HCL→YAML transformation +type ImportAdapter interface { + // Schemes returns the URL schemes/prefixes this adapter handles. + // Examples: []string{"http://", "https://", "git::", "github.com/"} + // + // Return nil or empty slice for the default adapter (LocalAdapter), + // which handles paths without recognized schemes. + Schemes() []string + + // Resolve processes an import path and returns resolved file paths. + // + // Parameters: + // - ctx: Context for cancellation and deadlines + // - importPath: The full import path (e.g., "git::https://github.com/org/repo//path") + // - basePath: The base path for resolving relative references + // - tempDir: Temporary directory for downloaded/generated files + // - currentDepth: Current recursion depth for nested imports + // - maxDepth: Maximum allowed recursion depth + // + // Returns: + // - []ResolvedPaths: List of resolved file paths to merge + // - error: Any error encountered during resolution + // + // Adapters are responsible for handling nested imports by calling + // processImports() recursively when the resolved config contains + // further import statements. + Resolve( + ctx context.Context, + importPath string, + basePath string, + tempDir string, + currentDepth int, + maxDepth int, + ) ([]ResolvedPaths, error) +} diff --git a/pkg/config/import_adapter_registry.go b/pkg/config/import_adapter_registry.go new file mode 100644 index 0000000000..d20fb837b0 --- /dev/null +++ b/pkg/config/import_adapter_registry.go @@ -0,0 +1,167 @@ +package config + +import ( + "context" + "strings" + "sync" + + "github.com/cloudposse/atmos/pkg/perf" +) + +var ( + importAdapterRegistry = &ImportAdapterRegistry{ + adapters: make([]ImportAdapter, 0), + } + initAdaptersOnce sync.Once +) + +// ImportAdapterRegistry manages import adapter registration and lookup. +// Thread-safe for concurrent access. +type ImportAdapterRegistry struct { + mu sync.RWMutex + adapters []ImportAdapter // Ordered list for prefix matching. + defaultAdapter ImportAdapter // Fallback adapter (LocalAdapter). +} + +// RegisterImportAdapter adds an import adapter to the registry. +// Adapters are matched in registration order, so register more specific +// schemes before less specific ones. +// +// Call this from init() functions to self-register adapters. +func RegisterImportAdapter(adapter ImportAdapter) { + defer perf.Track(nil, "config.RegisterImportAdapter")() + + if adapter == nil { + return + } + + importAdapterRegistry.mu.Lock() + defer importAdapterRegistry.mu.Unlock() + + // Check if this is the default adapter (no schemes). + schemes := adapter.Schemes() + if len(schemes) == 0 { + importAdapterRegistry.defaultAdapter = adapter + return + } + + importAdapterRegistry.adapters = append(importAdapterRegistry.adapters, adapter) +} + +// initBuiltinAdapters is called by initAdaptersOnce to register built-in adapters. +// This function pointer is set by the adapters package to avoid circular imports. +var initBuiltinAdapters func() + +// SetBuiltinAdaptersInitializer sets the function used to initialize built-in adapters. +// This should be called from the adapters package's init() function. +func SetBuiltinAdaptersInitializer(f func()) { + initBuiltinAdapters = f +} + +// EnsureAdaptersInitialized ensures all built-in adapters are registered. +// This is called automatically by FindImportAdapter. +func EnsureAdaptersInitialized() { + initAdaptersOnce.Do(func() { + if initBuiltinAdapters != nil { + initBuiltinAdapters() + } + }) +} + +// FindImportAdapter returns the appropriate adapter for the given import path. +// It checks registered adapters' schemes in order and returns the first match. +// If no adapter matches, returns the default adapter (LocalAdapter). +// +// This function always returns an adapter - never nil. +func FindImportAdapter(importPath string) ImportAdapter { + defer perf.Track(nil, "config.FindImportAdapter")() + + // Ensure adapters are initialized on first use. + EnsureAdaptersInitialized() + + importAdapterRegistry.mu.RLock() + defer importAdapterRegistry.mu.RUnlock() + + loweredPath := strings.ToLower(importPath) + + // Check each registered adapter's schemes. + for _, adapter := range importAdapterRegistry.adapters { + for _, scheme := range adapter.Schemes() { + if strings.HasPrefix(loweredPath, strings.ToLower(scheme)) { + return adapter + } + } + } + + // Return default adapter if no scheme matched. + if importAdapterRegistry.defaultAdapter != nil { + return importAdapterRegistry.defaultAdapter + } + + // Fallback: return a nil-safe adapter that does nothing. + // This should never happen in practice as LocalAdapter should be registered. + return &noopAdapter{} +} + +// SetDefaultAdapter sets the fallback adapter for paths without recognized schemes. +// This is typically the LocalAdapter for filesystem paths. +func SetDefaultAdapter(adapter ImportAdapter) { + defer perf.Track(nil, "config.SetDefaultAdapter")() + + importAdapterRegistry.mu.Lock() + defer importAdapterRegistry.mu.Unlock() + + importAdapterRegistry.defaultAdapter = adapter +} + +// ResetImportAdapterRegistry clears all registered adapters. +// This is intended for testing only. +func ResetImportAdapterRegistry() { + importAdapterRegistry.mu.Lock() + defer importAdapterRegistry.mu.Unlock() + + importAdapterRegistry.adapters = make([]ImportAdapter, 0) + importAdapterRegistry.defaultAdapter = nil + // Reset the sync.Once to allow reinitialization. + initAdaptersOnce = sync.Once{} +} + +// GetRegisteredAdapters returns a copy of all registered adapters. +// This is intended for testing and debugging. +func GetRegisteredAdapters() []ImportAdapter { + importAdapterRegistry.mu.RLock() + defer importAdapterRegistry.mu.RUnlock() + + result := make([]ImportAdapter, len(importAdapterRegistry.adapters)) + copy(result, importAdapterRegistry.adapters) + return result +} + +// GetDefaultAdapter returns the current default adapter. +// This is intended for testing and debugging. +func GetDefaultAdapter() ImportAdapter { + importAdapterRegistry.mu.RLock() + defer importAdapterRegistry.mu.RUnlock() + + return importAdapterRegistry.defaultAdapter +} + +// noopAdapter is a fallback adapter that does nothing. +// It's used when no default adapter is registered (should never happen in practice). +type noopAdapter struct{} + +func (n *noopAdapter) Schemes() []string { + return nil +} + +//nolint:revive // argument-limit: matches ImportAdapter interface signature. +func (n *noopAdapter) Resolve( + _ context.Context, + _ string, + _ string, + _ string, + _ int, + _ int, +) ([]ResolvedPaths, error) { + return nil, nil +} diff --git a/pkg/config/import_adapter_registry_test.go b/pkg/config/import_adapter_registry_test.go new file mode 100644 index 0000000000..a476922332 --- /dev/null +++ b/pkg/config/import_adapter_registry_test.go @@ -0,0 +1,242 @@ +package config + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +// mockTestAdapter is a simple mock adapter for testing the registry. +type mockTestAdapter struct { + schemes []string +} + +func (m *mockTestAdapter) Schemes() []string { + return m.schemes +} + +func (m *mockTestAdapter) Resolve( + _ context.Context, + _ string, + _ string, + _ string, + _ int, + _ int, +) ([]ResolvedPaths, error) { + return []ResolvedPaths{{FilePath: "mock", ImportType: ADAPTER}}, nil +} + +func TestRegisterImportAdapter_NilAdapter(t *testing.T) { + ResetImportAdapterRegistry() + + // Should not panic and should be a no-op. + RegisterImportAdapter(nil) + + adapters := GetRegisteredAdapters() + assert.Empty(t, adapters) +} + +func TestRegisterImportAdapter_WithSchemes(t *testing.T) { + ResetImportAdapterRegistry() + + adapter := &mockTestAdapter{schemes: []string{"test://"}} + RegisterImportAdapter(adapter) + + adapters := GetRegisteredAdapters() + assert.Len(t, adapters, 1) + assert.Equal(t, []string{"test://"}, adapters[0].Schemes()) +} + +func TestRegisterImportAdapter_DefaultAdapter(t *testing.T) { + ResetImportAdapterRegistry() + + // Adapter with no schemes should become the default adapter. + adapter := &mockTestAdapter{schemes: nil} + RegisterImportAdapter(adapter) + + adapters := GetRegisteredAdapters() + assert.Empty(t, adapters) // Should not be in the adapters list. + + defaultAdapter := GetDefaultAdapter() + assert.NotNil(t, defaultAdapter) + assert.Nil(t, defaultAdapter.Schemes()) +} + +func TestSetDefaultAdapter(t *testing.T) { + ResetImportAdapterRegistry() + + adapter := &mockTestAdapter{schemes: nil} + SetDefaultAdapter(adapter) + + result := GetDefaultAdapter() + assert.Equal(t, adapter, result) +} + +func TestGetRegisteredAdapters(t *testing.T) { + ResetImportAdapterRegistry() + + adapter1 := &mockTestAdapter{schemes: []string{"one://"}} + adapter2 := &mockTestAdapter{schemes: []string{"two://"}} + + RegisterImportAdapter(adapter1) + RegisterImportAdapter(adapter2) + + adapters := GetRegisteredAdapters() + assert.Len(t, adapters, 2) +} + +func TestGetDefaultAdapter_WhenNotSet(t *testing.T) { + ResetImportAdapterRegistry() + + result := GetDefaultAdapter() + assert.Nil(t, result) +} + +func TestResetImportAdapterRegistry(t *testing.T) { + // Register some adapters first. + adapter := &mockTestAdapter{schemes: []string{"test://"}} + RegisterImportAdapter(adapter) + SetDefaultAdapter(&mockTestAdapter{schemes: nil}) + + // Reset should clear everything. + ResetImportAdapterRegistry() + + assert.Empty(t, GetRegisteredAdapters()) + assert.Nil(t, GetDefaultAdapter()) +} + +func TestFindImportAdapter_NoDefaultAdapter(t *testing.T) { + ResetImportAdapterRegistry() + + // When no default adapter is set, should return noopAdapter. + adapter := FindImportAdapter("local/path.yaml") + assert.NotNil(t, adapter) + + // noopAdapter returns nil for schemes. + assert.Nil(t, adapter.Schemes()) + + // noopAdapter.Resolve returns nil, nil. + paths, err := adapter.Resolve(context.Background(), "", "", "", 0, 0) + assert.NoError(t, err) + assert.Nil(t, paths) +} + +func TestFindImportAdapter_MatchesScheme(t *testing.T) { + ResetImportAdapterRegistry() + + testAdapter := &mockTestAdapter{schemes: []string{"test://"}} + RegisterImportAdapter(testAdapter) + + adapter := FindImportAdapter("test://something") + assert.Equal(t, testAdapter, adapter) +} + +func TestFindImportAdapter_CaseInsensitive(t *testing.T) { + ResetImportAdapterRegistry() + + testAdapter := &mockTestAdapter{schemes: []string{"HTTP://"}} + RegisterImportAdapter(testAdapter) + + // Should match regardless of case. + adapter := FindImportAdapter("http://example.com") + assert.Equal(t, testAdapter, adapter) +} + +func TestFindImportAdapter_FallsBackToDefault(t *testing.T) { + ResetImportAdapterRegistry() + + testAdapter := &mockTestAdapter{schemes: []string{"test://"}} + defaultAdapter := &mockTestAdapter{schemes: nil} + + RegisterImportAdapter(testAdapter) + SetDefaultAdapter(defaultAdapter) + + // Path without matching scheme should return default adapter. + adapter := FindImportAdapter("local/path.yaml") + assert.Equal(t, defaultAdapter, adapter) +} + +func TestFindImportAdapter_MultipleSchemes(t *testing.T) { + ResetImportAdapterRegistry() + + multiSchemeAdapter := &mockTestAdapter{schemes: []string{"http://", "https://"}} + RegisterImportAdapter(multiSchemeAdapter) + + // Both schemes should match. + adapter1 := FindImportAdapter("http://example.com") + adapter2 := FindImportAdapter("https://example.com") + + assert.Equal(t, multiSchemeAdapter, adapter1) + assert.Equal(t, multiSchemeAdapter, adapter2) +} + +func TestSetBuiltinAdaptersInitializer(t *testing.T) { + ResetImportAdapterRegistry() + + called := false + testInitFunc := func() { + called = true + } + + SetBuiltinAdaptersInitializer(testInitFunc) + EnsureAdaptersInitialized() + + assert.True(t, called) +} + +func TestEnsureAdaptersInitialized_NilInitializer(t *testing.T) { + ResetImportAdapterRegistry() + + // Set initializer to nil. + SetBuiltinAdaptersInitializer(nil) + + // Should not panic. + EnsureAdaptersInitialized() +} + +func TestEnsureAdaptersInitialized_CalledOnce(t *testing.T) { + ResetImportAdapterRegistry() + + callCount := 0 + testInitFunc := func() { + callCount++ + } + + SetBuiltinAdaptersInitializer(testInitFunc) + + // Call multiple times. + EnsureAdaptersInitialized() + EnsureAdaptersInitialized() + EnsureAdaptersInitialized() + + // Should only be called once due to sync.Once. + assert.Equal(t, 1, callCount) +} + +func TestNoopAdapter_Schemes(t *testing.T) { + noop := &noopAdapter{} + assert.Nil(t, noop.Schemes()) +} + +func TestNoopAdapter_Resolve(t *testing.T) { + noop := &noopAdapter{} + paths, err := noop.Resolve(context.Background(), "path", "base", "temp", 1, 10) + assert.NoError(t, err) + assert.Nil(t, paths) +} + +func TestFindImportAdapter_RegistrationOrder(t *testing.T) { + ResetImportAdapterRegistry() + + // Register adapters - more specific first. + specificAdapter := &mockTestAdapter{schemes: []string{"github.com/"}} + generalAdapter := &mockTestAdapter{schemes: []string{"http://", "https://"}} + + RegisterImportAdapter(specificAdapter) + RegisterImportAdapter(generalAdapter) + + // GitHub URL should match specific adapter first. + adapter := FindImportAdapter("github.com/user/repo") + assert.Equal(t, specificAdapter, adapter) +} diff --git a/pkg/config/import_commands_test.go b/pkg/config/import_commands_test.go index 0ebd345401..f03ef40d75 100644 --- a/pkg/config/import_commands_test.go +++ b/pkg/config/import_commands_test.go @@ -13,6 +13,8 @@ import ( // TestImportCommandMerging tests various scenarios of command merging through imports. // This addresses the issue where imported commands should be merged (not replaced). func TestImportCommandMerging(t *testing.T) { + setupTestAdapters() + tests := []struct { name string setupFiles map[string]string @@ -258,6 +260,8 @@ commands: // TestImportCommandMergingEdgeCases tests edge cases in command merging. func TestImportCommandMergingEdgeCases(t *testing.T) { + setupTestAdapters() + tests := []struct { name string setupFiles map[string]string diff --git a/pkg/config/import_test.go b/pkg/config/import_test.go index a052f7891f..f4737444aa 100644 --- a/pkg/config/import_test.go +++ b/pkg/config/import_test.go @@ -2,8 +2,6 @@ package config import ( "fmt" - "net/http" - "net/http/httptest" "os" "path/filepath" "testing" @@ -19,17 +17,14 @@ func setupTestFile(content, tempDir string, filename string) (string, error) { // Test for processImports. func TestProcessImports(t *testing.T) { + setupTestAdapters() + err := os.Unsetenv("ATMOS_CLI_CONFIG_PATH") assert.NoError(t, err, "Unset 'ATMOS_CLI_CONFIG_PATH' environment variable should execute without error") err = os.Unsetenv("ATMOS_BASE_PATH") assert.NoError(t, err, "Unset 'ATMOS_BASE_PATH' environment variable should execute without error") err = os.Unsetenv("ATMOS_LOGS_LEVEL") assert.NoError(t, err, "Unset 'ATMOS_LOGS_LEVEL' environment variable should execute without error") - // Step 1: Setup a mock HTTP server for a remote URL - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintln(w, "base_path: ./") // Mock YAML content - })) - defer server.Close() // Step 2: Create temporary base directory and files baseDir := t.TempDir() @@ -58,15 +53,15 @@ func TestProcessImports(t *testing.T) { configFile3 := filepath.Join(configDir2, "config3.yaml") err = os.WriteFile(configFile3, []byte("key4: value4"), 0o600) assert.NoError(t, err) - // Step 3: Define test imports + // Step 3: Define test imports. + // Note: Remote URLs are not tested here because the test adapter only handles local paths. + // Remote URL handling is tested in the adapters package tests. imports := []string{ - server.URL, // Remote URL - "configs.d/**/*", // Recursive directory - "config/**/*.yaml", // recursive/**/*.yaml", // Recursive directory with specific pattern extension - "./logs.yaml", // Specific local file - "http://invalid-url.com", // Invalid URL - "", // Empty import path - "/config/foo.yaml", // Invalid import path + "configs.d/**/*", // Recursive directory + "config/**/*.yaml", // recursive/**/*.yaml", // Recursive directory with specific pattern extension + "./logs.yaml", // Specific local file + "", // Empty import path + "/config/foo.yaml", // Invalid import path } // Step 5: Run the processImports method @@ -76,7 +71,7 @@ func TestProcessImports(t *testing.T) { assert.NoError(t, err, "processImports should not return an error") var resolvedPaths []string for _, resolvedPath := range imported { - resolvedPaths = append(resolvedPaths, resolvedPath.filePath) + resolvedPaths = append(resolvedPaths, resolvedPath.FilePath) } // Verify resolved paths contain expected files @@ -90,12 +85,13 @@ func TestProcessImports(t *testing.T) { assert.Contains(t, resolvedPaths, expectedPath, fmt.Sprintf("resolvedPaths should contain %s", expectedPath)) } - // Ensure invalid and empty imports are handled gracefully - assert.NotContains(t, resolvedPaths, "http://invalid-url.com", "Invalid URL should not be resolved") + // Ensure empty imports are handled gracefully. assert.NotContains(t, resolvedPaths, "", "Empty import path should not be resolved") } func TestProcessImportNested(t *testing.T) { + setupTestAdapters() + err := os.Unsetenv("ATMOS_CLI_CONFIG_PATH") assert.NoError(t, err, "Unset 'ATMOS_CLI_CONFIG_PATH' environment variable should execute without error") err = os.Unsetenv("ATMOS_BASE_PATH") @@ -104,7 +100,7 @@ func TestProcessImportNested(t *testing.T) { assert.NoError(t, err, "Unset 'ATMOS_LOGS_LEVEL' environment variable should execute without error") baseDir := t.TempDir() - // Setting up test files + // Setting up test files. _, err = setupTestFile(` import: - "./nested-local.yaml" @@ -114,32 +110,6 @@ import: nestedLocalConfigPath, err := setupTestFile(`import: []`, baseDir, "nested-local.yaml") assert.NoError(t, err) - remoteContent := ` -import: - - nested-local.yaml -` - nestedRemoteContent := `import: []` - // Create an HTTP server to simulate remote imports - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/config.yaml": - fmt.Fprint(w, remoteContent) - case "/nested-remote.yaml": - fmt.Fprint(w, nestedRemoteContent) - default: - http.NotFound(w, r) - } - })) - defer server.Close() - - t.Run("Test remote import processing", func(t *testing.T) { - tempDir := t.TempDir() - importPaths := []string{server.URL + "/config.yaml"} - resolved, err := processImports(baseDir, importPaths, tempDir, 1, 5) - assert.NoError(t, err) - assert.Len(t, resolved, 2, "should resolve main and nested remote imports") - }) - t.Run("Test local import processing", func(t *testing.T) { tempDir := t.TempDir() importPaths := []string{"local.yaml"} @@ -147,17 +117,14 @@ import: assert.NoError(t, err) var resolvedPaths []string for _, resolvedPath := range imported { - resolvedPaths = append(resolvedPaths, resolvedPath.filePath) + resolvedPaths = append(resolvedPaths, resolvedPath.FilePath) } assert.Contains(t, resolvedPaths, nestedLocalConfigPath, "should resolve nested local imports") }) - t.Run("Test mixed imports with depth limit", func(t *testing.T) { + t.Run("Test imports with depth limit", func(t *testing.T) { tempDir := t.TempDir() - importPaths := []string{ - "local.yaml", - server.URL + "/config.yaml", - } + importPaths := []string{"local.yaml"} resolved, err := processImports(baseDir, importPaths, tempDir, 11, 10) assert.Error(t, err, "should return an error when maxDepth is exceeded") assert.Nil(t, resolved, "no resolved paths should be returned on depth limit breach") @@ -255,9 +222,11 @@ func TestSanitizeImport(t *testing.T) { } } -// TestProcessLocalImport_OutsideBaseDirectory verifies that imports outside the base directory +// TestProcessImports_OutsideBaseDirectory verifies that imports outside the base directory // work correctly and only log at trace level (not warning level). -func TestProcessLocalImport_OutsideBaseDirectory(t *testing.T) { +func TestProcessImports_OutsideBaseDirectory(t *testing.T) { + setupTestAdapters() + // Create a parent directory and a subdirectory. parentDir := t.TempDir() subDir := filepath.Join(parentDir, "repo") @@ -289,12 +258,13 @@ settings: // Process the import from the subdirectory (base path). // This simulates the case where .github/atmos.yaml imports ../atmos.yaml. tempDir := t.TempDir() - resolvedPaths, err := processLocalImport(subDir, "../atmos.yaml", tempDir, 1, 10) + + resolvedPaths, err := processImports(subDir, []string{"../atmos.yaml"}, tempDir, 1, 10) // Verify that the import resolves successfully. assert.NoError(t, err) assert.NotEmpty(t, resolvedPaths) - assert.Equal(t, parentConfigPath, resolvedPaths[0].filePath) + assert.Equal(t, parentConfigPath, resolvedPaths[0].FilePath) // The import should work despite being outside base directory. // The message is now logged at Trace level, not Warn level. diff --git a/pkg/config/import_test_helpers_test.go b/pkg/config/import_test_helpers_test.go new file mode 100644 index 0000000000..4d94b25da9 --- /dev/null +++ b/pkg/config/import_test_helpers_test.go @@ -0,0 +1,101 @@ +package config + +import ( + "context" + "path/filepath" + "strings" + + "github.com/spf13/viper" + + errUtils "github.com/cloudposse/atmos/errors" + log "github.com/cloudposse/atmos/pkg/logger" +) + +// testLocalAdapter is a minimal local adapter for testing. +// It implements the same logic as adapters.LocalAdapter but without +// importing the adapters package to avoid circular imports. +type testLocalAdapter struct{} + +func (l *testLocalAdapter) Schemes() []string { + return nil +} + +func (l *testLocalAdapter) Resolve( + _ context.Context, + importPath string, + basePath string, + tempDir string, + currentDepth int, + maxDepth int, +) ([]ResolvedPaths, error) { + if importPath == "" { + return nil, errUtils.ErrImportPathRequired + } + + // Make path absolute relative to basePath. + resolvedPath := importPath + if !filepath.IsAbs(importPath) { + resolvedPath = filepath.Join(basePath, importPath) + } + + // Log if import path is outside base directory. + if !strings.HasPrefix(filepath.Clean(resolvedPath), filepath.Clean(basePath)) { + log.Trace("Import path is outside of base directory", + "importPath", resolvedPath, + "basePath", basePath, + ) + } + + // Search for matching config files. + paths, err := SearchAtmosConfig(resolvedPath) + if err != nil { + log.Debug("failed to resolve local import path", "path", importPath, "err", err) + return nil, errUtils.ErrResolveLocal + } + + resolvedPaths := make([]ResolvedPaths, 0) + + // Process each matched file. + for _, path := range paths { + v := viper.New() + v.SetConfigFile(path) + v.SetConfigType("yaml") + err := v.ReadInConfig() + if err != nil { + log.Debug("failed to load local config", "path", path, "error", err) + continue + } + + resolvedPaths = append(resolvedPaths, ResolvedPaths{ + FilePath: path, + ImportPaths: importPath, + ImportType: LOCAL, + }) + + // Check for nested imports. + imports := v.GetStringSlice("import") + importBasePath := v.GetString("base_path") + if importBasePath == "" { + importBasePath = basePath + } + + // Recursively process nested imports. + if len(imports) > 0 { + nestedPaths, err := ProcessImportsFromAdapter(importBasePath, imports, tempDir, currentDepth+1, maxDepth) + if err != nil { + log.Debug("failed to process nested imports from", "path", path, "error", err) + continue + } + resolvedPaths = append(resolvedPaths, nestedPaths...) + } + } + + return resolvedPaths, nil +} + +// setupTestAdapters registers minimal test adapters for unit testing. +// This must be called at the start of tests that use processImports. +func setupTestAdapters() { + ResetImportAdapterRegistry() + SetDefaultAdapter(&testLocalAdapter{}) +} diff --git a/pkg/config/imports.go b/pkg/config/imports.go index a6330e0158..76681204b5 100644 --- a/pkg/config/imports.go +++ b/pkg/config/imports.go @@ -8,9 +8,7 @@ import ( "path/filepath" "sort" "strings" - "time" - "github.com/hashicorp/go-getter" "github.com/spf13/viper" errUtils "github.com/cloudposse/atmos/errors" @@ -55,17 +53,18 @@ func sanitizeImport(s string) string { type importTypes int const ( - LOCAL importTypes = 0 - REMOTE importTypes = 1 + LOCAL importTypes = 0 + REMOTE importTypes = 1 + ADAPTER importTypes = 2 ) var defaultFileSystem = filesystem.NewOSFileSystem() -// import Resolved Paths. +// ResolvedPaths represents a resolved import path with its file location and type. type ResolvedPaths struct { - filePath string - importPaths string // import path from atmos config - importType importTypes + FilePath string // Absolute path to the resolved file. + ImportPaths string // Original import path from atmos config. + ImportType importTypes // Type of import (LOCAL, REMOTE, or ADAPTER). } // processConfigImports It reads the import paths from the source configuration. @@ -105,18 +104,24 @@ func processConfigImportsWithFS(source *schema.AtmosConfiguration, dst *viper.Vi for _, resolvedPath := range resolvedPaths { // Trace: log what we're about to merge (sanitized). - log.Trace("attempting to merge import", "import", sanitizeImport(resolvedPath.importPaths), "file_path", resolvedPath.filePath) - err := mergeConfigFile(resolvedPath.filePath, dst) + log.Trace("attempting to merge import", "import", sanitizeImport(resolvedPath.ImportPaths), "file_path", resolvedPath.FilePath) + err := mergeConfigFile(resolvedPath.FilePath, dst) if err != nil { - log.Trace("error loading config file", "import", sanitizeImport(resolvedPath.importPaths), "file_path", resolvedPath.filePath, "error", err) + log.Trace("error loading config file", "import", sanitizeImport(resolvedPath.ImportPaths), "file_path", resolvedPath.FilePath, "error", err) continue } - log.Trace("successfully merged config from import", "import", sanitizeImport(resolvedPath.importPaths), "file_path", resolvedPath.filePath) + log.Trace("successfully merged config from import", "import", sanitizeImport(resolvedPath.ImportPaths), "file_path", resolvedPath.FilePath) } return nil } +// ProcessImportsFromAdapter is the public entry point for adapters to process nested imports. +// Adapters should call this when they discover nested import statements in resolved configs. +func ProcessImportsFromAdapter(basePath string, importPaths []string, tempDir string, currentDepth, maxDepth int) ([]ResolvedPaths, error) { + return processImports(basePath, importPaths, tempDir, currentDepth, maxDepth) +} + func processImports(basePath string, importPaths []string, tempDir string, currentDepth, maxDepth int) (resolvedPaths []ResolvedPaths, err error) { if basePath == "" { return nil, errUtils.ErrBasePath @@ -133,135 +138,25 @@ func processImports(basePath string, importPaths []string, tempDir string, curre return nil, err } + ctx := context.Background() + for _, importPath := range importPaths { if importPath == "" { continue } - if isRemoteImport(importPath) { - // Handle remote imports - paths, err := processRemoteImport(basePath, importPath, tempDir, currentDepth, maxDepth) - if err != nil { - log.Debug("failed to process remote import", "path", importPath, "error", err) - continue - } - resolvedPaths = append(resolvedPaths, paths...) - } else { - // Handle local imports - paths, err := processLocalImport(basePath, importPath, tempDir, currentDepth, maxDepth) - if err != nil { - log.Debug("failed to process local import", "path", importPath, "error", err) - continue - } - resolvedPaths = append(resolvedPaths, paths...) - } - } - - return resolvedPaths, nil -} - -// Helper to determine if the import is a supported remote source. -func isRemoteImport(importPath string) bool { - return strings.HasPrefix(importPath, "http://") || strings.HasPrefix(importPath, "https://") -} - -// Process remote imports. -func processRemoteImport(basePath, importPath, tempDir string, currentDepth, maxDepth int) ([]ResolvedPaths, error) { - parsedURL, err := url.Parse(importPath) - if err != nil || (parsedURL.Scheme != "http" && parsedURL.Scheme != "https") { - log.Debug("unsupported URL", "URL", importPath, "error", err) - return nil, err - } - - tempFile, err := downloadRemoteConfig(parsedURL.String(), tempDir) - if err != nil { - log.Debug("failed to download remote config", "path", importPath, "error", err) - return nil, err - } - v := viper.New() - v.SetConfigFile(tempFile) - err = v.ReadInConfig() - if err != nil { - log.Debug("failed to read remote config", "path", importPath, "error", err) - return nil, err - } - - resolvedPaths := make([]ResolvedPaths, 0) - resolvedPaths = append(resolvedPaths, ResolvedPaths{ - filePath: tempFile, - importPaths: importPath, - importType: REMOTE, - }) - imports := v.GetStringSlice("import") - importBasePath := v.GetString("base_path") - if importBasePath == "" { - importBasePath = basePath - } - - // Recursively process imports from the remote file - if len(imports) > 0 { - nestedPaths, err := processImports(importBasePath, imports, tempDir, currentDepth+1, maxDepth) - if err != nil { - log.Debug("failed to process nested imports", "import", importPath, "err", err) - return nil, err - } - resolvedPaths = append(resolvedPaths, nestedPaths...) - } - - return resolvedPaths, nil -} + var paths []ResolvedPaths + var resolveErr error -// Process local imports. -func processLocalImport(basePath string, importPath, tempDir string, currentDepth, maxDepth int) ([]ResolvedPaths, error) { - if importPath == "" { - return nil, errUtils.ErrImportPathRequired - } - if !filepath.IsAbs(importPath) { - importPath = filepath.Join(basePath, importPath) - } - if !strings.HasPrefix(filepath.Clean(importPath), filepath.Clean(basePath)) { - log.Trace("Import path is outside of base directory", - "importPath", importPath, - "basePath", basePath, - ) - } - paths, err := SearchAtmosConfig(importPath) - if err != nil { - log.Debug("failed to resolve local import path", "path", importPath, "err", err) - return nil, errUtils.ErrResolveLocal - } + // Use the adapter registry to find the appropriate adapter. + adapter := FindImportAdapter(importPath) + paths, resolveErr = adapter.Resolve(ctx, importPath, basePath, tempDir, currentDepth, maxDepth) - resolvedPaths := make([]ResolvedPaths, 0) - // Load the local configuration file to check for further imports - for _, path := range paths { - v := viper.New() - v.SetConfigFile(path) - v.SetConfigType("yaml") - err := v.ReadInConfig() - if err != nil { - log.Debug("failed to load local config", "path", path, "error", err) + if resolveErr != nil { + log.Debug("failed to resolve import", "path", importPath, "error", resolveErr) continue } - resolvedPaths = append(resolvedPaths, ResolvedPaths{ - filePath: path, - importPaths: importPath, - importType: LOCAL, - }) - imports := v.GetStringSlice("import") - importBasePath := v.GetString("base_path") - if importBasePath == "" { - importBasePath = basePath - } - - // Recursively process imports from the local file - if len(imports) > 0 { - nestedPaths, err := processImports(importBasePath, imports, tempDir, currentDepth+1, maxDepth) - if err != nil { - log.Debug("failed to process nested imports from", "path", path, "error", err) - continue - } - resolvedPaths = append(resolvedPaths, nestedPaths...) - } + resolvedPaths = append(resolvedPaths, paths...) } return resolvedPaths, nil @@ -427,23 +322,3 @@ func findMatchingFiles(patterns []string) ([]string, error) { return filePaths, nil } - -func downloadRemoteConfig(url string, tempDir string) (string, error) { - // uniq name for the temp file - fileName := fmt.Sprintf("atmos-import-%d.yaml", time.Now().UnixNano()) - tempFile := filepath.Join(tempDir, fileName) - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - client := &getter.Client{ - Ctx: ctx, - Src: url, - Dst: tempFile, - Mode: getter.ClientModeFile, - } - err := client.Get() - if err != nil { - os.RemoveAll(tempFile) - return "", fmt.Errorf("failed to download remote config: %w", err) - } - return tempFile, nil -} diff --git a/pkg/config/imports_error_paths_test.go b/pkg/config/imports_error_paths_test.go index b913f47904..6815706489 100644 --- a/pkg/config/imports_error_paths_test.go +++ b/pkg/config/imports_error_paths_test.go @@ -119,150 +119,6 @@ func TestProcessImports_EmptyImportPath(t *testing.T) { assert.Empty(t, paths) // Empty import path should be skipped } -// TestProcessRemoteImport_InvalidURL tests non-http/https URL handling at imports.go:157-161. -// Note: Current implementation returns nil error for unsupported schemes when url.Parse succeeds. -func TestProcessRemoteImport_InvalidURL(t *testing.T) { - tempDir := t.TempDir() - - // Invalid URL scheme (not http/https) - current implementation returns nil, nil - _, err := processRemoteImport(tempDir, "ftp://invalid.com/config.yaml", tempDir, 1, MaximumImportLvL) - assert.NoError(t, err) // Current behavior: no error for unsupported schemes -} - -// TestProcessRemoteImport_ParseError tests error path at imports.go:157-161. -func TestProcessRemoteImport_ParseError(t *testing.T) { - tempDir := t.TempDir() - - // Malformed URL - _, err := processRemoteImport(tempDir, "http://[::1]:namedport", tempDir, 1, MaximumImportLvL) - assert.Error(t, err) -} - -// TestProcessRemoteImport_DownloadError tests error path at imports.go:163-167. -func TestProcessRemoteImport_DownloadError(t *testing.T) { - tempDir := t.TempDir() - - // Non-existent remote URL - _, err := processRemoteImport(tempDir, "https://nonexistent.invalid/config.yaml", tempDir, 1, MaximumImportLvL) - assert.Error(t, err) // Download should fail -} - -// TestProcessRemoteImport_ReadConfigError tests error path at imports.go:170-174. -func TestProcessRemoteImport_ReadConfigError(t *testing.T) { - tempDir := t.TempDir() - - // Create a local file with invalid YAML to simulate download success but read failure - invalidYAML := `invalid: [yaml: content` - invalidPath := filepath.Join(tempDir, "invalid.yaml") - err := os.WriteFile(invalidPath, []byte(invalidYAML), 0o644) - assert.NoError(t, err) - - // Simulate what processRemoteImport does with a malformed file - v := viper.New() - v.SetConfigFile(invalidPath) - err = v.ReadInConfig() - assert.Error(t, err) // Should fail to read invalid YAML -} - -// TestProcessRemoteImport_NestedImportsError tests error path at imports.go:189-194. -func TestProcessRemoteImport_NestedImportsError(t *testing.T) { - tempDir := t.TempDir() - - // Create a config with nested imports that exceed max depth - configContent := ` -base_path: /test -import: - - nested.yaml -` - configPath := filepath.Join(tempDir, "remote.yaml") - err := os.WriteFile(configPath, []byte(configContent), 0o644) - assert.NoError(t, err) - - // Manually test the nested import logic - v := viper.New() - v.SetConfigFile(configPath) - err = v.ReadInConfig() - assert.NoError(t, err) - - imports := v.GetStringSlice("import") - assert.NotEmpty(t, imports) - - // Test that processImports would fail at max depth - _, err = processImports(tempDir, imports, tempDir, MaximumImportLvL+1, MaximumImportLvL) - assert.Error(t, err) - assert.ErrorIs(t, err, errUtils.ErrMaxImportDepth) -} - -// TestProcessLocalImport_EmptyImportPath tests error path at imports.go:203-205. -func TestProcessLocalImport_EmptyImportPath(t *testing.T) { - tempDir := t.TempDir() - - _, err := processLocalImport(tempDir, "", tempDir, 1, MaximumImportLvL) - assert.Error(t, err) - assert.ErrorIs(t, err, errUtils.ErrImportPathRequired) -} - -// TestProcessLocalImport_SearchConfigError tests error path at imports.go:215-219. -func TestProcessLocalImport_SearchConfigError(t *testing.T) { - tempDir := t.TempDir() - - // Try to import non-existent path - nonExistentPath := filepath.Join(tempDir, "nonexistent", "config.yaml") - - _, err := processLocalImport(tempDir, nonExistentPath, tempDir, 1, MaximumImportLvL) - assert.Error(t, err) - assert.ErrorIs(t, err, errUtils.ErrResolveLocal) -} - -// TestProcessLocalImport_ReadConfigError tests error path at imports.go:227-231. -func TestProcessLocalImport_ReadConfigError(t *testing.T) { - tempDir := t.TempDir() - - // Create invalid YAML file - invalidContent := `invalid: [yaml: content` - invalidPath := filepath.Join(tempDir, "invalid.yaml") - err := os.WriteFile(invalidPath, []byte(invalidContent), 0o644) - assert.NoError(t, err) - - // processLocalImport should skip files that fail to load - paths, err := processLocalImport(tempDir, "invalid.yaml", tempDir, 1, MaximumImportLvL) - // Should not error but should continue processing - assert.NoError(t, err) - // The invalid file might still be in paths, but ReadInConfig would have failed - _ = paths -} - -// TestProcessLocalImport_NestedImportsError tests error path at imports.go:244-249. -func TestProcessLocalImport_NestedImportsError(t *testing.T) { - tempDir := t.TempDir() - - // Create config with nested import - configContent := ` -base_path: /test -import: - - nested.yaml -` - configPath := filepath.Join(tempDir, "config.yaml") - err := os.WriteFile(configPath, []byte(configContent), 0o644) - assert.NoError(t, err) - - // Create nested config that would exceed max depth - nestedContent := ` -base_path: /test -import: - - another.yaml -` - nestedPath := filepath.Join(tempDir, "nested.yaml") - err = os.WriteFile(nestedPath, []byte(nestedContent), 0o644) - assert.NoError(t, err) - - // Process with depth that will hit max. - // The first-level import succeeds, but nested import processing should fail at max depth. - _, err = processLocalImport(tempDir, "config.yaml", tempDir, MaximumImportLvL, MaximumImportLvL) - // The first-level import should succeed. - assert.NoError(t, err) -} - // TestSearchAtmosConfig_FindMatchingFilesError tests error path at imports.go:268-271. func TestSearchAtmosConfig_FindMatchingFilesError(t *testing.T) { tempDir := t.TempDir() @@ -403,36 +259,4 @@ func TestFindMatchingFiles_NoMatches(t *testing.T) { assert.ErrorIs(t, err, errUtils.ErrNoFileMatchPattern) } -// TestDownloadRemoteConfig_DownloadError tests error path at imports.go:430-434. -func TestDownloadRemoteConfig_DownloadError(t *testing.T) { - tempDir := t.TempDir() - - // Invalid URL that will fail to download - _, err := downloadRemoteConfig("https://nonexistent.invalid/config.yaml", tempDir) - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to download remote config") -} - -// TestIsRemoteImport tests remote import detection. -func TestIsRemoteImport(t *testing.T) { - tests := []struct { - name string - path string - expected bool - }{ - {"http URL", "http://example.com/config.yaml", true}, - {"https URL", "https://example.com/config.yaml", true}, - {"local path", "local/path/config.yaml", false}, - {"absolute path", "/absolute/path/config.yaml", false}, - {"git URL", "git::https://github.com/user/repo.git", false}, // Not http/https prefix - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := isRemoteImport(tt.path) - assert.Equal(t, tt.expected, result) - }) - } -} - // Additional credential redaction tests are in import_test.go:TestSanitizeImport. diff --git a/pkg/devcontainer/lifecycle_rebuild_test.go b/pkg/devcontainer/lifecycle_rebuild_test.go index 4a7e3a30ab..bdcc2f7392 100644 --- a/pkg/devcontainer/lifecycle_rebuild_test.go +++ b/pkg/devcontainer/lifecycle_rebuild_test.go @@ -409,7 +409,7 @@ func TestManager_Rebuild(t *testing.T) { Context: ".", Dockerfile: "Dockerfile", Args: map[string]string{ - "ATMOS_VERSION": "1.203.0", + "ATMOS_VERSION": "1.204.0", }, }, } @@ -431,7 +431,7 @@ func TestManager_Rebuild(t *testing.T) { assert.Equal(t, ".", buildConfig.Context) assert.Equal(t, "Dockerfile", buildConfig.Dockerfile) assert.Equal(t, []string{"atmos-devcontainer-geodesic"}, buildConfig.Tags) - assert.Equal(t, map[string]string{"ATMOS_VERSION": "1.203.0"}, buildConfig.Args) + assert.Equal(t, map[string]string{"ATMOS_VERSION": "1.204.0"}, buildConfig.Args) return nil }) // Pull is NOT called for locally built images since they don't exist diff --git a/website/blog/2025-01-06-import-adapter-registry.mdx b/website/blog/2025-01-06-import-adapter-registry.mdx new file mode 100644 index 0000000000..45e21fe37a --- /dev/null +++ b/website/blog/2025-01-06-import-adapter-registry.mdx @@ -0,0 +1,33 @@ +--- +slug: import-adapter-registry +title: "Unified Import Adapter Registry" +authors: [osterman] +tags: [feature] +--- + +Atmos now includes a unified import adapter registry that provides a modular, extensible architecture for configuration imports. + + + +## What Changed + +The import system has been refactored to use an adapter-based registry pattern: + +- **GoGetterAdapter** handles all remote imports (http://, https://, git::, s3::, gcs::, oci://, etc.) +- **LocalAdapter** handles local filesystem paths +- **MockAdapter** provides synthetic YAML generation for testing + +This replaces the previous if/else chain with a clean, extensible registry that routes imports to the appropriate adapter based on URL scheme. + +## Why This Matters + +The new architecture: + +- **Fixes go-getter routing** - All go-getter schemes now work correctly (git::, s3::, gcs::, oci://, etc.) +- **Enables extensibility** - New import schemes can be added by implementing the `ImportAdapter` interface +- **Improves testability** - The `mock://` scheme enables testing without external dependencies +- **Follows established patterns** - Consistent with the command registry and store registry patterns + +## How to Use It + +Existing imports continue to work unchanged. The new architecture is transparent to users while enabling future extensibility like `terragrunt://` for HCL-to-YAML transformation. diff --git a/website/src/data/roadmap.js b/website/src/data/roadmap.js index d6760f2c7e..7116a83833 100644 --- a/website/src/data/roadmap.js +++ b/website/src/data/roadmap.js @@ -266,6 +266,7 @@ export const roadmapConfig = { { label: 'Added `!env` function with stack manifest support', status: 'shipped', quarter: 'q4-2025', changelog: 'env-function-stack-manifest-support', version: 'v1.199.0', description: 'The !env function now reads from stack manifest env sections, not just system environment.', benefits: 'Reference environment variables defined in stack configuration. Consistent behavior across local and CI.' }, { label: 'Circular dependency detection for YAML functions', status: 'shipped', quarter: 'q4-2025', changelog: 'yaml-function-circular-dependency-detection', version: 'v1.196.0', description: 'Automatic detection of circular dependencies between YAML functions with clear error messages.', benefits: 'Catch configuration errors before deployment. Clear error messages show the dependency chain.' }, { label: 'Deferred YAML function evaluation in merge', status: 'shipped', quarter: 'q4-2025', changelog: 'deferred-yaml-functions-evaluation-in-merge', description: 'YAML functions are now evaluated after merge, allowing functions to reference inherited values.', benefits: 'Functions work correctly with inheritance. Reference values from base stacks in function arguments.' }, + { label: 'Unified import adapter registry', status: 'shipped', quarter: 'q1-2026', pr: 1897, changelog: 'import-adapter-registry', description: 'Pluggable import adapter system for configuration imports with support for local files, remote URLs (HTTP, Git, S3, OCI), and custom adapters.', benefits: 'Extend import sources without modifying core code. Add support for new protocols with a simple adapter interface.' }, { label: 'Boolean flags with defaults for custom commands', status: 'shipped', quarter: 'q4-2025', changelog: 'custom-command-boolean-flags', version: 'v1.201.0', description: 'Custom commands now support boolean flags with configurable default values.', benefits: 'Create intuitive CLI flags for custom commands. Defaults reduce repetitive typing.' }, { label: 'Added `!unset` YAML function', status: 'planned', quarter: 'q1-2026', pr: 1521, description: 'Delete keys from inherited configuration for clean overrides.', codeExample: 'deprecated_setting: !unset', benefits: 'Remove inherited values without workarounds. Clean up configuration by exception.' }, { label: 'Added `!append` YAML function', status: 'planned', quarter: 'q1-2026', pr: 1513, description: 'Append items to inherited lists instead of replacing them.', codeExample: 'extra_tags: !append [new-tag]', benefits: 'Add to inherited lists without copying the full list. Simpler inheritance for array values.' },