From e574a2e2a869b7d3fa02f48da7941db8a8f8c975 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Tue, 3 Dec 2024 14:37:13 +0100 Subject: [PATCH] chore(errors): Better error management We must remove all "Fatal" calls and use errors instead, to be returned and managed globally. This is the first step, but it is, at this time, a real problem. Tests are complicated without this. --- cmd/katenary/main.go | 4 ++-- generator/configMap.go | 21 ++++++++++++--------- generator/configMap_test.go | 17 +++++++++++++++++ generator/converter.go | 13 +++++++------ generator/tools_test.go | 4 +++- 5 files changed, 41 insertions(+), 18 deletions(-) diff --git a/cmd/katenary/main.go b/cmd/katenary/main.go index dc0c9ca..8a88969 100644 --- a/cmd/katenary/main.go +++ b/cmd/katenary/main.go @@ -141,11 +141,11 @@ func generateConvertCommand() *cobra.Command { convertCmd := &cobra.Command{ Use: "convert", Short: "Converts a docker-compose file to a Helm Chart", - Run: func(cmd *cobra.Command, args []string) { + RunE: func(cmd *cobra.Command, args []string) error { if givenAppVersion != "" { appVersion = &givenAppVersion } - generator.Convert(generator.ConvertOptions{ + return generator.Convert(generator.ConvertOptions{ Force: force, OutputDir: outputDir, Profiles: profiles, diff --git a/generator/configMap.go b/generator/configMap.go index 61cfd23..5d79449 100644 --- a/generator/configMap.go +++ b/generator/configMap.go @@ -1,6 +1,7 @@ package generator import ( + "fmt" "katenary/generator/labels" "katenary/generator/labels/labelStructs" "katenary/utils" @@ -141,7 +142,7 @@ func NewConfigMapFromDirectory(service types.ServiceConfig, appName, path string // cumulate the path to the WorkingDir path = filepath.Join(service.WorkingDir, path) path = filepath.Clean(path) - cm.AppenddDir(path) + cm.AppendDir(path) return cm } @@ -160,17 +161,17 @@ func (c *ConfigMap) AddBinaryData(key string, value []byte) { // AddFile adds files from given path to the configmap. It is not recursive, to add all files in a directory, // you need to call this function for each subdirectory. -func (c *ConfigMap) AppenddDir(path string) { +func (c *ConfigMap) AppendDir(path string) error { // read all files in the path and add them to the configmap stat, err := os.Stat(path) if err != nil { - log.Fatalf("Path %s does not exist\n", path) + return fmt.Errorf("Path %s does not exist, %w\n", path, err) } // recursively read all files in the path and add them to the configmap if stat.IsDir() { files, err := os.ReadDir(path) if err != nil { - log.Fatal(err) + return err } for _, file := range files { if file.IsDir() { @@ -180,7 +181,7 @@ func (c *ConfigMap) AppenddDir(path string) { path := filepath.Join(path, file.Name()) content, err := os.ReadFile(path) if err != nil { - log.Fatal(err) + return err } // remove the path from the file filename := filepath.Base(path) @@ -195,7 +196,7 @@ func (c *ConfigMap) AppenddDir(path string) { // add the file to the configmap content, err := os.ReadFile(path) if err != nil { - log.Fatal(err) + return err } filename := filepath.Base(path) if utf8.Valid(content) { @@ -204,20 +205,21 @@ func (c *ConfigMap) AppenddDir(path string) { c.AddBinaryData(filename, content) } } + return nil } -func (c *ConfigMap) AppendFile(path string) { +func (c *ConfigMap) AppendFile(path string) error { // read all files in the path and add them to the configmap stat, err := os.Stat(path) if err != nil { - log.Fatalf("Path %s does not exist\n", path) + return fmt.Errorf("Path %s doesn not exists, %w", path, err) } // recursively read all files in the path and add them to the configmap if !stat.IsDir() { // add the file to the configmap content, err := os.ReadFile(path) if err != nil { - log.Fatal(err) + return err } if utf8.Valid(content) { c.AddData(filepath.Base(path), string(content)) @@ -226,6 +228,7 @@ func (c *ConfigMap) AppendFile(path string) { } } + return nil } // Filename returns the filename of the configmap. If the configmap is used for files, the filename contains the path. diff --git a/generator/configMap_test.go b/generator/configMap_test.go index a0b5f80..6e880e6 100644 --- a/generator/configMap_test.go +++ b/generator/configMap_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + "github.com/compose-spec/compose-go/types" v1 "k8s.io/api/core/v1" "sigs.k8s.io/yaml" ) @@ -73,3 +74,19 @@ services: t.Errorf("Expected FOO to be baz, got %s", v) } } + +func TestAppendBadFile(t *testing.T) { + cm := NewConfigMap(types.ServiceConfig{}, "app", true) + err := cm.AppendFile("foo") + if err == nil { + t.Errorf("Expected error, got nil") + } +} + +func TestAppendBadDir(t *testing.T) { + cm := NewConfigMap(types.ServiceConfig{}, "app", true) + err := cm.AppendDir("foo") + if err == nil { + t.Errorf("Expected error, got nil") + } +} diff --git a/generator/converter.go b/generator/converter.go index cd0d11d..1d55e46 100644 --- a/generator/converter.go +++ b/generator/converter.go @@ -90,7 +90,7 @@ var keyRegExp = regexp.MustCompile(`^\s*[^#]+:.*`) // Convert a compose (docker, podman...) project to a helm chart. // It calls Generate() to generate the chart and then write it to the disk. -func Convert(config ConvertOptions, dockerComposeFile ...string) { +func Convert(config ConvertOptions, dockerComposeFile ...string) error { var ( templateDir = filepath.Join(config.OutputDir, "templates") helpersPath = filepath.Join(config.OutputDir, "templates", "_helpers.tpl") @@ -105,7 +105,7 @@ func Convert(config ConvertOptions, dockerComposeFile ...string) { // go to the root of the project if err := os.Chdir(filepath.Dir(dockerComposeFile[0])); err != nil { fmt.Println(utils.IconFailure, err) - os.Exit(1) + return err } defer os.Chdir(currentDir) // after the generation, go back to the original directory @@ -118,13 +118,13 @@ func Convert(config ConvertOptions, dockerComposeFile ...string) { project, err := parser.Parse(config.Profiles, config.EnvFiles, dockerComposeFile...) if err != nil { fmt.Println(err) - os.Exit(1) + return err } // check older version of labels if err := checkOldLabels(project); err != nil { fmt.Println(utils.IconFailure, err) - os.Exit(1) + return err } // TODO: use katenary.yaml file here to set the labels @@ -140,7 +140,7 @@ func Convert(config ConvertOptions, dockerComposeFile ...string) { ) if !overwrite { fmt.Println("Aborting") - os.Exit(126) // 126 is the exit code for "Command invoked cannot execute" + return nil } } fmt.Println() // clean line @@ -150,7 +150,7 @@ func Convert(config ConvertOptions, dockerComposeFile ...string) { chart, err := Generate(project) if err != nil { fmt.Println(err) - os.Exit(1) + return err } // if the app version is set from the command line, use it @@ -194,6 +194,7 @@ func Convert(config ConvertOptions, dockerComposeFile ...string) { // call helm update if needed callHelmUpdate(config) + return nil } func addChartDoc(values []byte, project *types.Project) []byte { diff --git a/generator/tools_test.go b/generator/tools_test.go index 9d1591e..2395411 100644 --- a/generator/tools_test.go +++ b/generator/tools_test.go @@ -48,7 +48,9 @@ func internalCompileTest(t *testing.T, options ...string) string { AppVersion: appVersion, ChartVersion: chartVersion, } - Convert(convertOptions, "compose.yml") + if err := Convert(convertOptions, "compose.yml"); err != nil { + return err.Error() + } // launch helm lint to check the generated chart if helmLint(convertOptions) != nil {