Change logger info to errors where appropriate (#62)

* Initial commit

* Error message for basic requirements

* Only check registries on push

* Update from save

* Update error handling

* Fix error handle

* Update golangci-lint version

* Update tests

* Update registry tests

* Update to main branch

* Remove test project filepath

* Rename registries to registry_url and add documentation

* Add missing comments

* Add more to the comments

* Comment addition

.github/workflows/build.yaml

@@ -17,7 +17,7 @@ jobs:
       - name: Run golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.49
+          version: v1.52.2
 
   test:
     runs-on: ubuntu-latest

internal/act/act.go

@@ -2,6 +2,7 @@ package act
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	log2 "log"
 	"os"
@@ -68,9 +69,12 @@ func AllTrue(checks []bool) bool {
 }
 
 func CliACT(build bool, push bool, logger log.Logger, cec_choice string) error {
-	conf, err := dto.Unmarshal(logger)
+	conf, err := dto.Unmarshal(push, logger)
 	if err != nil {
-		return fmt.Errorf("error in act configuration file (%w)", err)
+		return fmt.Errorf("error found in configuration: (%w)", err)
+	}
+	if push && len(conf.Registries) == 0 {
+		return errors.New("no registries passed configuration with the push argument")
 	}
 	cleanpath := filepath.Clean(conf.Project_Filepath)
 	abspath, err := filepath.Abs(cleanpath)

internal/dto/act.go

@@ -18,10 +18,14 @@ type ACT struct {
 	Registries       []Registry
 }
 
-func Unmarshal(logger log.Logger) (ACT, error) {
-	filteredRegistries, err := UnmarshalRegistries(logger)
-	if err != nil {
-		return ACT{}, err
+func Unmarshal(push bool, logger log.Logger) (ACT, error) {
+	var registries Registries
+	if push {
+		filteredRegistries, err := UnmarshalRegistries(logger)
+		if err != nil {
+			return ACT{}, err
+		}
+		registries = filteredRegistries
 	}
 	conf := ACT{
 		Revision:         viper.GetString("revision"),
@@ -30,7 +34,7 @@ func Unmarshal(logger log.Logger) (ACT, error) {
 		Image_Tag:        viper.GetString("image_tag"),
 		Quay_Img_Exp:     viper.GetString("quay_img_exp"),
 		Build_Timeout:    viper.GetUint32("build_timeout"),
-		Registries:       filteredRegistries}
+		Registries:       registries}
 	if err := defaults.Set(&conf); err != nil {
 		return ACT{}, fmt.Errorf("error setting defaults (%w)", err)
 	}

internal/dto/env.go

@@ -2,23 +2,22 @@ package dto
 
 import (
 	"fmt"
-	"go.arcalot.io/log"
 	"os"
+
+	"go.arcalot.io/log"
 )
 
-func LookupEnvVar(key string, logger log.Logger) Verbose {
+// LookupEnvVar returns the value of an enviornment variable.
+// lookupEnvVar will return an error if the enviornment variable is not set or empty.
+// The error includes the registry url to destinguish which registry encountered the error if found.
+func LookupEnvVar(registry_url string, key string, logger log.Logger) (string, error) {
 	val, ok := os.LookupEnv(key)
-	var msg string
 	if !ok {
-		msg = fmt.Sprintf("%s not set", key)
+		err := fmt.Errorf("%s environment variable not set to push to %s", key, registry_url)
+		return "", err
 	} else if len(val) == 0 {
-		msg = fmt.Sprintf("%s is empty", key)
+		err := fmt.Errorf("%s environment variable empty to push to %s", key, registry_url)
+		return "", err
 	}
-	logger.Infof(msg)
-	return Verbose{Return_value: val, Msg: msg}
-}
-
-type Verbose struct {
-	Msg          string
-	Return_value string
+	return val, nil
 }

internal/dto/env_test.go

@@ -1,7 +1,6 @@
 package dto_test
 
 import (
-	"fmt"
 	"log"
 	"os"
 	"testing"
@@ -12,30 +11,34 @@ import (
 )
 
 func TestLookupEnvVar(t *testing.T) {
+	registries := "test"
 	logger := arcalog.NewLogger(arcalog.LevelInfo, arcalog.NewNOOPLogger())
-	envvar_key := "i_hope_this_isnt_used"
-	envvar_val := ""
+	envvar_key := "foo"
+	envvar_val := "bar"
 
-	v := dto.LookupEnvVar(envvar_key, logger)
-	assert.Equals(t, v.Msg, fmt.Sprintf("%s not set", envvar_key))
-	assert.Equals(t, v.Return_value, "")
+	_, err := dto.LookupEnvVar(registries, envvar_key, logger)
+	assert.Error(t, err)
 
-	err := os.Setenv(envvar_key, envvar_val)
+	err = os.Setenv(envvar_key, envvar_val)
+	if err != nil {
+		log.Fatal(err)
+	}
+	v, err := dto.LookupEnvVar(registries, envvar_key, logger)
 	if err != nil {
 		log.Fatal(err)
 	}
-	v = dto.LookupEnvVar(envvar_key, logger)
-	assert.Equals(t, v.Msg, fmt.Sprintf("%s is empty", envvar_key))
-	assert.Equals(t, v.Return_value, "")
+	assert.Equals(t, v, envvar_val)
 
 	envvar_val = "robot"
 	err = os.Setenv(envvar_key, envvar_val)
 	if err != nil {
 		log.Fatal(err)
 	}
-	v = dto.LookupEnvVar(envvar_key, logger)
-	assert.Equals(t, v.Msg, "")
-	assert.Equals(t, v.Return_value, envvar_val)
+	v, err = dto.LookupEnvVar(registries, envvar_key, logger)
+	if err != nil {
+		log.Fatal(err)
+	}
+	assert.Equals(t, v, "robot")
 
 	err = os.Unsetenv(envvar_key)
 	if err != nil {

internal/dto/registry.go

@@ -51,7 +51,7 @@ func UnmarshalRegistries(logger log.Logger) ([]Registry, error) {
 	var registries Registries
 	err := viper.UnmarshalKey("registries", &registries)
 	if err != nil {
-		return nil, fmt.Errorf("error unmarshalling registries from act file (%w)", err)
+		return nil, fmt.Errorf("error unmarshalling registries from act file ((%w))", err)
 	}
 	return registries.Parse(logger)
 }
@@ -64,26 +64,48 @@ func (registries Registries) Parse(logger log.Logger) (Registries, error) {
 		password_envvar := registries[i].Password_Envvar
 		namespace_envvar := registries[i].Namespace_Envvar
 		quay_custom_namespace_envvar := registries[i].Quay_Custom_Namespace_Envvar
-		username := LookupEnvVar(username_envvar, logger).Return_value
-		password := LookupEnvVar(password_envvar, logger).Return_value
-		namespace := LookupEnvVar(namespace_envvar, logger).Return_value
-		quay_custom_namespace := LookupEnvVar(quay_custom_namespace_envvar, logger).Return_value
+		quay_custom_namespace, _ := LookupEnvVar(registries[i].Url, quay_custom_namespace_envvar, logger)
+		if quay_custom_namespace != "" && registries[i].Url == "quay.io" {
+			logger.Infof("value is:%s", quay_custom_namespace_envvar)
+			logger.Infof("QUAY_CUSTOM_NAMESPACE environment variable detected,"+
+				"using value in place of QUAY_NAMESPACE for %s", registries[i].Url)
+			namespace_envvar = quay_custom_namespace_envvar
+		}
+		username, err := LookupEnvVar(registries[i].Url, username_envvar, logger)
+		if err != nil {
+			logger.Errorf("Push argument detected but error found. Not attempting to build or push"+
+				" to %s from error: (%w)", registries[i].Url, err)
+			misconfigured_registries[strconv.FormatInt(int64(i), 10)] = PlaceHolder
+			continue
+		}
+		password, err := LookupEnvVar(registries[i].Url, password_envvar, logger)
+		if err != nil {
+			logger.Errorf("Push argument detected but error found. Not attempting to build or push"+
+				" to %s from error: (%w)", registries[i].Url, err)
+			misconfigured_registries[strconv.FormatInt(int64(i), 10)] = PlaceHolder
+			continue
+		}
+		namespace, err := LookupEnvVar(registries[i].Url, namespace_envvar, logger)
+		if err != nil {
+			logger.Errorf("Push argument detected but error found. Not attempting to build or push"+
+				" to %s from error: (%w)", registries[i].Url, err)
+			misconfigured_registries[strconv.FormatInt(int64(i), 10)] = PlaceHolder
+			continue
+		}
 		if !registries[i].ValidCredentials(username) {
-			logger.Infof("Missing credentials for %s\n", registries[i].Url)
+			logger.Errorf("Missing credentials for %s\n", registries[i].Url)
 			misconfigured_registries[strconv.FormatInt(int64(i), 10)] = PlaceHolder
 			continue
 		}
 		registries[i].Username = username
 		registries[i].Password = password
-		if quay_custom_namespace != "" && registries[i].Url == "quay.io" {
-			registries[i].Namespace = quay_custom_namespace
-		} else {
-			inferred_namespace, err := InferNamespace(namespace, username)
-			if err != nil {
-				return nil, err
-			}
-			registries[i].Namespace = inferred_namespace
+
+		inferred_namespace, err := InferNamespace(namespace, username)
+		if err != nil {
+			return nil, err
 		}
+		registries[i].Namespace = inferred_namespace
+
 	}
 	filteredRegistries := FilterByIndex(registries, misconfigured_registries)
 	return filteredRegistries, nil

internal/dto/registry_test.go

@@ -69,10 +69,10 @@ func TestRegistries_Parse(t *testing.T) {
 	envvars := map[string]string{
 		"reg1_username":  "reg1_username",
 		"reg1_password":  "reg1_password",
-		"reg1_namespace": "",
+		"reg1_namespace": "reg1_namespace",
 		"reg2_username":  "reg2_username+robot",
 		"reg2_password":  "reg2_password",
-		"reg2_namespace": "",
+		"reg2_namespace": "reg2_namespace",
 	}
 
 	reg2_namespace := envvars["reg2_namespace"]
@@ -108,13 +108,13 @@ func TestRegistries_Parse(t *testing.T) {
 	assert.Equals(t, v, rs2[0].Username)
 	v = envvars["reg1_password"]
 	assert.Equals(t, v, rs2[0].Password)
-	assert.Equals(t, "reg1_username", rs2[0].Namespace)
+	assert.Equals(t, "reg1_namespace", rs2[0].Namespace)
 
 	v = envvars["reg2_username"]
 	assert.Equals(t, v, rs2[1].Username)
 	v = envvars["reg2_password"]
 	assert.Equals(t, v, rs2[1].Password)
-	assert.Equals(t, "reg2_username", rs2[1].Namespace)
+	assert.Equals(t, "reg2_namespace", rs2[1].Namespace)
 
 	for envvar_key := range envvars {
 		err := os.Unsetenv(envvar_key)

internal/requirements/basic.go

@@ -4,32 +4,27 @@ import "go.arcalot.io/log"
 
 func BasicRequirements(filenames []string, logger log.Logger) (bool, error) {
 	meets_reqs := true
-	output := ""
 
 	if present, err := HasFilename(filenames, "README.md"); err != nil {
 		return false, err
 	} else if !present {
-		output = "Missing README.md\n"
-		logger.Infof(output)
+		logger.Errorf("Missing required file README.md")
 		meets_reqs = false
 	}
 
 	if present, err := HasFilename(filenames, "Dockerfile"); err != nil {
 		return false, err
 	} else if !present {
-		output = "Missing Dockerfile\n"
-		logger.Infof(output)
+		logger.Errorf("Missing required file Dockerfile")
 		meets_reqs = false
 	}
 
 	if present, err := HasFilename(filenames, "(?i).*test.*"); err != nil {
 		return false, err
 	} else if !present {
 		// match case-insensitive 'test'?
-		output = "Missing a test file\n"
-		logger.Infof(output)
+		logger.Errorf("Missing required test file")
 		meets_reqs = false
 	}
-
 	return meets_reqs, nil
 }