Skip to content

Conversation

@HarrisonWAffel
Copy link
Contributor

Issue: rancher/rancher#50041

Passing a NO_PROXY value which includes spaces has the potential to break the upgrade script. This is due to run.sh iterating across previously set environment variables using white space as a delimiter, instead of new lines, and due to how install.sh parses the set NO_PROXY environment variable when crafting curl requests.

From what I can determine, including whitespace in NO_PROXY is a misconfiguration of the variable. Unfortunately, documentation doesn't explicitly state the expected format of NO_PROXY. However, all examples of NO_PROXY in Rancher strictly follow the expected Go format (comma-delimited entries). I have an associated docs PR up to clarify the expected value.

At the moment, In the event that NO_PROXY is not already exported, and needs to be reapplied using the value in rancher-system-agent.env, only the contents before the first whitespace character will be properly set to NO_PROXY. The remainder of the values will be exported individually, with no effect, polluting the SUC job logs.

This change updates run.sh to add best effort handling of this case, and to print a useful warning message when a malformed value is provided. It also updates the loop to break on newlines, and not whitespace.

Testing

After making these changes I built a new system-agent SUC image and tested it against a local custom cluster. I was able to confirm that no change in behavior occurs when properly setting NO_PROXY, and that an upgrade to a system-agent with this change does not result in a restart of rke2.

When NO_PROXY included one or more spaces, I confirmed that the expected warning message was printed to the job logs and that the errant spaces were removed before exporting NO_PROXY and invoking install.sh.

@HarrisonWAffel HarrisonWAffel requested a review from a team July 1, 2025 14:32
@HarrisonWAffel
Copy link
Contributor Author

closed in favor of a webhook update rancher/webhook#962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants