From c249c6ae9c6d8d291815a328900c3760d4730ed1 Mon Sep 17 00:00:00 2001 From: decentral1se Date: Mon, 9 Oct 2023 14:37:20 +0200 Subject: [PATCH] fix: fix: trim comments that are not modifers See https://git.coopcloud.tech/coop-cloud/organising/issues/505 --- cli/app/new.go | 6 ++++-- cli/app/secret.go | 4 ++-- pkg/compose/compose.go | 4 ++-- pkg/config/app.go | 2 +- pkg/config/env.go | 11 +++++++--- pkg/config/env_test.go | 35 +++++++++++++++++++++++++++---- pkg/lint/recipe.go | 2 +- pkg/recipe/recipe.go | 6 +++--- pkg/secret/secret.go | 9 ++++++-- pkg/secret/secret_test.go | 5 +++-- tests/integration/app_deploy.bats | 9 ++++++++ 11 files changed, 71 insertions(+), 22 deletions(-) diff --git a/cli/app/new.go b/cli/app/new.go index 34ee69ea..6ff12e6e 100644 --- a/cli/app/new.go +++ b/cli/app/new.go @@ -2,6 +2,7 @@ package app import ( "fmt" + "path" "coopcloud.tech/abra/cli/internal" "coopcloud.tech/abra/pkg/autocomplete" @@ -96,7 +97,7 @@ var appNewCommand = cli.Command{ var secrets AppSecrets var secretTable *jsontable.JSONTable if internal.Secrets { - sampleEnv, err := recipe.SampleEnv() + sampleEnv, err := recipe.SampleEnv(config.ReadEnvOptions{}) if err != nil { logrus.Fatal(err) } @@ -106,7 +107,8 @@ var appNewCommand = cli.Command{ logrus.Fatal(err) } - secretsConfig, err := secret.ReadSecretsConfig(sampleEnv, composeFiles, recipe.Name) + envSamplePath := path.Join(config.RECIPES_DIR, recipe.Name, ".env.sample") + secretsConfig, err := secret.ReadSecretsConfig(envSamplePath, composeFiles, recipe.Name) if err != nil { return err } diff --git a/cli/app/secret.go b/cli/app/secret.go index 44385867..c0bf460b 100644 --- a/cli/app/secret.go +++ b/cli/app/secret.go @@ -87,7 +87,7 @@ var appSecretGenerateCommand = cli.Command{ logrus.Fatal(err) } - secretsConfig, err := secret.ReadSecretsConfig(app.Env, composeFiles, app.Recipe) + secretsConfig, err := secret.ReadSecretsConfig(app.Path, composeFiles, app.Recipe) if err != nil { logrus.Fatal(err) } @@ -276,7 +276,7 @@ Example: logrus.Fatal(err) } - secretsConfig, err := secret.ReadSecretsConfig(app.Env, composeFiles, app.Recipe) + secretsConfig, err := secret.ReadSecretsConfig(app.Path, composeFiles, app.Recipe) if err != nil { logrus.Fatal(err) } diff --git a/pkg/compose/compose.go b/pkg/compose/compose.go index 3fd98191..86c4dfb4 100644 --- a/pkg/compose/compose.go +++ b/pkg/compose/compose.go @@ -29,7 +29,7 @@ func UpdateTag(pattern, image, tag, recipeName string) (bool, error) { opts := stack.Deploy{Composefiles: []string{composeFile}} envSamplePath := path.Join(config.RECIPES_DIR, recipeName, ".env.sample") - sampleEnv, err := config.ReadEnv(envSamplePath) + sampleEnv, err := config.ReadEnv(envSamplePath, config.ReadEnvOptions{}) if err != nil { return false, err } @@ -97,7 +97,7 @@ func UpdateLabel(pattern, serviceName, label, recipeName string) error { opts := stack.Deploy{Composefiles: []string{composeFile}} envSamplePath := path.Join(config.RECIPES_DIR, recipeName, ".env.sample") - sampleEnv, err := config.ReadEnv(envSamplePath) + sampleEnv, err := config.ReadEnv(envSamplePath, config.ReadEnvOptions{}) if err != nil { return err } diff --git a/pkg/config/app.go b/pkg/config/app.go index 7082db79..c5426786 100644 --- a/pkg/config/app.go +++ b/pkg/config/app.go @@ -150,7 +150,7 @@ func (a ByName) Less(i, j int) bool { } func ReadAppEnvFile(appFile AppFile, name AppName) (App, error) { - env, err := ReadEnv(appFile.Path) + env, err := ReadEnv(appFile.Path, ReadEnvOptions{}) if err != nil { return App{}, fmt.Errorf("env file for %s couldn't be read: %s", name, err.Error()) } diff --git a/pkg/config/env.go b/pkg/config/env.go index 923b406a..f6fbbd51 100644 --- a/pkg/config/env.go +++ b/pkg/config/env.go @@ -55,6 +55,11 @@ func GetServers() ([]string, error) { return servers, nil } +// ReadEnvOptions modifies the ReadEnv processing of env vars. +type ReadEnvOptions struct { + IncludeModifiers bool +} + // ContainsEnvVarModifier determines if an env var contains a modifier. func ContainsEnvVarModifier(envVar string) bool { for _, mod := range envVarModifiers { @@ -66,7 +71,7 @@ func ContainsEnvVarModifier(envVar string) bool { } // ReadEnv loads an app envivornment into a map. -func ReadEnv(filePath string) (AppEnv, error) { +func ReadEnv(filePath string, opts ReadEnvOptions) (AppEnv, error) { var envVars AppEnv envVars, err := godotenv.Read(filePath) @@ -76,7 +81,7 @@ func ReadEnv(filePath string) (AppEnv, error) { for idx, envVar := range envVars { if strings.Contains(envVar, "#") { - if ContainsEnvVarModifier(envVar) { + if opts.IncludeModifiers && ContainsEnvVarModifier(envVar) { continue } vals := strings.Split(envVar, "#") @@ -222,7 +227,7 @@ func CheckEnv(app App) ([]EnvVar, error) { return envVars, err } - envSample, err := ReadEnv(envSamplePath) + envSample, err := ReadEnv(envSamplePath, ReadEnvOptions{}) if err != nil { return envVars, err } diff --git a/pkg/config/env_test.go b/pkg/config/env_test.go index d3741f32..6c62e108 100644 --- a/pkg/config/env_test.go +++ b/pkg/config/env_test.go @@ -70,7 +70,7 @@ func TestGetAllFilesInDirectory(t *testing.T) { } func TestReadEnv(t *testing.T) { - env, err := config.ReadEnv(ExpectedAppFile.Path) + env, err := config.ReadEnv(ExpectedAppFile.Path, config.ReadEnvOptions{}) if err != nil { t.Fatal(err) } @@ -123,7 +123,7 @@ func TestCheckEnv(t *testing.T) { } envSamplePath := path.Join(config.RECIPES_DIR, r.Name, ".env.sample") - envSample, err := config.ReadEnv(envSamplePath) + envSample, err := config.ReadEnv(envSamplePath, config.ReadEnvOptions{}) if err != nil { t.Fatal(err) } @@ -157,7 +157,7 @@ func TestCheckEnvError(t *testing.T) { } envSamplePath := path.Join(config.RECIPES_DIR, r.Name, ".env.sample") - envSample, err := config.ReadEnv(envSamplePath) + envSample, err := config.ReadEnv(envSamplePath, config.ReadEnvOptions{}) if err != nil { t.Fatal(err) } @@ -203,7 +203,7 @@ func TestEnvVarCommentsRemoved(t *testing.T) { } envSamplePath := path.Join(config.RECIPES_DIR, r.Name, ".env.sample") - envSample, err := config.ReadEnv(envSamplePath) + envSample, err := config.ReadEnv(envSamplePath, config.ReadEnvOptions{}) if err != nil { t.Fatal(err) } @@ -216,4 +216,31 @@ func TestEnvVarCommentsRemoved(t *testing.T) { if strings.Contains(envVar, "should be removed") { t.Fatalf("comment from '%s' should be removed", envVar) } + + envVar, exists = envSample["SECRET_TEST_PASS_TWO_VERSION"] + if !exists { + t.Fatal("WITH_COMMENT env var should be present in .env.sample") + } + + if strings.Contains(envVar, "length") { + t.Fatal("comment from env var SECRET_TEST_PASS_TWO_VERSION should have been removed") + } +} + +func TestEnvVarModifiersIncluded(t *testing.T) { + offline := true + r, err := recipe.Get("abra-test-recipe", offline) + if err != nil { + t.Fatal(err) + } + + envSamplePath := path.Join(config.RECIPES_DIR, r.Name, ".env.sample") + envSample, err := config.ReadEnv(envSamplePath, config.ReadEnvOptions{IncludeModifiers: true}) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(envSample["SECRET_TEST_PASS_TWO_VERSION"], "length") { + t.Fatal("comment from env var SECRET_TEST_PASS_TWO_VERSION should not be removed") + } } diff --git a/pkg/lint/recipe.go b/pkg/lint/recipe.go index cae797f3..e9214104 100644 --- a/pkg/lint/recipe.go +++ b/pkg/lint/recipe.go @@ -227,7 +227,7 @@ func LintAppService(recipe recipe.Recipe) (bool, error) { // therefore no matching traefik deploy label will be present. func LintTraefikEnabledSkipCondition(recipe recipe.Recipe) (bool, error) { envSamplePath := path.Join(config.RECIPES_DIR, recipe.Name, ".env.sample") - sampleEnv, err := config.ReadEnv(envSamplePath) + sampleEnv, err := config.ReadEnv(envSamplePath, config.ReadEnvOptions{}) if err != nil { return false, fmt.Errorf("Unable to discover .env.sample for %s", recipe.Name) } diff --git a/pkg/recipe/recipe.go b/pkg/recipe/recipe.go index 622d24c7..9ddf7d02 100644 --- a/pkg/recipe/recipe.go +++ b/pkg/recipe/recipe.go @@ -221,7 +221,7 @@ func Get(recipeName string, offline bool) (Recipe, error) { } envSamplePath := path.Join(config.RECIPES_DIR, recipeName, ".env.sample") - sampleEnv, err := config.ReadEnv(envSamplePath) + sampleEnv, err := config.ReadEnv(envSamplePath, config.ReadEnvOptions{}) if err != nil { return Recipe{}, err } @@ -249,9 +249,9 @@ func Get(recipeName string, offline bool) (Recipe, error) { }, nil } -func (r Recipe) SampleEnv() (map[string]string, error) { +func (r Recipe) SampleEnv(opts config.ReadEnvOptions) (map[string]string, error) { envSamplePath := path.Join(config.RECIPES_DIR, r.Name, ".env.sample") - sampleEnv, err := config.ReadEnv(envSamplePath) + sampleEnv, err := config.ReadEnv(envSamplePath, opts) if err != nil { return sampleEnv, fmt.Errorf("unable to discover .env.sample for %s", r.Name) } diff --git a/pkg/secret/secret.go b/pkg/secret/secret.go index e8964739..aa5262d2 100644 --- a/pkg/secret/secret.go +++ b/pkg/secret/secret.go @@ -69,9 +69,14 @@ func GeneratePassphrases(count uint) ([]string, error) { // and some times you don't (as the caller). We need to be able to handle the // "app new" case where we pass in the .env.sample and the "secret generate" // case where the app is created. -func ReadSecretsConfig(appEnv map[string]string, composeFiles []string, recipeName string) (map[string]string, error) { +func ReadSecretsConfig(appEnvPath string, composeFiles []string, recipeName string) (map[string]string, error) { secretConfigs := make(map[string]string) + appEnv, err := config.ReadEnv(appEnvPath, config.ReadEnvOptions{IncludeModifiers: true}) + if err != nil { + return secretConfigs, err + } + opts := stack.Deploy{Composefiles: composeFiles} config, err := loader.LoadComposefile(opts, appEnv) if err != nil { @@ -232,7 +237,7 @@ func PollSecretsStatus(cl *dockerClient.Client, app config.App) (secretStatuses, return secStats, err } - secretsConfig, err := ReadSecretsConfig(app.Env, composeFiles, app.Recipe) + secretsConfig, err := ReadSecretsConfig(app.Path, composeFiles, app.Recipe) if err != nil { return secStats, err } diff --git a/pkg/secret/secret_test.go b/pkg/secret/secret_test.go index 443b3bab..a4b0fc0c 100644 --- a/pkg/secret/secret_test.go +++ b/pkg/secret/secret_test.go @@ -18,13 +18,14 @@ func TestReadSecretsConfig(t *testing.T) { t.Fatal(err) } - sampleEnv, err := recipe.SampleEnv() + sampleEnv, err := recipe.SampleEnv(config.ReadEnvOptions{}) if err != nil { t.Fatal(err) } composeFiles := []string{path.Join(config.RECIPES_DIR, recipe.Name, "compose.yml")} - secretsFromConfig, err := ReadSecretsConfig(sampleEnv, composeFiles, recipe.Name) + envSamplePath := path.Join(config.RECIPES_DIR, recipe.Name, ".env.sample") + secretsFromConfig, err := ReadSecretsConfig(envSamplePath, composeFiles, recipe.Name) if err != nil { t.Fatal(err) } diff --git a/tests/integration/app_deploy.bats b/tests/integration/app_deploy.bats index a3a5ebdd..30fb9f74 100644 --- a/tests/integration/app_deploy.bats +++ b/tests/integration/app_deploy.bats @@ -344,3 +344,12 @@ teardown(){ _reset_app } + +@test "recipe config comments not present in values" { + run $ABRA app deploy "$TEST_APP_DOMAIN" --no-input + assert_success + + run $ABRA app run "$TEST_APP_DOMAIN" app env + assert_success + refute_output --partial 'should be removed' +}