From 80ae63928b190d108c35fe0cf6e1072396260dd8 Mon Sep 17 00:00:00 2001
From: Helder Correia <174525+helderco@users.noreply.github.com>
Date: Mon, 11 Apr 2022 18:31:36 +0000
Subject: [PATCH] feat: Allow default values in `client: env` (#2122)

Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com>
---
 docs/tests/core-concepts/client/plans/env.cue | 15 +++++++++++++--
 plan/task/clientenv.go                        | 17 +++++++++++------
 tests/plan.bats                               |  2 +-
 tests/plan/client/env/usage.cue               |  5 +++++
 4 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/docs/tests/core-concepts/client/plans/env.cue b/docs/tests/core-concepts/client/plans/env.cue
index 4f8a084e..8ed209b5 100644
--- a/docs/tests/core-concepts/client/plans/env.cue
+++ b/docs/tests/core-concepts/client/plans/env.cue
@@ -1,11 +1,22 @@
+package main
+
+import (
+	"dagger.io/dagger"
+	"universe.dagger.io/docker"
+)
+
 dagger.#Plan & {
 	client: env: {
-		REGISTRY_USER:  string
+		// load as a string
+		REGISTRY_USER: string
+		// load as a secret
 		REGISTRY_TOKEN: dagger.#Secret
+		// load as a string, using a default if not defined
+		BASE_IMAGE: string | *"registry.example.com/image"
 	}
 
 	actions: pull: docker.#Pull & {
-		source: "registry.example.com/image"
+		source: client.env.BASE_IMAGE
 		auth: {
 			username: client.env.REGISTRY_USER
 			secret:   client.env.REGISTRY_TOKEN
diff --git a/plan/task/clientenv.go b/plan/task/clientenv.go
index 52fe75db..5e51f625 100644
--- a/plan/task/clientenv.go
+++ b/plan/task/clientenv.go
@@ -44,21 +44,26 @@ func (t clientEnvTask) Run(ctx context.Context, pctx *plancontext.Context, _ *so
 }
 
 func (t clientEnvTask) getEnv(envvar string, v *compiler.Value, pctx *plancontext.Context) (interface{}, error) {
-	env := os.Getenv(envvar)
-	if env == "" {
+	// Resolve default in disjunction if a type hasn't been specified
+	val, hasDefault := v.Default()
+
+	env, hasEnv := os.LookupEnv(envvar)
+	if !hasEnv {
+		if hasDefault {
+			// we can only have default strings so
+			// don't worry about secret values here
+			return val, nil
+		}
 		return nil, fmt.Errorf("environment variable %q not set", envvar)
 	}
 
-	// Resolve default in disjunction if a type hasn't been specified
-	val, _ := v.Default()
-
 	if plancontext.IsSecretValue(val) {
 		secret := pctx.Secrets.New(env)
 		return secret.MarshalCUE(), nil
 	}
 
 	if val.IsConcrete() {
-		return nil, fmt.Errorf("%s: unexpected concrete value, please use a type", envvar)
+		return nil, fmt.Errorf("%s: unexpected concrete value, please use a type or set a default", envvar)
 	}
 
 	k := val.IncompleteKind()
diff --git a/tests/plan.bats b/tests/plan.bats
index 25c77106..1eae849b 100644
--- a/tests/plan.bats
+++ b/tests/plan.bats
@@ -190,7 +190,7 @@ setup() {
 
   run "$DAGGER" "do" -p ./plan/client/env/usage.cue test
   assert_failure
-  assert_output --regexp "environment variable \"TEST_(STRING|SECRET)\" not set"
+  assert_output --regexp "environment variable \"TEST_(STRING|DEFAULT|SECRET)\" not set"
 }
 
 @test "plan/client/env concrete" {
diff --git a/tests/plan/client/env/usage.cue b/tests/plan/client/env/usage.cue
index 5ca80503..72ebac2a 100644
--- a/tests/plan/client/env/usage.cue
+++ b/tests/plan/client/env/usage.cue
@@ -9,6 +9,7 @@ dagger.#Plan & {
 	client: env: {
 		TEST_STRING: string
 		TEST_SECRET: dagger.#Secret
+		TEST_DEFAULT: string | *"hello world"
 	}
 	actions: {
 		image: core.#Pull & {
@@ -19,6 +20,10 @@ dagger.#Plan & {
 				input: image.output
 				args: ["test", client.env.TEST_STRING, "=", "foo"]
 			}
+			default: core.#Exec & {
+				input: image.output
+				args: ["test", client.env.TEST_DEFAULT, "=", "hello world"]
+			}
 			secret: core.#Exec & {
 				input: image.output
 				mounts: secret: {