-
Notifications
You must be signed in to change notification settings - Fork 10
feat: include rendered spec location in component list output #85
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
9e2755f
Initial plan
Copilot f797135
feat: include rendered spec dir in component list output
Copilot 043f8df
fix: use filepath.Join in tests for cross-platform path construction
Copilot c18dadc
refactor: move RenderedSpecDir into ComponentConfig as a derived field
Copilot 0f60825
fix: omit RenderedSpecDir from table output
Copilot 3bd1ac1
commonize on fileutils.VaidateFilename()
dmcilvaney 3f93a81
feedback: don't recalc dir, validate even without config dir
dmcilvaney 78f07df
fix lo to for loop swap
dmcilvaney File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package components | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "path/filepath" | ||
|
|
||
| "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" | ||
| ) | ||
|
|
||
| // RenderedSpecDir returns the rendered spec output directory for a given component. | ||
| // The path is computed as {renderedSpecsDir}/{componentName}. | ||
| // Returns an empty string if renderedSpecsDir is not configured (empty). | ||
| // Returns an error if componentName is unsafe (absolute, contains path separators | ||
| // or traversal sequences). | ||
| func RenderedSpecDir(renderedSpecsDir, componentName string) (string, error) { | ||
| if err := fileutils.ValidateFilename(componentName); err != nil { | ||
| return "", fmt.Errorf("invalid component name for rendered spec dir:\n%w", err) | ||
| } | ||
|
|
||
| if renderedSpecsDir == "" { | ||
| return "", nil | ||
| } | ||
|
|
||
| return filepath.Join(renderedSpecsDir, componentName), nil | ||
| } | ||
69 changes: 69 additions & 0 deletions
69
internal/app/azldev/core/components/renderedspecdir_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package components_test | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestRenderedSpecDir(t *testing.T) { | ||
| t.Run("ReturnsPathWhenConfigured", func(t *testing.T) { | ||
| result, err := components.RenderedSpecDir("/path/to/specs", "vim") | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "/path/to/specs/vim", result) | ||
| }) | ||
|
|
||
| t.Run("ReturnsEmptyWhenNotConfigured", func(t *testing.T) { | ||
| result, err := components.RenderedSpecDir("", "vim") | ||
| require.NoError(t, err) | ||
| assert.Empty(t, result) | ||
| }) | ||
|
|
||
| t.Run("HandlesComponentNameWithDashes", func(t *testing.T) { | ||
| result, err := components.RenderedSpecDir("/rendered", "my-component") | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "/rendered/my-component", result) | ||
| }) | ||
|
|
||
| t.Run("RejectsAbsoluteComponentName", func(t *testing.T) { | ||
| _, err := components.RenderedSpecDir("/rendered", "/tmp") | ||
| assert.Error(t, err) | ||
| }) | ||
|
|
||
| t.Run("RejectsTraversalInComponentName", func(t *testing.T) { | ||
| _, err := components.RenderedSpecDir("/rendered", "../escape") | ||
| assert.Error(t, err) | ||
| }) | ||
|
|
||
| t.Run("RejectsPathSeparatorInComponentName", func(t *testing.T) { | ||
| _, err := components.RenderedSpecDir("/rendered", "sub/dir") | ||
| assert.Error(t, err) | ||
| }) | ||
|
|
||
| t.Run("RejectsEmptyComponentName", func(t *testing.T) { | ||
| _, err := components.RenderedSpecDir("/rendered", "") | ||
| assert.Error(t, err) | ||
| }) | ||
|
|
||
| t.Run("RejectsDotComponentName", func(t *testing.T) { | ||
| _, err := components.RenderedSpecDir("/rendered", ".") | ||
| assert.Error(t, err) | ||
| }) | ||
|
|
||
| t.Run("RejectsDotDotComponentName", func(t *testing.T) { | ||
| _, err := components.RenderedSpecDir("/rendered", "..") | ||
| assert.Error(t, err) | ||
| }) | ||
|
|
||
| t.Run("ValidatesEvenWhenNotConfigured", func(t *testing.T) { | ||
| // Component name is validated even when renderedSpecsDir is empty, | ||
| // so invalid names are caught early regardless of configuration. | ||
| _, err := components.RenderedSpecDir("", "../escape") | ||
| assert.Error(t, err) | ||
| }) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RenderedSpecDir relies on fileutils.ValidateFilename(componentName), but ValidateFilename does not reject Windows volume-qualified names (e.g. "C:tmp"). On Windows, filepath.Join(renderedSpecsDir, "C:tmp") can discard the base directory and produce a path on another volume, undermining the safety guarantee this helper is meant to provide. Consider rejecting componentName values where filepath.VolumeName(componentName) != "" (ideally inside ValidateFilename so all callers benefit).