[Exec] Allow expanding envs at tester runtime

[amwat]

ref: kubernetes/test-infra#21331

kubetest2 passes a bunch of info through envs to the tester, but typical inline bash prow jobs imply that they will be expanded during deployer invocation and not tester invocation. This especially affects the exec tester.

Might be possible to quote it in some fancy way, but instead opted to support it directly in the tester.

Covered only the most common use cases and is not intended to cover all shell special character shenanigans.

Previously

$ export ARTIFACTS=/tmp
$ kubetest2 noop --test=exec -- echo "$ARTIFACTS"
$ /tmp
$ kubetest2 noop --test=exec -- echo '$ARTIFACTS'
$ $ARTIFACTS

Now

$ export ARTIFACTS=/tmp
$ kubetest2 noop --test=exec -- echo "$ARTIFACTS"
$ /tmp
$ kubetest2 noop --test=exec -- echo '$ARTIFACTS'
$ /tmp/<kubetest2_run_id>
$ kubetest2 noop --test=exec -- echo '\$ARTIFACTS'
$ $ARTIFACTS

Note: while running on the command line users still need to keep in mind the shell expansion itself.

/cc @spiffxp @BenTheElder @MushuEE

[amwat]

ping

[MushuEE]

Suggested change

	if strings.Contains(arg, `\$`) {
	expandedArgs[i] = strings.ReplaceAll(arg, `\$`, `$`)
	} else {
	expandedArgs[i] = os.ExpandEnv(arg)
	}
	if strings.Contains(arg, `\$`) {
	arg = strings.ReplaceAll(arg, `\$`, `$`)
	}
	expandedArgs[i] = os.ExpandEnv(arg)

flows a little better imo

[amwat]

we explicitly don't want to expand escaped $ hence the else?

[BenTheElder]

usually an expandenv function supports shell syntax like --foo=${BAR}.

we should look at how kubernetes implements this for pods for ecosystem consistency

[amwat]

It's using os.ExpandEnv inside so this syntax also works?

[amwat]

Also note:

This can currently be worked around by spawning a subshell (and letting it do the evaluation) as the test command instead of the actual test command.

kubetest2 noop --test=exec -- sh -c 'echo $ARTIFACTS'

But I think this (expanding envs at tester runtime) is going to be the more widely required use case, so it's better if the exec tester itself supports this and we can avoid having subshells in all the configs.

[spiffxp]

/approve
/lgtm
The syntax in the description is pretty straightforward to me re: when does my env get expanded

I sorta wonder if this should happen consistently for all testers though? Maybe exec is the only one that needs it for now

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amwat, spiffxp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [amwat,spiffxp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment