From 34b6c289dd5e0a16ae6d69db96c70b91d051fa65 Mon Sep 17 00:00:00 2001 From: Helder Correia <174525+helderco@users.noreply.github.com> Date: Wed, 9 Mar 2022 11:29:26 -0100 Subject: [PATCH] Remove path based task lookup Signed-off-by: Helder Correia <174525+helderco@users.noreply.github.com> --- compiler/value.go | 13 ---- pkg/dagger.io/dagger/plan.cue | 9 ++- plan/task/clientenv.go | 35 +++++++--- plan/task/task.go | 66 +------------------ tests/plan.bats | 9 +-- tests/plan/client/env/concrete.cue | 19 ++++++ tests/plan/client/env/{test.cue => usage.cue} | 29 ++++---- 7 files changed, 71 insertions(+), 109 deletions(-) create mode 100644 tests/plan/client/env/concrete.cue rename tests/plan/client/env/{test.cue => usage.cue} (51%) diff --git a/compiler/value.go b/compiler/value.go index bc64e323..54ad84fd 100644 --- a/compiler/value.go +++ b/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) { diff --git a/pkg/dagger.io/dagger/plan.cue b/pkg/dagger.io/dagger/plan.cue index 03b16002..f85400bf 100644 --- a/pkg/dagger.io/dagger/plan.cue +++ b/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" diff --git a/plan/task/clientenv.go b/plan/task/clientenv.go index 82a13f81..5b51c483 100644 --- a/plan/task/clientenv.go +++ b/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) } diff --git a/plan/task/task.go b/plan/task/task.go index e7981663..516b7d70 100644 --- a/plan/task/task.go +++ b/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 -} diff --git a/tests/plan.bats b/tests/plan.bats index bc5d5e04..bbeeed18 100644 --- a/tests/plan.bats +++ b/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" } diff --git a/tests/plan/client/env/concrete.cue b/tests/plan/client/env/concrete.cue new file mode 100644 index 00000000..5cbd4656 --- /dev/null +++ b/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] + } + } +} diff --git a/tests/plan/client/env/test.cue b/tests/plan/client/env/usage.cue similarity index 51% rename from tests/plan/client/env/test.cue rename to tests/plan/client/env/usage.cue index af24b219..2c48785f 100644 --- a/tests/plan/client/env/test.cue +++ b/tests/plan/client/env/usage.cue @@ -8,36 +8,29 @@ 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 & { + string: dagger.#Exec & { input: image.output - args: [client.env.TEST_FAIL] + args: ["test", client.env.TEST_STRING, "=", "foo"] } - usage: { - string: dagger.#Exec & { - input: image.output - args: ["test", client.env.TEST_STRING, "=", "foo"] + secret: dagger.#Exec & { + input: image.output + mounts: secret: { + dest: "/run/secrets/test" + contents: client.env.TEST_SECRET } - secret: dagger.#Exec & { - input: image.output - mounts: secret: { - dest: "/run/secrets/test" - contents: client.env.TEST_SECRET - } - args: [ - "sh", "-c", - #""" + args: [ + "sh", "-c", + #""" test "$(cat /run/secrets/test)" = "bar" ls -l /run/secrets/test | grep -- "-r--------" """#, - ] - } + ] } } }