fix: improve app check

See coop-cloud/organising#446

cli/app/check.go

@@ -1,13 +1,10 @@
 package app
 
 import (
-	"os"
-	"path"
-	"strings"
-
 	"coopcloud.tech/abra/cli/internal"
 	"coopcloud.tech/abra/pkg/autocomplete"
 	"coopcloud.tech/abra/pkg/config"
+	"coopcloud.tech/abra/pkg/formatter"
 	"coopcloud.tech/abra/pkg/recipe"
 	recipePkg "coopcloud.tech/abra/pkg/recipe"
 	"github.com/sirupsen/logrus"
@@ -15,9 +12,21 @@ import (
 )
 
 var appCheckCommand = cli.Command{
-	Name:      "check",
-	Aliases:   []string{"chk"},
-	Usage:     "Check if an app is configured correctly",
+	Name:    "check",
+	Aliases: []string{"chk"},
+	Usage:   "Ensure an app is well configured",
+	Description: `
+This command compares env vars in both the app ".env" and recipe ".env.sample"
+file.
+
+The goal is to ensure that recipe ".env.sample" env vars are defined in your
+app ".env" file. Only env var definitions in the ".env.sample" which are
+uncommented, e.g. "FOO=bar" are checked. If an app ".env" file does not include
+these env vars, then "check" will complain.
+
+Recipe maintainers may or may not provide defaults for env vars within their
+recipes regardless of commenting or not (e.g. through the use of
+${FOO:<default>} syntax). "check" does not confirm or deny this for you.`,
 	ArgsUsage: "<domain>",
 	Flags: []cli.Flag{
 		internal.DebugFlag,
@@ -49,32 +58,23 @@ var appCheckCommand = cli.Command{
 			}
 		}
 
-		envSamplePath := path.Join(config.RECIPES_DIR, app.Recipe, ".env.sample")
-		if _, err := os.Stat(envSamplePath); err != nil {
-			if os.IsNotExist(err) {
-				logrus.Fatalf("%s does not exist?", envSamplePath)
-			}
-			logrus.Fatal(err)
-		}
+		tableCol := []string{"recipe env sample", "app env"}
+		table := formatter.CreateTable(tableCol)
 
-		envSample, err := config.ReadEnv(envSamplePath)
+		envVars, err := config.CheckEnv(app)
 		if err != nil {
 			logrus.Fatal(err)
 		}
 
-		var missing []string
-		for k := range envSample {
-			if _, ok := app.Env[k]; !ok {
-				missing = append(missing, k)
+		for _, envVar := range envVars {
+			if envVar.Present {
+				table.Append([]string{envVar.Name, "✅"})
+			} else {
+				table.Append([]string{envVar.Name, "❌"})
 			}
 		}
 
-		if len(missing) > 0 {
-			missingEnvVars := strings.Join(missing, ", ")
-			logrus.Fatalf("%s is missing %s", app.Path, missingEnvVars)
-		}
-
-		logrus.Infof("all necessary environment variables defined for %s", app.Name)
+		table.Render()
 
 		return nil
 	},

cli/app/deploy.go

@@ -200,6 +200,17 @@ recipes.
 		config.SetChaosVersionLabel(compose, stackName, version)
 		config.SetUpdateLabel(compose, stackName, app.Env)
 
+		envVars, err := config.CheckEnv(app)
+		if err != nil {
+			logrus.Fatal(err)
+		}
+
+		for _, envVar := range envVars {
+			if !envVar.Present {
+				logrus.Warnf("env var %s missing from %s.env, present in recipe .env.sample", envVar.Name, app.Domain)
+			}
+		}
+
 		if err := internal.DeployOverview(app, version, "continue with deployment?"); err != nil {
 			logrus.Fatal(err)
 		}

cli/app/upgrade.go

@@ -254,6 +254,17 @@ recipes.
 		config.SetChaosVersionLabel(compose, stackName, chosenUpgrade)
 		config.SetUpdateLabel(compose, stackName, app.Env)
 
+		envVars, err := config.CheckEnv(app)
+		if err != nil {
+			logrus.Fatal(err)
+		}
+
+		for _, envVar := range envVars {
+			if !envVar.Present {
+				logrus.Warnf("env var %s missing from %s.env, present in recipe .env.sample", envVar.Name, app.Domain)
+			}
+		}
+
 		if err := internal.NewVersionOverview(app, deployedVersion, chosenUpgrade, releaseNotes); err != nil {
 			logrus.Fatal(err)
 		}

pkg/config/env.go

@@ -9,6 +9,7 @@ import (
 	"path"
 	"path/filepath"
 	"regexp"
+	"sort"
 	"strings"
 
 	"github.com/Autonomic-Cooperative/godotenv"
@@ -179,3 +180,42 @@ func ReadAbraShEnvVars(abraSh string) (map[string]string, error) {
 
 	return envVars, nil
 }
+
+type EnvVar struct {
+	Name    string
+	Present bool
+}
+
+func CheckEnv(app App) ([]EnvVar, error) {
+	var envVars []EnvVar
+
+	envSamplePath := path.Join(RECIPES_DIR, app.Recipe, ".env.sample")
+	if _, err := os.Stat(envSamplePath); err != nil {
+		if os.IsNotExist(err) {
+			return envVars, fmt.Errorf("%s does not exist?", envSamplePath)
+		}
+		return envVars, err
+	}
+
+	envSample, err := ReadEnv(envSamplePath)
+	if err != nil {
+		return envVars, err
+	}
+
+	var keys []string
+	for key := range envSample {
+		keys = append(keys, key)
+	}
+
+	sort.Strings(keys)
+
+	for _, key := range keys {
+		if _, ok := app.Env[key]; ok {
+			envVars = append(envVars, EnvVar{Name: key, Present: true})
+		} else {
+			envVars = append(envVars, EnvVar{Name: key, Present: false})
+		}
+	}
+
+	return envVars, nil
+}

pkg/config/env_test.go

@@ -114,3 +114,73 @@ func TestReadAbraShEnvVars(t *testing.T) {
 		t.Error("OUTER_FOO should be exported")
 	}
 }
+
+func TestCheckEnv(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)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	app := config.App{
+		Name:   "test-app",
+		Recipe: r.Name,
+		Domain: "example.com",
+		Env:    envSample,
+		Path:   "example.com.env",
+		Server: "example.com",
+	}
+
+	envVars, err := config.CheckEnv(app)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, envVar := range envVars {
+		if !envVar.Present {
+			t.Fatalf("%s should be present", envVar.Name)
+		}
+	}
+}
+
+func TestCheckEnvError(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)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	delete(envSample, "DOMAIN")
+
+	app := config.App{
+		Name:   "test-app",
+		Recipe: r.Name,
+		Domain: "example.com",
+		Env:    envSample,
+		Path:   "example.com.env",
+		Server: "example.com",
+	}
+
+	envVars, err := config.CheckEnv(app)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, envVar := range envVars {
+		if envVar.Name == "DOMAIN" && envVar.Present {
+			t.Fatalf("%s should not be present", envVar.Name)
+		}
+	}
+}