runner-go icon indicating copy to clipboard operation
runner-go copied to clipboard

(feat) adding reading of an external variables to a step

Open tphoney opened this issue 3 years ago • 1 comments

Dependant on https://github.com/drone/drone-go/pull/72

tphoney avatar Jun 28 '22 15:06 tphoney

Adding notes from our chat.

Add the array of step outputs to runtime.Step interface:

	Step interface {
		// GetName returns the step name.
		GetName() string

		// GetDependencies returns the step dependencies
		// used to define an execution graph.
		GetDependencies() []string

		// GetEnviron returns the step environment variables.
		GetEnviron() map[string]string

		// SetEnviron updates the step environment variables.
		SetEnviron(map[string]string)

+		// GetOutput returns the step outputs
+		GetOutputs() []string

You could then add Outputs here: https://github.com/drone-runners/drone-runner-docker/blob/master/engine/spec.go#L26

And satisfy the GetOutputs interface here: https://github.com/drone-runners/drone-runner-docker/blob/master/engine/spec.go#L150

Since the output variables are no longer stored in the step, you could aggregate them in the state:

// State stores the pipeline state.
type State struct {
	sync.Mutex

	Build  *drone.Build
	Repo   *drone.Repo
	Stage  *drone.Stage
	System *drone.System
+	Outputs map[string]string
}

As you mentioned, we might want to add the outputs to the Drone database at some point so we can surface in the user interface. If we do this, we'll want to store Outputs map[string]string in the drone-go library. This is a really interesting idea and Harness does this, but I want to think through the sec implementations a little further. It is possible people will store sensitive data in the output variables. As an example, GitHub Actions has a vault Action that gets secrets from vault and then passes the secrets to subsequent steps as output variables; we would not want this data exposed in our user interface. But let's think on this further, and we can always do a follow-up enhancement once we've figured it out.

bradrydzewski avatar Oct 13 '22 14:10 bradrydzewski