From 405ad6c379757fd094474ad650c78201985df9c2 Mon Sep 17 00:00:00 2001 From: AtomicFS Date: Mon, 2 Dec 2024 16:12:10 +0100 Subject: [PATCH] feat(action): check for any undefined environment variable in config - until now it was rather hard to debug when the issue was one or more missing environment variables - with this patch, all environment detected variables in the JSON configuration file must be defined at the time of execution Signed-off-by: AtomicFS --- action/recipes/config.go | 40 ++++++++++++++++++++-- action/recipes/config_test.go | 64 +++++++++++++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 6 deletions(-) diff --git a/action/recipes/config.go b/action/recipes/config.go index 0b52d46f..91aa23b1 100644 --- a/action/recipes/config.go +++ b/action/recipes/config.go @@ -11,6 +11,7 @@ import ( "log/slog" "os" "path/filepath" + "regexp" "strings" "dagger.io/dagger" @@ -19,8 +20,12 @@ import ( "github.com/go-playground/validator/v10" ) -// ErrVerboseJSON is raised when JSONVerboseError can't find location of problem in JSON configuration file -var ErrVerboseJSON = errors.New("unable to pinpoint the problem in JSON file") +var ( + // ErrVerboseJSON is raised when JSONVerboseError can't find location of problem in JSON configuration file + ErrVerboseJSON = errors.New("unable to pinpoint the problem in JSON file") + // ErrEnvVarUndefined is raised when undefined environment variable is found in JSON configuration file + ErrEnvVarUndefined = errors.New("environment variable used in JSON file is not present in the environment") +) // ================= // Data structures @@ -221,6 +226,16 @@ func ValidateConfig(conf Config) error { return nil } +// FindAllEnvVars returns all environment variables found in the provided string +func FindAllEnvVars(text string) []string { + pattern := regexp.MustCompile(`\${?([a-zA-Z0-9_]+)}?`) + result := pattern.FindAllString(text, -1) + for index, value := range result { + result[index] = pattern.ReplaceAllString(value, "$1") + } + return result +} + // ReadConfig is for reading and parsing JSON configuration file into Config struct func ReadConfig(filepath string) (*Config, error) { // Read JSON file @@ -233,8 +248,27 @@ func ReadConfig(filepath string) (*Config, error) { return nil, err } - // Expand environment variables contentStr := string(content) + + // Check if all environment variables are defined + envVars := FindAllEnvVars(contentStr) + undefinedVarFound := false + for _, envVar := range envVars { + _, found := os.LookupEnv(envVar) + if !found { + slog.Error( + fmt.Sprintf("environment variable '%s' is undefined", envVar), + slog.String("suggestion", "define the environment variable in the environment"), + slog.Any("error", ErrEnvVarUndefined), + ) + undefinedVarFound = true + } + } + if undefinedVarFound { + return nil, ErrEnvVarUndefined + } + + // Expand environment variables contentStr = os.ExpandEnv(contentStr) // Decode JSON diff --git a/action/recipes/config_test.go b/action/recipes/config_test.go index 0111c4c2..e67ba143 100644 --- a/action/recipes/config_test.go +++ b/action/recipes/config_test.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -120,6 +121,53 @@ func TestConfigReadAndWrite(t *testing.T) { } } +func TestFindAllEnvVars(t *testing.T) { + testCases := []struct { + name string + text string + expectedEnvVars []string + }{ + { + name: "no env vars", + text: "dummy string", + expectedEnvVars: []string{}, + }, + { + name: "one env var", + text: "dummy string with $MY_VAR", + expectedEnvVars: []string{"MY_VAR"}, + }, + { + name: "one env var with brackets", + text: "dummy string with ${MY_VAR}", + expectedEnvVars: []string{"MY_VAR"}, + }, + { + name: "two env vars", + text: "dummy string with $MY_VAR and ${MY_VAR}", + expectedEnvVars: []string{"MY_VAR", "MY_VAR"}, + }, + { + name: "two env vars with numbers", + text: "dummy string with $MY_VAR1 and ${MY_VAR2}", + expectedEnvVars: []string{"MY_VAR1", "MY_VAR2"}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + foundVars := FindAllEnvVars(tc.text) + fmt.Println(foundVars) + fmt.Println(tc.expectedEnvVars) + + assert.Equal(t, len(tc.expectedEnvVars), len(foundVars)) + // If both slices are of zero length, then the comparison fails for whatever reason + if len(tc.expectedEnvVars) > 0 { + assert.True(t, reflect.DeepEqual(tc.expectedEnvVars, foundVars)) + } + }) + } +} + func TestConfigEnvVars(t *testing.T) { commonDummy := CommonOpts{ SdkURL: "ghcr.io/9elements/firmware-action/coreboot_4.19:main", @@ -182,6 +230,15 @@ func TestConfigEnvVars(t *testing.T) { "TEST_ENV_VAR_REPOPATH": "dummy/dir/", }, }, + { + name: "undefined env var", + wantErr: ErrEnvVarUndefined, + url: "ghcr.io/$TEST_ENV_VAR/coreboot_4.19:main", + urlExpected: commonDummy.SdkURL, + repoPath: commonDummy.RepoPath, + repoPathExpected: commonDummy.RepoPath, + envVars: map[string]string{}, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -213,12 +270,13 @@ func TestConfigEnvVars(t *testing.T) { assert.NoError(t, err) // Read optsConverted, err := ReadConfig(configFilepath) - assert.NoError(t, err) // err = ValidateConfig(optsConverted) assert.ErrorIs(t, err, tc.wantErr) - assert.Equal(t, tc.urlExpected, optsConverted.Coreboot["coreboot-A"].SdkURL) - assert.Equal(t, tc.repoPathExpected, optsConverted.Coreboot["coreboot-A"].RepoPath) + if tc.wantErr == nil { + assert.Equal(t, tc.urlExpected, optsConverted.Coreboot["coreboot-A"].SdkURL) + assert.Equal(t, tc.repoPathExpected, optsConverted.Coreboot["coreboot-A"].RepoPath) + } }) } }