chore(refactor): split Options into ConfigOptions & JobOptions

They are both command line options to be parsed as if provided to
docker-run, but they are not to be trusted in the same way.

4 files changed, 22 insertions, 13 deletions

diff --git a/pkg/container/container_types.go b/pkg/container/container_types.go
index 511db9e..1d0acac 100644
--- a/pkg/container/container_types.go
+++ b/pkg/container/container_types.go
@@ -26,11 +26,13 @@ type NewContainerInput struct {
 	Privileged     bool
 	UsernsMode     string
 	Platform       string
-	Options        string
 	NetworkAliases []string
 	ExposedPorts   nat.PortSet
 	PortBindings   nat.PortMap
 
+	ConfigOptions string
+	JobOptions    string
+
 	// Gitea specific
 	AutoRemove bool
 
diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go
index 08bf53d..b086579 100644
--- a/pkg/container/docker_run.go
+++ b/pkg/container/docker_run.go
@@ -350,22 +350,24 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 	logger := common.Logger(ctx)
 	input := cr.input
 
-	if input.Options == "" {
+	if input.ConfigOptions == "" && input.JobOptions == "" {
 		return config, hostConfig, nil
 	}
 
+	options := input.ConfigOptions + " " + input.JobOptions
+
 	// parse configuration from CLI container.options
 	flags := pflag.NewFlagSet("container_flags", pflag.ContinueOnError)
 	copts := addFlags(flags)
 
-	optionsArgs, err := shellquote.Split(input.Options)
+	optionsArgs, err := shellquote.Split(options)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Cannot split container options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("Cannot split container options: '%s': '%w'", options, err)
 	}
 
 	err = flags.Parse(optionsArgs)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Cannot parse container options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("Cannot parse container options: '%s': '%w'", options, err)
 	}
 
 	// If a service container's network is set to `host`, the container will not be able to
@@ -386,14 +388,14 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 
 	containerConfig, err := parse(flags, copts, runtime.GOOS)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Cannot process container options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("Cannot process container options: '%s': '%w'", options, err)
 	}
 
 	logger.Debugf("Custom container.Config from options ==> %+v", containerConfig.Config)
 
 	err = mergo.Merge(config, containerConfig.Config, mergo.WithOverride, mergo.WithAppendSlice)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Cannot merge container.Config options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("Cannot merge container.Config options: '%s': '%w'", options, err)
 	}
 	logger.Debugf("Merged container.Config ==> %+v", config)
 
@@ -406,7 +408,7 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 	networkMode := hostConfig.NetworkMode
 	err = mergo.Merge(hostConfig, containerConfig.HostConfig, mergo.WithOverride)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Cannot merge container.HostConfig options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("Cannot merge container.HostConfig options: '%s': '%w'", options, err)
 	}
 	hostConfig.Binds = binds
 	hostConfig.Mounts = mounts
diff --git a/pkg/runner/action.go b/pkg/runner/action.go
index b26e52a..c417467 100644
--- a/pkg/runner/action.go
+++ b/pkg/runner/action.go
@@ -429,9 +429,10 @@ func newStepContainer(ctx context.Context, step step, image string, cmd []string
 		Privileged:   rc.Config.Privileged,
 		UsernsMode:   rc.Config.UsernsMode,
 		Platform:     rc.Config.ContainerArchitecture,
-		Options:      rc.Config.ContainerOptions,
 		AutoRemove:   rc.Config.AutoRemove,
 		ValidVolumes: rc.Config.ValidVolumes,
+
+		ConfigOptions: rc.Config.ContainerOptions,
 	})
 	return stepContainer
 }
diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go
index a833324..82443f8 100644
--- a/pkg/runner/run_context.go
+++ b/pkg/runner/run_context.go
@@ -489,12 +489,14 @@ func (rc *RunContext) startJobContainer() common.Executor {
 				UsernsMode:     rc.Config.UsernsMode,
 				Platform:       rc.Config.ContainerArchitecture,
 				AutoRemove:     rc.Config.AutoRemove,
-				Options:        rc.ExprEval.Interpolate(ctx, spec.Options),
 				NetworkMode:    networkName,
 				NetworkAliases: []string{serviceID},
 				ExposedPorts:   exposedPorts,
 				PortBindings:   portBindings,
 				ValidVolumes:   rc.Config.ValidVolumes,
+
+				JobOptions:    rc.ExprEval.Interpolate(ctx, spec.Options),
+				ConfigOptions: rc.Config.ContainerOptions,
 			})
 			rc.ServiceContainers = append(rc.ServiceContainers, c)
 		}
@@ -549,9 +551,11 @@ func (rc *RunContext) startJobContainer() common.Executor {
 			Privileged:     rc.Config.Privileged,
 			UsernsMode:     rc.Config.UsernsMode,
 			Platform:       rc.Config.ContainerArchitecture,
-			Options:        rc.options(ctx),
 			AutoRemove:     rc.Config.AutoRemove,
 			ValidVolumes:   rc.Config.ValidVolumes,
+
+			JobOptions:    rc.options(ctx),
+			ConfigOptions: "",
 		})
 		if rc.JobContainer == nil {
 			return errors.New("Failed to create job container")
@@ -889,10 +893,10 @@ func (rc *RunContext) options(ctx context.Context) string {
 	job := rc.Run.Job()
 	c := job.Container()
 	if c != nil {
-		return rc.Config.ContainerOptions + " " + rc.ExprEval.Interpolate(ctx, c.Options)
+		return rc.ExprEval.Interpolate(ctx, c.Options)
 	}
 
-	return rc.Config.ContainerOptions
+	return ""
 }
 
 func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) {