test: Allow Storm to accept any env vars, not just proxy vars

[ayaegashi]

🔍 Description

This PR allows for Storm to accept a list of env vars with which to run Trident, not just HTTP(S)_PROXY

📝 Checks

pr-e2e: https://dev.azure.com/mariner-org/ECF/_build/results?buildId=932743&view=results
trident-testing (baremetal, example with multiple env vars): https://dev.azure.com/mariner-org/ECF/_build/results?buildId=931964&view=results

[ndubchak]

So, these BAREMETAL_ENV_VARS will include other var-s, and HTTPS_PROXY will only be one of them? Just curious why this change is needed

[ayaegashi]

Yep, that's correct. This PR is in response to this comment on my previous PR a couple weeks back: #86 (comment) I didn't see this comment until recently haha

[frhuelsz]

I would suggest that everything in the pipeline stays mostly the same, otherwise there are many edge cases not being considered, and only when we get to invoking the storm binary we transform it into the proxy, ie

storm-bin ... --env HTTPS_PROXY=${{ parameters.HTTPS_PROXY }} ...

[alejandro-microsoft]

I think this could be a little misleading. The Go code was modified to receive the list of env variables, and I think each is received with the same flag, but here the parameter is just for the httpsProxy and it will use the env-vars flag. Changing the name here, would imply these are all the env variables, but more can be added by using the same flag too. This parameter only carries the httpsProxy value, but maybe the idea would be to store the rest here too (if so, I think you may need to handle the input in Go in a different way).

[ayaegashi]

That's true, although I was thinking that any env vars used for clean install would be the same ones used during a/b updates as well so it makes sense to set them once in baremetal-testing.yml and then just receive and use them here.
I think if we needed to add another env var in this template, we can also just append to the parameter env var and pass the whole list.

[alejandro-microsoft]

If we needed to add another env var in this template, we can append to the parameter env var and pass the whole list.

I definitely, agree with this approach, but I'm unsure if with the current code the whole list of env vars can be passed in one string with just the flag at the beginning.

For example, can you pass just the flag and then the list of env variables (space separated or comma separated):
-env-vars HTTPS_PROXY=http://0.0.0.0 env-var2 env-var3
or do you need to precede each variable with the flag:
-env-vars HTTPS_PROXY=http://0.0.0.0 --env-vars env-var2 --env-vars env-var3
?

If it is the former, I see no problem. For the latter, it may be a good idea to explicitly add each env variable, change how the flags are added, or simply add a comment for clarification.

[ayaegashi]

Either format should work, so both --env-vars VAR_ONE="var_one",VAR_TWO="var_two" and --env-vars VAR_ONE="var_one" --env-vars VAR_TWO="var_two should lead to the equivalent outcome.

I think though as you said it may be confusing, so I switched the parameter back to httpsProxy :)

[Copilot]

Pull Request Overview

This PR refactors the Storm tool to accept multiple environment variables when running Trident, expanding beyond just proxy variables to support any arbitrary environment variables.

• Changed function signatures to accept []string for environment variables instead of a single proxy string
• Updated command-line flag from --proxy to --env-vars with support for multiple values
• Modified environment variable handling logic to process arrays of key-value pairs

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tools/storm/utils/trident.go	Core logic changes to handle multiple env vars in Trident command building and invocation
tools/storm/helpers/ab_update.go	Updated struct field and function calls to use new env vars parameter
.pipelines/templates/stages/testing_common/e2e-test-run.yml	Pipeline configuration updates to use new --env-vars flag syntax
.pipelines/templates/stages/testing_common/e2e-ab-update-stage-finalize-test-run.yml	Pipeline configuration updates to use new --env-vars flag syntax

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

This code doesn't handle environment variables without an equals sign (e.g., just 'PATH'). strings.Split(v, "=")[0] will return the entire string if no '=' is found, but the check key != "" will always pass for non-empty strings, potentially causing issues with malformed env vars.

Suggested change

	if key := strings.Split(v, "=")[0]; key != "" {
	envKeys = append(envKeys, key)
	if strings.Contains(v, "=") {
	if key := strings.SplitN(v, "=", 2)[0]; key != "" {
	envKeys = append(envKeys, key)
	}

[Copilot]

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}