utils: T7095: make vrf and netns arguments aware of the shell

[xeluior]

Change summary

Changes the vrf and netns functionality in the utils/process.py to be aware of whether the shell is required or not. Fixes a regression they introduced that broke callers passing a list rather than a string to the popen (and family) of util functions, notably breaking the git commit-archive functionality.

I also removed the auth parameter as it was not fully implemented, was not used, was not documented, and I could not find any examples of the form it would take, making it difficult to integrate into the refactor.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Related Task(s)

https://vyos.dev/T7095

Bug introduced in https://vyos.dev/T6975

Related PR(s)

How to test / Smoketest result

configure
set system config-management commit-archive location git://<git url>.git
commit

Prior to this patch, the commit-archive will fail with an error similar to the following.

Archiving config...
  git://<git url>.git Unable to upload "git://<git url>.git/config.boot-vyos-dev2.20250127_165226": [Errno 2] No such file or directory: " ['git', 'clone', '<git url>.git', '/tmp/git-commit-archive-0l3qtm5h/repository', '--depth=1']"
run-parts: /etc/commit/post-hooks.d/02vyos-commit-archive exited with return code 1

After this change the functionality works again.

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly

[github-actions]

❌
PR title does not match the required format

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[github-actions]

CI integration ❌ failed!

Details

CI logs

• CLI Smoketests (no interfaces) ❌ failed
• CLI Smoketests (interfaces only) 👍 passed
• Config tests 👍 passed
• RAID1 tests 👍 passed
• TPM tests 👍 passed

[Copilot]

PR Overview

This PR fixes a regression in the execution of commands involving VRF and network namespaces and removes the obsolete auth parameter.

• Removed the auth parameter from get_wrapper, popen, and cmd functions.
• Updated command construction by switching from string wrappers to list-based wrappers and using shlex.join for proper formatting.

Reviewed Changes

File	Description
python/vyos/utils/process.py	Adjusts VRF/netns command handling and removes obsolete auth usage.

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

Comments suppressed due to low confidence (1)

python/vyos/utils/process.py:182

• The 'get_wrapper' function now returns a list, so directly interpolating 'wrapper' in the f-string will print its Python list representation rather than a valid shell command. Consider using shlex.join(wrapper) (e.g., command = f'{shlex.join(wrapper)} {command}') to format the command correctly.

command = f'{wrapper} {command}'

[c-po]

Is there a reason to drop the auth argument?

[xeluior]

The auth arg is passed directly to the get_wrapper helper function. This PR converts the get_wrapper from returning a shell-style string to an exec-style list. At the time of making this PR, I couldn't find any examples of the auth arg being used anywhere else in the codebase. As such, I couldn't properly convert it from whatever string-format was expected to the new list format. I opted to drop it rather than potentially include a broken implementation.

If someone could provide an example of where the auth argument may be used I would be happy to re-implement it in the exec-list format.

[sever-sever]

If someone could provide an example of where the auth argument may be used I would be happy to re-implement it in the exec-list format.

Probably here

vyos-1x/src/op_mode/image_installer.py

Line 548 in d1e8dbd

cmd(vrf_cmd, auth=remote_auth)

Isn't it?

[xeluior]

Just got a chance to look at this and tbh the auth parameter still seems redundant to me. This is the line where the format is defined

vyos-1x/src/op_mode/image_installer.py

Line 545 in d1e8dbd

remote_auth = f'REMOTE_USERNAME={username} REMOTE_PASSWORD={password}'

which is pretty clearly just setting some environment variables. That's already done a few lines prior where the process's environment is updated by setting the vars in os.environ

vyos-1x/src/op_mode/image_installer.py

Line 539 in d1e8dbd

environ['REMOTE_USERNAME'] = username

but even if the subprocess that's opened isn't inheriting these for some reason they should probably be passed with the env parameter instead of a separate parameter.

I'm unable to find any other examples of the auth parameter being used (I did another look over just to be safe) so I'll push a commit here soon that changes this use to the env parameter and then I think it's safe to still remove the auth parameter.

[dmbaturin]

I was thinking the same. My plan is to make a PR that simply moves that bit to the command. Then your PR should become mergeable as is.

[xeluior]

Ah, that works too!

[sever-sever]

Any updates?

[xeluior]

Any updates?

Sorry, my GitHub email alerts weren't getting received properly. I'll get a new commit here soon to address the auth parameter.