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.

Sign up for GitHub

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

feat: Support Relative Path Imports #891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c50d071
Handle relative paths for stack imports
milldr Dec 24, 2024
f79e16a
Tests for relative paths
milldr Dec 24, 2024
74ab1d8
Refactor handling of relative paths in ProcessImportSection
milldr Dec 24, 2024
e2af2db
Handle relative paths in StackImport.Path and string imports
milldr Dec 24, 2024
9d39012
Add new test2/us-west-1.yaml file and update test counts
milldr Dec 24, 2024
12358b0
Update import paths in us-west-1.yaml file
milldr Dec 24, 2024
093ac5e
Merge branch 'main' into feat/DEV-1856_relative-paths
milldr Dec 24, 2024
b9856ff
Merge branch 'main' into feat/DEV-1856_relative-paths
osterman Dec 26, 2024
71eaca0
Merge branch 'main' into feat/DEV-1856_relative-paths
milldr Dec 26, 2024
4f6170c
Merge branch 'feat/DEV-1856_relative-paths' of github.com:cloudposse/…
milldr Dec 26, 2024
362e979
Merge branch 'main' into feat/DEV-1856_relative-paths
milldr Dec 27, 2024
8d76059
Refactor handling of import paths in ProcessImportSection
milldr Dec 27, 2024
1beeb8a
Resolve relative paths in ProcessImportSection function
milldr Dec 27, 2024
dd55c0f
Update internal/exec/stack_processor_utils.go
milldr Dec 27, 2024
dfa5a84
Refactor resolveRelativePath for improved path handling
milldr Dec 27, 2024
507a5dd
Refactor filepath handling for better readability
milldr Dec 27, 2024
cafdfdd
Remove unnecessary path normalization code and simplify logic
milldr Dec 27, 2024
ddde3d5
Improve path resolution handling in stack_processor_utils
milldr Dec 27, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
28 changes: 24 additions & 4 deletions internal/exec/stack_processor_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1743,11 +1743,29 @@ 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
}

// Check if the path starts with "." or ".."
firstElement := filepath.Clean(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.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 +1791,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 +1805,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
View file
Open in desktop
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