feat: Allow overriding the argoexec binary location

[mprahl]

Motivation

The specific usecase is to allow a distribution of Argo Workflows to build one container image with a CGO_ENABLED=1 binary of argoexec and one with CGO_ENABLED=0. When the cluster must be FIPS compliant, we use the CGO_ENABLED=1 binary to use FIPS approved C libraries by setting the ARGO_EXEC_PATH environment variable. When the cluster does not require FIPS, we will just use the CGO_ENABLED=0 binary which allows much more flexibility for the container image used by the main container as the C libraries don't need to match from the build environment.

Modifications

This introduces the ARGO_EXEC_PATH environment variable and when set, the value is used as the path to copy to /var/run/argo/argoexec.

Verification

I deployed the workflow controller with the the ARGO_EXEC_PATH environment variable and looked for the new logs to ensure the right code path was being executed.

Documentation

Documented in docs/environment-variables.md.

[mprahl]

/cc @terrytangyuan if you have a chance, could you please review or help me find someone to review it?

[Joibel]

I don't think it's the right approach. Building a container with both binaries wouldn't satisfy some users, so they'd just build a single binary container and switch to using that for all workflows by setting executor.image in the workflow-controller-configmap.

If you really need to switch on a per workflow or per template basis this is already doable using a podSpecPatch which you can attempt with this (not-working because alpine isn't argoexec) example:

podSpecPatch: '{"containers":[{"name":"wait", "image":"alpine"}],"initContainers":[{"name":"init", "image":"alpine","command":["echo","hello"]}]}'

Instead of this approach in the PR using an env variable I'd support doing this through a workflow and/or template level entry which worked just like the workflow-controller-configmap executor.image.

I'd actually like it if this was implemented as a full PodSpec override just like it is there. This would then allow per template resource requests for the executor, which would be useful where specific templates use a lot of artifacts for example.

[mprahl]

I don't think it's the right approach. Building a container with both binaries wouldn't satisfy some users, so they'd just build a single binary container and switch to using that for all workflows by setting executor.image in the workflow-controller-configmap.

If you really need to switch on a per workflow or per template basis this is already doable using a podSpecPatch which you can attempt with this (not-working because alpine isn't argoexec) example:

podSpecPatch: '{"containers":[{"name":"wait", "image":"alpine"}],"initContainers":[{"name":"init", "image":"alpine","command":["echo","hello"]}]}'

Instead of this approach in the PR using an env variable I'd support doing this through a workflow and/or template level entry which worked just like the workflow-controller-configmap executor.image.

I'd actually like it if this was implemented as a full PodSpec override just like it is there. This would then allow per template resource requests for the executor, which would be useful where specific templates use a lot of artifacts for example.

@Joibel thanks for the feedback! For more context, we only want to maintain one container image with both binaries because we only want to maintain one container image per architecture for Argo Exec rather than two. With the one container image with two binaries, we would have different manifests setting the correct Argo Exec binary to use depending on Kubernetes cluster environment. This is less complicated for us to package as part of Red Hat's distribution of Kubeflow Pipelines (which depends on Argo Workflows). In our case, we are trying to support clusters in FIPS mode with a dynamically linked Go binary using a FIPS certified GLIBC and OpenSSL, while also using a statically linked Go binary (CGO disabled) for clusters not requiring FIPS. The latter binary doesn't require the user to be using container images for their pipeline step with the same GLIBC and OpenSSL versions of the build environment.

Back to your suggestion, do you mean to allow command to be set to a different path inside the workflow-controller-configmap instead of the environment variable?

For example:

executor: |
  image: quay.io/my-org/my-repo/argoexec:latest
  command: /bin/argoexec-fips

Then the Argo Workflows code would do something like:

func (woc *wfOperationCtx) newInitContainer(tmpl *wfv1.Template) apiv1.Container {
	ctr := woc.newExecContainer(common.InitContainerName, tmpl)
	if len(ctr.Command) == 0 {
		ctr.Command = []string{"argoexec"}
	}

	ctr.Command = append(ctr.Command, "init")
	ctr.Command = append(ctr.Command, woc.getExecutorLogOpts()...)

	return *ctr
}

[mprahl]

@Joibel did you get a chance to see my previous comment? Thanks for your help.

[terrytangyuan]

Alan is on vacation and will be back next week

[mprahl]

@Joibel did you get a chance to see my previous comment? Thanks for your help.