Skip to content

Commit

Permalink
Better error handling and Handle potential permissions issue
Browse files Browse the repository at this point in the history
err could be nil, so it is checked before checking for the
type of error. This may not be needed, but is a better
default to establish since code is often copied by new
contributors when adding features.

Addresses comment by @rgee0 for handling error from stat.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
  • Loading branch information
alexellis committed Jan 23, 2025
1 parent 2a9b309 commit 452db1e
Show file tree
Hide file tree
Showing 13 changed files with 28 additions and 23 deletions.
2 changes: 1 addition & 1 deletion builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func BuildImage(image string, handler string, functionName string, language stri

if stack.IsValidTemplate(language) {
pathToTemplateYAML := fmt.Sprintf("./template/%s/template.yml", language)
if _, err := os.Stat(pathToTemplateYAML); os.IsNotExist(err) {
if _, err := os.Stat(pathToTemplateYAML); err != nil && os.IsNotExist(err) {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion builder/copy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func checkDestinationFiles(dir string, numberOfFiles, mode int) error {
// Check each file inside the destination folder
for i := 1; i <= numberOfFiles; i++ {
fileStat, err := os.Stat(fmt.Sprintf("%s/test-file-%d", dir, i))
if os.IsNotExist(err) {
if err != nil && os.IsNotExist(err) {
return err
}
if fileStat.Mode() != os.FileMode(mode) {
Expand Down
2 changes: 1 addition & 1 deletion builder/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func PublishImage(image string, handler string, functionName string, language st

if stack.IsValidTemplate(language) {
pathToTemplateYAML := fmt.Sprintf("./template/%s/template.yml", language)
if _, err := os.Stat(pathToTemplateYAML); os.IsNotExist(err) {
if _, err := os.Stat(pathToTemplateYAML); err != nil && os.IsNotExist(err) {
return err
}

Expand Down
18 changes: 12 additions & 6 deletions commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ func runBuild(cmd *cobra.Command, args []string) error {
}
} else {
templateAddress := getTemplateURL("", os.Getenv(templateURLEnvironment), DefaultTemplateRepository)
if pullErr := pullTemplates(templateAddress, templateName); pullErr != nil {
return fmt.Errorf("could not pull templates for OpenFaaS: %v", pullErr)
if err := pullTemplates(templateAddress, templateName); err != nil {
return fmt.Errorf("could not pull templates: %v", err)
}
}

Expand Down Expand Up @@ -307,12 +307,18 @@ func build(services *stack.Services, queueDepth int, shrinkwrap, quietBuild bool
// pullTemplates pulls templates from specified git remote. templateURL may be a pinned repository.
func pullTemplates(templateURL, templateName string) error {

if _, err := os.Stat("./template"); err != nil && os.IsNotExist(err) {
if _, err := os.Stat("./template"); err != nil {
if os.IsNotExist(err) {

fmt.Printf("No templates found in current directory.\n")
fmt.Printf("No templates found in current directory.\n")

templateURL, refName := versioncontrol.ParsePinnedRemote(templateURL)
if err := fetchTemplates(templateURL, refName, templateName, false); err != nil {
templateURL, refName := versioncontrol.ParsePinnedRemote(templateURL)
if err := fetchTemplates(templateURL, refName, templateName, false); err != nil {
return err
}
} else {

// Perhaps there was a permissions issue or something else.
return err
}
}
Expand Down
2 changes: 1 addition & 1 deletion commands/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ func deriveFprocess(function stack.Function) (string, error) {
}

pathToTemplateYAML := "./template/" + function.Language + "/template.yml"
if _, err := os.Stat(pathToTemplateYAML); os.IsNotExist(err) {
if _, err := os.Stat(pathToTemplateYAML); err != nil && os.IsNotExist(err) {
return "", err
}

Expand Down
2 changes: 1 addition & 1 deletion commands/new_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ Download templates:
}

pathToTemplateYAML := fmt.Sprintf("./template/%s/template.yml", language)
if _, err := os.Stat(pathToTemplateYAML); os.IsNotExist(err) {
if _, err := os.Stat(pathToTemplateYAML); err != nil && os.IsNotExist(err) {
return err
}

Expand Down
6 changes: 3 additions & 3 deletions commands/new_function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,18 +150,18 @@ func runNewFunctionTest(t *testing.T, nft NewFunctionTest) {
if nft.expectedMsg == SuccessMsg {

// Make sure that the folder and file was created:
if _, err := os.Stat("./" + dirName); os.IsNotExist(err) {
if _, err := os.Stat("./" + dirName); err != nil && os.IsNotExist(err) {
t.Fatalf("%s/ directory was not created", dirName)
}

// Check that the Dockerfile was created
if funcLang == "Dockerfile" || funcLang == "dockerfile" {
if _, err := os.Stat("./" + dirName + "/Dockerfile"); os.IsNotExist(err) {
if _, err := os.Stat("./" + dirName + "/Dockerfile"); err != nil && os.IsNotExist(err) {
t.Fatalf("Dockerfile language should create a Dockerfile for you: %s", funcName)
}
}

if _, err := os.Stat(funcYAML); os.IsNotExist(err) {
if _, err := os.Stat(funcYAML); err != nil && os.IsNotExist(err) {
t.Fatalf("\"%s\" yaml file was not created", funcYAML)
}

Expand Down
2 changes: 1 addition & 1 deletion commands/plugin_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func runPluginGetCmd(cmd *cobra.Command, args []string) error {
}
}

if _, err := os.Stat(pluginDir); os.IsNotExist(err) {
if _, err := os.Stat(pluginDir); err != nil && os.IsNotExist(err) {
if err := os.MkdirAll(pluginDir, 0755); err != nil && os.ErrExist != err {
return fmt.Errorf("failed to create plugin directory %s: %w", pluginDir, err)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func runPublish(cmd *cobra.Command, args []string) error {
}

if needTemplates {
if _, err := os.Stat("./template"); os.IsNotExist(err) {
if _, err := os.Stat("./template"); err != nil && os.IsNotExist(err) {

return fmt.Errorf(`the "template" directory is missing but required by at least one function`)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/registry_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func generateECRRegistryAuth(accountID, region string) ([]byte, error) {

func writeFileToFassCLITmp(fileBytes []byte) error {
path := "./credentials"
if _, err := os.Stat(path); os.IsNotExist(err) {
if _, err := os.Stat(path); err != nil && os.IsNotExist(err) {
err := os.Mkdir(path, 0744)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion commands/template_pull_stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func filterExistingTemplates(templateInfo []stack.TemplateSource, templatesDir s
var newTemplates []stack.TemplateSource
for _, info := range templateInfo {
templatePath := fmt.Sprintf("%s/%s", templatesDir, info.Name)
if _, err := os.Stat(templatePath); os.IsNotExist(err) {
if _, err := os.Stat(templatePath); err != nil && os.IsNotExist(err) {
newTemplates = append(newTemplates, info)
}
}
Expand Down
6 changes: 3 additions & 3 deletions config/config_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func EnsureFile() (string, error) {
return "", err
}

if _, err := os.Stat(filePath); os.IsNotExist(err) {
if _, err := os.Stat(filePath); err != nil && os.IsNotExist(err) {
file, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
if err != nil {
return "", err
Expand All @@ -155,7 +155,7 @@ func fileExists() bool {
}

filePath := path.Clean(filepath.Join(dirPath, DefaultFile))
if _, err := os.Stat(filePath); os.IsNotExist(err) {
if _, err := os.Stat(filePath); err != nil && os.IsNotExist(err) {
return false
}

Expand Down Expand Up @@ -185,7 +185,7 @@ func (configFile *ConfigFile) save() error {
func (configFile *ConfigFile) load() error {
conf := &ConfigFile{}

if _, err := os.Stat(configFile.FilePath); os.IsNotExist(err) {
if _, err := os.Stat(configFile.FilePath); err != nil && os.IsNotExist(err) {
return fmt.Errorf("can't load config from non existent filePath")
}

Expand Down
3 changes: 1 addition & 2 deletions config/config_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,7 @@ func Test_EnsureFile(t *testing.T) {
if err != nil {
t.Error(err.Error())
}
_, err = os.Stat(cfg)
if os.IsNotExist(err) {
if _, err = os.Stat(cfg); err != nil && os.IsNotExist(err) {
t.Errorf("expected config at %s", cfg)
}
}
Expand Down

0 comments on commit 452db1e

Please sign in to comment.