Merge pull request #1729 from helderco/remove-path-lookup

Remove path based task lookup

compiler/value.go

@@ -101,19 +101,6 @@ func (f Field) Label() string {
 	return l
 }
 
-// ParentLabel returns the unquoted parent selector of a value
-func (v *Value) ParentLabel(depth int) string {
-	sel := v.Path().Selectors()
-	if depth > len(sel) {
-		return ""
-	}
-	l := sel[len(sel)-depth].String()
-	if unquoted, err := strconv.Unquote(l); err == nil {
-		return unquoted
-	}
-	return l
-}
-
 // Proxy function to the underlying cue.Value
 // Field ordering is guaranteed to be stable.
 func (v *Value) Fields(opts ...cue.Option) ([]Field, error) {

pkg/dagger.io/dagger/plan.cue

@@ -24,7 +24,7 @@ package dagger
 		}
 
 		// Access client environment variables
-		env: [string]: *string | #Secret
+		env: _#clientEnv
 
 		// Execute commands in the client
 		commands: [id=string]: _#clientCommand
@@ -93,6 +93,13 @@ _#clientFilesystemWrite: {
 	}
 }
 
+_#clientEnv: {
+	$dagger: task: _name: "ClientEnv"
+
+	// CUE type defines expected content
+	[!~"\\$dagger"]: *string | #Secret
+}
+
 _#clientCommand: {
 	$dagger: task: _name: "ClientCommand"
 

plan/task/clientenv.go

@@ -13,19 +13,37 @@ import (
 )
 
 func init() {
-	Register("client.env.*", func() Task { return &clientEnvTask{} })
+	Register("ClientEnv", func() Task { return &clientEnvTask{} })
 }
 
 type clientEnvTask struct {
 }
 
 func (t clientEnvTask) Run(ctx context.Context, pctx *plancontext.Context, _ solver.Solver, v *compiler.Value) (*compiler.Value, error) {
-	lg := log.Ctx(ctx)
+	log.Ctx(ctx).Debug().Msg("loading environment variables")
 
-	envvar := v.ParentLabel(1)
+	fields, err := v.Fields()
+	if err != nil {
+		return nil, err
+	}
 
-	lg.Debug().Str("envvar", envvar).Msg("loading environment variable")
+	envs := make(map[string]interface{})
+	for _, field := range fields {
+		if field.Selector == cue.Str("$dagger") {
+			continue
+		}
+		envvar := field.Label()
+		val, err := t.getEnv(envvar, field.Value, pctx)
+		if err != nil {
+			return nil, err
+		}
+		envs[envvar] = val
+	}
 
+	return compiler.NewValue().FillFields(envs)
+}
+
+func (t clientEnvTask) getEnv(envvar string, v *compiler.Value, pctx *plancontext.Context) (interface{}, error) {
 	env := os.Getenv(envvar)
 	if env == "" {
 		return nil, fmt.Errorf("environment variable %q not set", envvar)
@@ -33,21 +51,20 @@ func (t clientEnvTask) Run(ctx context.Context, pctx *plancontext.Context, _ sol
 
 	// Resolve default in disjunction if a type hasn't been specified
 	val, _ := v.Default()
-	out := compiler.NewValue()
 
 	if plancontext.IsSecretValue(val) {
 		secret := pctx.Secrets.New(env)
-		return out.Fill(secret.MarshalCUE())
+		return secret.MarshalCUE(), nil
 	}
 
 	if val.IsConcrete() {
-		return nil, fmt.Errorf("unexpected concrete value, please use a type")
+		return nil, fmt.Errorf("%s: unexpected concrete value, please use a type", envvar)
 	}
 
 	k := val.IncompleteKind()
 	if k == cue.StringKind {
-		return out.Fill(env)
+		return env, nil
 	}
 
-	return nil, fmt.Errorf("unsupported type %q", k)
+	return nil, fmt.Errorf("%s: unsupported type %q", envvar, k)
 }

plan/task/task.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"strings"
 	"sync"
 
 	"cuelang.org/go/cue"
@@ -21,10 +20,6 @@ var (
 		cue.Str("$dagger"),
 		cue.Str("task"),
 		cue.Hid("_name", pkg.DaggerPackage))
-	lookups = []LookupFunc{
-		defaultLookup,
-		pathLookup,
-	}
 )
 
 // State is the state of the task.
@@ -38,7 +33,6 @@ const (
 )
 
 type NewFunc func() Task
-type LookupFunc func(*compiler.Value) (Task, error)
 
 type Task interface {
 	Run(ctx context.Context, pctx *plancontext.Context, s solver.Solver, v *compiler.Value) (*compiler.Value, error)
@@ -66,26 +60,13 @@ func New(typ string) Task {
 }
 
 func Lookup(v *compiler.Value) (Task, error) {
-	for _, lookup := range lookups {
-		t, err := lookup(v)
-		if err != nil {
-			return nil, err
-		}
-		if t != nil {
-			return t, nil
-		}
-	}
-	return nil, ErrNotTask
-}
-
-func defaultLookup(v *compiler.Value) (Task, error) {
 	if v.Kind() != cue.StructKind {
-		return nil, nil
+		return nil, ErrNotTask
 	}
 
 	typ := v.LookupPath(typePath)
 	if !typ.Exists() {
-		return nil, nil
+		return nil, ErrNotTask
 	}
 
 	typeString, err := typ.String()
@@ -100,46 +81,3 @@ func defaultLookup(v *compiler.Value) (Task, error) {
 
 	return t, nil
 }
-
-func pathLookup(v *compiler.Value) (Task, error) {
-	selectors := v.Path().Selectors()
-
-	// The `actions` field won't have any path based tasks since it's in user land
-	if len(selectors) == 0 || selectors[0].String() == "actions" {
-		return nil, nil
-	}
-
-	// Try an exact match first
-	if t := New(v.Path().String()); t != nil {
-		return t, nil
-	}
-
-	// FIXME: is there a way to avoid having to loop here?
-	var t Task
-	tasks.Range(func(key, value interface{}) bool {
-		if matchPathMask(selectors, key.(string)) {
-			fn := value.(NewFunc)
-			t = fn()
-			return false
-		}
-		return true
-	})
-	return t, nil
-}
-
-func matchPathMask(sels []cue.Selector, mask string) bool {
-	parts := strings.Split(mask, ".")
-	if len(sels) != len(parts) {
-		return false
-	}
-	for i, sel := range sels {
-		// use a '*' in a path mask part to match any selector
-		if parts[i] == "*" {
-			continue
-		}
-		if sel.String() != parts[i] {
-			return false
-		}
-	}
-	return true
-}

tests/plan.bats

@@ -150,22 +150,23 @@ setup() {
   export TEST_STRING="foo"
   export TEST_SECRET="bar"
 
-  "$DAGGER" "do" -p ./plan/client/env test usage
+  "$DAGGER" "do" -p ./plan/client/env/usage.cue test
 }
 
 @test "plan/client/env not exists" {
   cd "${TESTDIR}"
 
-  run "$DAGGER" "do" -p ./plan/client/env test usage
+  run "$DAGGER" "do" -p ./plan/client/env/usage.cue test
   assert_failure
+  assert_output --regexp "environment variable \"TEST_(STRING|SECRET)\" not set"
 }
 
-@test "plan/client/env invalid" {
+@test "plan/client/env concrete" {
   cd "${TESTDIR}"
 
   export TEST_FAIL="foobar"
 
-  run "$DAGGER" "do" -p ./plan/client/env test concrete
+  run "$DAGGER" "do" -p ./plan/client/env/concrete.cue test
   assert_failure
   assert_output --partial "TEST_FAIL: unexpected concrete value"
 }

tests/plan/client/env/concrete.cue

@@ -0,0 +1,19 @@
+package main
+
+import (
+	"dagger.io/dagger"
+)
+
+dagger.#Plan & {
+	client: env: TEST_FAIL: "env"
+
+	actions: {
+		image: dagger.#Pull & {
+			source: "alpine:3.15.0@sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3"
+		}
+		test: dagger.#Exec & {
+			input: image.output
+			args: [client.env.TEST_FAIL]
+		}
+	}
+}

tests/plan/client/env/usage.cue

@@ -8,18 +8,12 @@ dagger.#Plan & {
 	client: env: {
 		TEST_STRING: string
 		TEST_SECRET: dagger.#Secret
-		TEST_FAIL:   "env"
 	}
 	actions: {
 		image: dagger.#Pull & {
 			source: "alpine:3.15.0@sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3"
 		}
 		test: {
-			concrete: dagger.#Exec & {
-				input: image.output
-				args: [client.env.TEST_FAIL]
-			}
-			usage: {
 			string: dagger.#Exec & {
 				input: image.output
 				args: ["test", client.env.TEST_STRING, "=", "foo"]
@@ -40,5 +34,4 @@ dagger.#Plan & {
 			}
 		}
 	}
-	}
 }