Skip to content

fix(workspace): ignore unreadable directories when listing Dockerfiles #1764

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

Merged
mergify merged 2 commits into from
Dec 18, 2020
Merged

fix(workspace): ignore unreadable directories when listing Dockerfiles #1764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
262fdbd
fix(workspace): ignore unreadable directories when searching for dock…
bvtujo Dec 18, 2020
80b9b3e
fix(prompt): add new noInterrupt() option to prevent segfaults on SIG…
bvtujo Dec 18, 2020
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
16 changes: 10 additions & 6 deletions internal/pkg/term/prompt/prompt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ func (p Prompt) Get(message, help string, validator ValidatorFunc, promptOpts ..
var result string
var err error
if validator == nil {
err = p(prompt, &result, stdio(), icons())
err = p(prompt, &result, stdio(), icons(), noInterrupt())
Copy link
Copy Markdown
Contributor

@iamhopaul123 iamhopaul123 Dec 18, 2020

Choose a reason for hiding this comment

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

Good catch!

} else {
err = p(prompt, &result, stdio(), validators(validator), icons())
err = p(prompt, &result, stdio(), validators(validator), icons(), noInterrupt())
}
return result, err
}
Expand Down Expand Up @@ -220,7 +220,7 @@ func (p Prompt) GetSecret(message, help string, promptOpts ...Option) (string, e
}

var result string
err := p(prompt, &result, stdio(), icons())
err := p(prompt, &result, stdio(), icons(), noInterrupt())
return result, err
}

Expand Down Expand Up @@ -248,7 +248,7 @@ func (p Prompt) SelectOne(message, help string, options []string, promptOpts ...
}

var result string
err := p(prompt, &result, stdio(), icons())
err := p(prompt, &result, stdio(), icons(), noInterrupt())
return result, err
}

Expand All @@ -275,7 +275,7 @@ func (p Prompt) MultiSelect(message, help string, options []string, promptOpts .
option(prompt)
}

err := p(prompt, &result, stdio(), icons())
err := p(prompt, &result, stdio(), icons(), noInterrupt())
return result, err
}

Expand All @@ -296,7 +296,7 @@ func (p Prompt) Confirm(message, help string, promptOpts ...Option) (bool, error
}

var result bool
err := p(prompt, &result, stdio(), icons())
err := p(prompt, &result, stdio(), icons(), noInterrupt())
return result, err
}

Expand Down Expand Up @@ -332,6 +332,10 @@ func stdio() survey.AskOpt {
return survey.WithStdio(os.Stdin, os.Stderr, os.Stderr)
}

func noInterrupt() survey.AskOpt {
return survey.WithInterruptFunc(func() {})
}

func icons() survey.AskOpt {
return survey.WithIcons(func(icons *survey.IconSet) {
// The question mark "?" icon to denote a prompt will be colored in bold.
Expand Down
10 changes: 5 additions & 5 deletions internal/pkg/term/prompt/prompt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestPrompt_Get(t *testing.T) {

*result = mockInput

require.Equal(t, 3, len(opts))
require.Equal(t, 4, len(opts))

return nil
},
Expand Down Expand Up @@ -100,7 +100,7 @@ func TestPrompt_GetSecret(t *testing.T) {

*result = mockSecret

require.Equal(t, 2, len(opts))
require.Equal(t, 3, len(opts))

return nil
},
Expand Down Expand Up @@ -154,7 +154,7 @@ func TestPrompt_SelectOne(t *testing.T) {

*result = sel.Options[0]

require.Equal(t, 2, len(opts))
require.Equal(t, 3, len(opts))

return nil
},
Expand Down Expand Up @@ -215,7 +215,7 @@ func TestPrompt_MultiSelect(t *testing.T) {

*result = sel.Options

require.Equal(t, 2, len(opts))
require.Equal(t, 3, len(opts))

return nil
},
Expand Down Expand Up @@ -275,7 +275,7 @@ func TestPrompt_Confirm(t *testing.T) {

*result = true

require.Equal(t, 2, len(opts))
require.Equal(t, 3, len(opts))

return nil
},
Expand Down
3 changes: 2 additions & 1 deletion internal/pkg/workspace/workspace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ func (ws *Workspace) ListDockerfiles() ([]string, error) {
// Add sub-directories containing a Dockerfile one level below current directory.
subFiles, err := ws.fsUtils.ReadDir(wdFile.Name())
if err != nil {
return nil, fmt.Errorf("read directory: %w", err)
// swallow errors for unreadable directories
continue
Copy link
Copy Markdown
Contributor

@iamhopaul123 iamhopaul123 Dec 18, 2020

Choose a reason for hiding this comment

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

Do we need to update unit tests for this change?

Copy link
Copy Markdown
Contributor Author

@bvtujo bvtujo Dec 18, 2020

Choose a reason for hiding this comment

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

There are currently no unit tests reliant on it, apparently. I'll add one in my next PR.

Copy link
Copy Markdown
Contributor Author

@bvtujo bvtujo Dec 18, 2020

Choose a reason for hiding this comment

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

It turns out that afero does not check permissions when opening files. See spf13/afero#150 for others complaining about this.

}
for _, f := range subFiles {
// NOTE: ignore directories in sub-directories.
Expand Down