pkg/step: read `proto.StepDefinition` directly

This exposes only `Read/Write/Serialize/Deserialize`
methods to parse step definition from the file.

The `step-runner ci` also consider this as a regular
`step.yml` which is constructed dynamically.

.gitlab-ci.yml

@@ -36,12 +36,10 @@ run-e2e:
     - /step-runner ci
   variables:
     STEPS: |
-      type: steps
-      steps:
-        - name: hello-world
-          step: https+git://gitlab.com/gitlab-org/ci-cd/runner-tools/echo-step
-          inputs:
-            echo: ${CI_PIPELINE_ID}
+      - name: hello-world
+        step: https+git://gitlab.com/gitlab-org/ci-cd/runner-tools/echo-step
+        inputs:
+          echo: ${CI_PIPELINE_ID}
   artifacts:
     paths:
       - step-results.json

README.md

@@ -7,12 +7,10 @@ hello-world-job:
     - /step-runner ci
   variables:
     STEPS: |
-      type: steps
-      steps:
-        - name: hello-world-step
-          step: https+git://gitlab.com/gitlab-org/ci-cd/runner-tools/echo-step
-          inputs:
-            echo: hello world
+      - name: hello-world-step
+        step: https+git://gitlab.com/gitlab-org/ci-cd/runner-tools/echo-step
+        inputs:
+          echo: hello world
   artifacts:
     paths:
       - step-results.json

cmd/ci/ci.go

@@ -21,9 +21,16 @@ var Cmd = &cobra.Command{
 	RunE:  run,
 }
 
+const stepsTemplate = `
+spec: {}
+---
+type: steps
+steps:
+`
+
 func run(cmd *cobra.Command, args []string) error {
 	steps := os.Getenv("STEPS")
-	def, err := step.ReadSteps(steps)
+	stepDefinition, err := step.Deserialize(stepsTemplate+steps, "")
 	if err != nil {
 		return fmt.Errorf("reading STEPS %q: %w", steps, err)
 	}
@@ -40,15 +47,9 @@ func run(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("creating execution: %w", err)
 	}
 
-	specDefinition := &proto.StepDefinition{
-		Spec: &proto.Spec{
-			Spec: &proto.Spec_Content{},
-		},
-		Definition: def,
-	}
 	stepCall := &proto.StepCall{}
 
-	result, err := execution.Run(ctx.Background(), specDefinition, stepCall, globalCtx)
+	result, err := execution.Run(ctx.Background(), stepDefinition, stepCall, globalCtx)
 	if err != nil {
 		return fmt.Errorf("running execution: %w", err)
 	}

pkg/cache/cache.go

@@ -15,7 +15,7 @@ import (
 )
 
 type Cache interface {
-	Get(ctx context.Context, step string) (*proto.Spec, *proto.Definition, string, error)
+	Get(ctx context.Context, step string) (*proto.StepDefinition, error)
 }
 
 var _ Cache = &cache{}
@@ -36,14 +36,14 @@ func New() (Cache, error) {
 	}, nil
 }
 
-func (c *cache) Get(ctx context.Context, stepRef string) (*proto.Spec, *proto.Definition, string, error) {
-	load := func(dir string) (*proto.Spec, *proto.Definition, string, error) {
+func (c *cache) Get(ctx context.Context, stepRef string) (*proto.StepDefinition, error) {
+	load := func(dir string) (*proto.StepDefinition, error) {
 		filename := filepath.Join(dir, "step.yml")
-		spec, def, err := step.LoadSpecDef(filename)
+		stepDefinition, err := step.Read(filename)
 		if err != nil {
-			return nil, nil, "", fmt.Errorf("loading file %q: %w", dir, err)
+			return nil, fmt.Errorf("loading file %q: %w", dir, err)
 		}
-		return spec, def, dir, nil
+		return stepDefinition, nil
 	}
 	switch {
 	case strings.HasPrefix(stepRef, "."):
@@ -51,11 +51,11 @@ func (c *cache) Get(ctx context.Context, stepRef string) (*proto.Spec, *proto.De
 	case strings.HasPrefix(stepRef, "https+git"):
 		dir, err := c.getCacheDir(ctx, stepRef)
 		if err != nil {
-			return nil, nil, "", fmt.Errorf("fetching step %q: %w", stepRef, err)
+			return nil, fmt.Errorf("fetching step %q: %w", stepRef, err)
 		}
 		return load(dir)
 	default:
-		return nil, nil, "", fmt.Errorf("invalid step reference: %v", stepRef)
+		return nil, fmt.Errorf("invalid step reference: %v", stepRef)
 	}
 }
 

pkg/cache/cache_integration_test.go

@@ -53,10 +53,8 @@ func runEchoSteps(t *testing.T) {
 }
 
 const echoSteps = `
-type: steps
-steps:
-  - name: hello-world
-    step: "https+git://gitlab.com/gitlab-org/ci-cd/runner-tools/echo-step"
-    inputs:
-       echo: hello world
+- name: hello-world
+  step: "https+git://gitlab.com/gitlab-org/ci-cd/runner-tools/echo-step"
+  inputs:
+    echo: hello world
 `

pkg/runner/runner.go

@@ -194,19 +194,12 @@ func (e *Execution) runStep(ctx ctx.Context, stepReference *proto.Step, stepsCtx
 		stepCall.Env[k] = res
 	}
 
-	spec, def, dir, err := e.defs.Get(ctx, stepReference.Step)
+	stepDefinition, err := e.defs.Get(ctx, stepReference.Step)
 	if err != nil {
 		return nil, fmt.Errorf("getting step %q definition: %w", stepReference.Name, err)
 	}
 
-	// TODO: The `defs.Get` should return `proto.StepDefinition`
-	stepDef := &proto.StepDefinition{
-		Spec:       spec,
-		Definition: def,
-		Dir:        dir,
-	}
-
-	result, err := e.Run(ctx, stepDef, stepCall, stepsCtx.Global)
+	result, err := e.Run(ctx, stepDefinition, stepCall, stepsCtx.Global)
 	if err != nil {
 		return nil, err
 	}

pkg/runner/runner_test.go

@@ -24,6 +24,8 @@ func TestRun(t *testing.T) {
 	}{{
 		name: "greeting with defaults",
 		yaml: `
+spec: {}
+---
 type: steps
 steps:
   - name: greet-steppy
@@ -38,6 +40,8 @@ steps:
 	}, {
 		name: "greeting outputs and exports name parameter",
 		yaml: `
+spec: {}
+---
 type: steps
 steps:
   - name: greet-foo
@@ -53,6 +57,8 @@ steps:
 	}, {
 		name: "can access outputs of a previous step",
 		yaml: `
+spec: {}
+---
 type: steps
 steps:
   - name: greet-foo
@@ -72,6 +78,8 @@ steps:
 	}, {
 		name: "can access outputs of a composite step",
 		yaml: `
+spec: {}
+---
 type: steps
 steps:
   - name: greet-the-crew
@@ -90,6 +98,8 @@ steps:
 	}, {
 		name: "cannot access outputs of composite children",
 		yaml: `
+spec: {}
+---
 type: steps
 steps:
   - name: greet-the-crew
@@ -103,6 +113,8 @@ steps:
 	}, {
 		name: "complex steps",
 		yaml: `
+spec: {}
+---
 type: steps
 steps:
   - name: greet-steppy
@@ -136,7 +148,7 @@ meet joe who is 42 likes {"characters":["sponge bob","patrick star"]} and is hun
 	}}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			def, err := step.ReadSteps(c.yaml)
+			stepDef, err := step.Deserialize(c.yaml, "")
 			require.NoError(t, err)
 
 			defs, err := cache.New()
@@ -151,15 +163,9 @@ meet joe who is 42 likes {"characters":["sponge bob","patrick star"]} and is hun
 			globalCtx.Stdout = &log
 			globalCtx.Stderr = &log
 
-			specDefinition := &proto.StepDefinition{
-				Spec: &proto.Spec{
-					Spec: &proto.Spec_Content{},
-				},
-				Definition: def,
-			}
 			stepCall := &proto.StepCall{}
 
-			result, err := runner.Run(ctx.Background(), specDefinition, stepCall, globalCtx)
+			result, err := runner.Run(ctx.Background(), stepDef, stepCall, globalCtx)
 			if c.wantErr != nil {
 				require.Equal(t, c.wantErr, err)
 			} else {

pkg/step/read.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"google.golang.org/protobuf/encoding/protojson"
@@ -15,64 +16,34 @@ import (
 
 const defaultStorePerm = 0o640
 
-func LoadSpecDef(filename string) (*proto.Spec, *proto.Definition, error) {
-	buf, err := os.ReadFile(filename)
-	if err != nil {
-		return nil, nil, fmt.Errorf("reading file: %w", err)
-	}
-
-	return ReadSpecDef(string(buf))
-}
-
-func ReadSpecDef(stepDefinitionYAML string) (*proto.Spec, *proto.Definition, error) {
-	var (
-		spec proto.Spec
-		def  proto.Definition
-	)
-
-	if err := unmarshal(stepDefinitionYAML, &spec, &def); err != nil {
-		return nil, nil, err
-	}
-
-	return &spec, &def, nil
-}
-
-func StoreSpecDef(spec *proto.Spec, def *proto.Definition, filename string) error {
-	encoded, err := WriteSpecDef(spec, def)
-	if err != nil {
-		return err
-	}
-
-	return os.WriteFile(filename, []byte(encoded), defaultStorePerm)
-}
-
-func WriteSpecDef(spec *proto.Spec, def *proto.Definition) (string, error) {
-	return marshal(spec, def)
-}
-
-func LoadSteps(filename string) (*proto.Definition, error) {
+func Read(filename string) (*proto.StepDefinition, error) {
 	buf, err := os.ReadFile(filename)
 	if err != nil {
 		return nil, fmt.Errorf("reading file: %w", err)
 	}
 
-	return ReadSteps(string(buf))
+	return Deserialize(string(buf), filepath.Dir(filename))
 }
 
-func ReadSteps(stepsYAML string) (*proto.Definition, error) {
+func Deserialize(content, dir string) (*proto.StepDefinition, error) {
 	var (
-		def proto.Definition
+		spec       proto.Spec
+		definition proto.Definition
 	)
 
-	if err := unmarshal(stepsYAML, &def); err != nil {
+	if err := unmarshal(content, &spec, &definition); err != nil {
 		return nil, err
 	}
 
-	return &def, nil
+	return &proto.StepDefinition{
+		Spec:       &spec,
+		Definition: &definition,
+		Dir:        dir,
+	}, nil
 }
 
-func StoreSteps(def *proto.Definition, filename string) error {
-	encoded, err := WriteSteps(def)
+func Write(stepDef *proto.StepDefinition, filename string) error {
+	encoded, err := Serialize(stepDef)
 	if err != nil {
 		return err
 	}
@@ -80,12 +51,8 @@ func StoreSteps(def *proto.Definition, filename string) error {
 	return os.WriteFile(filename, []byte(encoded), defaultStorePerm)
 }
 
-func WriteSteps(def *proto.Definition) (string, error) {
-	if def.Type != proto.DefinitionType_steps {
-		return "", fmt.Errorf("want a definition of type steps. got %v", def.Type)
-	}
-
-	return marshal(def)
+func Serialize(stepDef *proto.StepDefinition) (string, error) {
+	return marshal(stepDef.Spec, stepDef.Definition)
 }
 
 func unmarshal(input string, subjects ...protoreflect.ProtoMessage) error {

pkg/step/read_test.go

@@ -10,7 +10,7 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 )
 
-func TestReadWriteSpecDef(t *testing.T) {
+func TestSerialize(t *testing.T) {
 	cases := []struct {
 		name     string
 		yaml     string
@@ -47,7 +47,7 @@ type: exec
 			},
 		},
 	}, {
-		name: "everything",
+		name: "complex type: exec",
 		yaml: `
 spec:
     inputs:
@@ -121,81 +121,10 @@ type: exec
 			},
 		},
 	}, {
-		name: "documents out of order",
+		name: "complex type: steps",
 		yaml: `
-type: exec
-exec:
-  command: [echo, "${{inputs.name}}"]
+spec: {}
 ---
-spec:
-  inputs:
-    name:
-`,
-		wantErr: true,
-	}, {
-		name: "missing spec",
-		yaml: `
-type: exec
-exec:
-  command: [echo, "${{inputs.name}}"]
-`,
-		wantErr: true,
-	}, {
-		name: "missing exec",
-		yaml: `
-spec:
-  inputs:
-    name:
-`,
-		wantErr: true,
-	}}
-	for _, c := range cases {
-		t.Run(c.name, func(t *testing.T) {
-			spec, def, err := ReadSpecDef(c.yaml)
-			if c.wantErr {
-				require.Error(t, err)
-				require.Nil(t, spec)
-				require.Nil(t, def)
-			} else {
-				require.NoError(t, err)
-				if !protobuf.Equal(c.wantSpec, spec) {
-					t.Errorf("wanted:\n%+v\ngot:\n%+v", c.wantSpec, spec)
-				}
-				if !protobuf.Equal(c.wantDef, def) {
-					t.Errorf("wanted:\n%+v\ngot:\n%+v", c.wantDef, def)
-				}
-				yaml, err := WriteSpecDef(spec, def)
-				require.NoError(t, err)
-				require.Equal(t, strings.TrimSpace(c.yaml), strings.TrimSpace(yaml))
-			}
-		})
-	}
-}
-
-func TestReadWriteSteps(t *testing.T) {
-	cases := []struct {
-		name      string
-		yaml      string
-		wantErr   bool
-		wantSteps *proto.Definition
-	}{{
-		name: "simple case",
-		yaml: `
-steps:
-    - name: foo
-      step: bar
-type: steps
-`,
-		wantSteps: &proto.Definition{
-			Type: proto.DefinitionType_steps,
-			Steps: []*proto.Step{{
-				Name: "foo",
-				Step: "bar",
-			}},
-		},
-	}, {
-		name: "everything",
-		yaml: `
 outputs:
     eye_color: brown
 steps:
@@ -218,7 +147,8 @@ steps:
       step: ../steps/redux
 type: steps
 `,
-		wantSteps: &proto.Definition{
+		wantSpec: &proto.Spec{Spec: &proto.Spec_Content{}},
+		wantDef: &proto.Definition{
 			Type: proto.DefinitionType_steps,
 			Steps: []*proto.Step{{
 				Name: "foo to the max",
@@ -253,19 +183,50 @@ type: steps
 				"eye_color": "brown",
 			},
 		},
+	}, {
+		name: "documents out of order",
+		yaml: `
+type: exec
+exec:
+  command: [echo, "${{inputs.name}}"]
+---
+spec:
+  inputs:
+    name:
+`,
+		wantErr: true,
+	}, {
+		name: "missing spec",
+		yaml: `
+type: exec
+exec:
+  command: [echo, "${{inputs.name}}"]
+`,
+		wantErr: true,
+	}, {
+		name: "missing exec",
+		yaml: `
+spec:
+  inputs:
+    name:
+`,
+		wantErr: true,
 	}}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			steps, err := ReadSteps(c.yaml)
+			stepDef, err := Deserialize(c.yaml, "")
 			if c.wantErr {
 				require.Error(t, err)
-				require.Nil(t, steps)
+				require.Nil(t, stepDef)
 			} else {
 				require.NoError(t, err)
-				if !protobuf.Equal(c.wantSteps, steps) {
-					t.Errorf("wanted:\n%+v\ngot:\n%+v", c.wantSteps, steps)
+				if !protobuf.Equal(c.wantSpec, stepDef.Spec) {
+					t.Errorf("wanted:\n%+v\ngot:\n%+v", c.wantSpec, stepDef.Spec)
+				}
+				if !protobuf.Equal(c.wantDef, stepDef.Definition) {
+					t.Errorf("wanted:\n%+v\ngot:\n%+v", c.wantDef, stepDef.Definition)
 				}
-				yaml, err := WriteSteps(steps)
+				yaml, err := Serialize(stepDef)
 				require.NoError(t, err)
 				require.Equal(t, strings.TrimSpace(c.yaml), strings.TrimSpace(yaml))
 			}