Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support Relative Path Imports #891

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions examples/tests/stacks/mixins/stage/test2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# yaml-language-server: $schema=https://atmos.tools/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json

vars:
stage: test-2

settings:
config:
is_prod: false
5 changes: 5 additions & 0 deletions examples/tests/stacks/orgs/cp/tenant1/test2/_defaults.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# yaml-language-server: $schema=https://atmos.tools/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json

import:
- mixins/stage/test2
- ../_defaults # validate relative paths
5 changes: 5 additions & 0 deletions examples/tests/stacks/orgs/cp/tenant1/test2/us-east-2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# yaml-language-server: $schema=https://atmos.tools/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json

import:
- mixins/region/us-east-2
- ./_defaults # validate relative paths
5 changes: 5 additions & 0 deletions examples/tests/stacks/orgs/cp/tenant1/test2/us-west-1.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# yaml-language-server: $schema=https://atmos.tools/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json

import:
- path: mixins/region/us-west-1
- path: ./_defaults # validate relative paths using the `path` field
32 changes: 28 additions & 4 deletions internal/exec/stack_processor_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -1743,11 +1743,33 @@ func FindComponentDependenciesLegacy(
return unique, nil
}

// resolveRelativePath checks if a path is relative to the current directory and if so,
// resolves it relative to the current file's directory
func resolveRelativePath(path string, currentFilePath string) string {
if path == "" {
return path
}

// Normalize input path to support multiple OS types
path = filepath.FromSlash(path)
currentFilePath = filepath.FromSlash(currentFilePath)

// Check if the path starts with "." or ".."
firstElement := strings.Split(path, string(filepath.Separator))[0]
if firstElement == "." || firstElement == ".." {
// Join the current local path with the current stack file path
baseDir := filepath.Dir(currentFilePath)
result := filepath.ToSlash(filepath.Clean(filepath.Join(baseDir, path)))
return result
}
return path
}

// ProcessImportSection processes the `import` section in stack manifests
// The `import` section` can be of the following types:
// 1. list of `StackImport` structs
// 2. list of strings
// 3. List of strings and `StackImport` structs in the same file
// The `import` section can contain:
// 1. Project-relative paths (e.g. "mixins/region/us-east-2")
// 2. Paths relative to the current stack file (e.g. "./_defaults")
// 3. StackImport structs containing either of the above path types (e.g. "path: mixins/region/us-east-2")
func ProcessImportSection(stackMap map[string]any, filePath string) ([]schema.StackImport, error) {
stackImports, ok := stackMap[cfg.ImportSectionName]

Expand All @@ -1773,6 +1795,7 @@ func ProcessImportSection(stackMap map[string]any, filePath string) ([]schema.St
var importObj schema.StackImport
err := mapstructure.Decode(imp, &importObj)
if err == nil {
importObj.Path = resolveRelativePath(importObj.Path, filePath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve path resolution validation in ProcessImportSection

The resolved paths should be validated before being used in imports.

Apply these improvements:

-            importObj.Path = resolveRelativePath(importObj.Path, filePath)
+            resolvedPath := resolveRelativePath(importObj.Path, filePath)
+            if err := validateImportPath(resolvedPath); err != nil {
+                return nil, fmt.Errorf("invalid import path '%s' in file '%s': %v", importObj.Path, filePath, err)
+            }
+            importObj.Path = resolvedPath

// ... and similarly for string imports:

-            s = resolveRelativePath(s, filePath)
+            resolvedPath := resolveRelativePath(s, filePath)
+            if err := validateImportPath(resolvedPath); err != nil {
+                return nil, fmt.Errorf("invalid import path '%s' in file '%s': %v", s, filePath, err)
+            }
+            s = resolvedPath

Add this validation helper:

func validateImportPath(path string) error {
    // Ensure path is not empty
    if path == "" {
        return fmt.Errorf("empty path")
    }

    // Check for invalid characters
    if strings.ContainsAny(path, "<>:\"\\|?*") {
        return fmt.Errorf("path contains invalid characters")
    }

    // Additional validation as needed
    return nil
}

Also applies to: 1808-1808

result = append(result, importObj)
continue
}
Expand All @@ -1786,6 +1809,7 @@ func ProcessImportSection(stackMap map[string]any, filePath string) ([]schema.St
return nil, fmt.Errorf("invalid empty import in the file '%s'", filePath)
}

s = resolveRelativePath(s, filePath)
result = append(result, schema.StackImport{Path: s})
}

Expand Down
9 changes: 6 additions & 3 deletions pkg/stack/stack_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ func TestStackProcessor(t *testing.T) {
"../../examples/tests/stacks/orgs/cp/tenant1/prod/us-east-2.yaml",
"../../examples/tests/stacks/orgs/cp/tenant1/staging/us-east-2.yaml",
"../../examples/tests/stacks/orgs/cp/tenant1/test1/us-east-2.yaml",
"../../examples/tests/stacks/orgs/cp/tenant1/test2/us-east-2.yaml",
"../../examples/tests/stacks/orgs/cp/tenant1/test2/us-west-1.yaml",
}

processStackDeps := true
Expand Down Expand Up @@ -50,15 +52,16 @@ func TestStackProcessor(t *testing.T) {
)

assert.Nil(t, err)
assert.Equal(t, 4, len(listResult))
assert.Equal(t, 4, len(mapResult))
assert.Equal(t, 6, len(listResult))
assert.Equal(t, 6, len(mapResult))

mapResultKeys := u.StringKeysFromMap(mapResult)
assert.Equal(t, "orgs/cp/tenant1/dev/us-east-2", mapResultKeys[0])
assert.Equal(t, "orgs/cp/tenant1/prod/us-east-2", mapResultKeys[1])
assert.Equal(t, "orgs/cp/tenant1/staging/us-east-2", mapResultKeys[2])
assert.Equal(t, "orgs/cp/tenant1/test1/us-east-2", mapResultKeys[3])

assert.Equal(t, "orgs/cp/tenant1/test2/us-east-2", mapResultKeys[4])
assert.Equal(t, "orgs/cp/tenant1/test2/us-west-1", mapResultKeys[5])
mapConfig1, err := u.UnmarshalYAML[schema.AtmosSectionMapType](listResult[0])
assert.Nil(t, err)

Expand Down
Loading