Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] [onedir] cmd.run runas cannot get environment #62565

Closed
d--j opened this issue Aug 29, 2022 · 3 comments
Closed

[BUG] [onedir] cmd.run runas cannot get environment #62565

d--j opened this issue Aug 29, 2022 · 3 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior needs-triage Phosphorus v3005.0 Release code name and version Regression The issue is a bug that breaks functionality known to work in previous releases.

Comments

@d--j
Copy link
Contributor

d--j commented Aug 29, 2022

Description
cmdmod.run uses sys.executable to execute some dynamic Python code to get the environment of the user the command should run as. See

salt/salt/modules/cmdmod.py

Lines 524 to 535 in 4bbdd65

else:
env_cmd = ("su", "-s", shell, "-", runas, "-c", sys.executable)
msg = "env command: {}".format(env_cmd)
log.debug(log_callback(msg))
env_bytes, env_encoded_err = subprocess.Popen(
env_cmd,
stderr=subprocess.PIPE,
stdout=subprocess.PIPE,
stdin=subprocess.PIPE,
).communicate(salt.utils.stringutils.to_bytes(py_code))
marker_count = env_bytes.count(marker_b)

This does not work in tiamat / onedir installations since sys.executable is /opt/saltstack/salt/run/run and /opt/saltstack/salt/run/run cannot handle piped in Python code.

Steps to Reproduce the behavior

  • Install salt 3005 (onedir)
  • salt-call cmd.run id runas=root
  • Output:
[ERROR   ] Environment could not be retrieved for user 'root': stderr='Must pass in a salt command, available commands are:\nminion\nmaster\ncall\napi\ncloud\ncp\nextend\nkey\nproxy\npip\nrun\nshell\nspm\nssh\nsupport\nsyndic\npython\n' stdout=''
local:
    uid=0(root) gid=0(root) groups=0(root)

Expected behavior
No error message. Salt is able to pick up the environment variables defined for the user.

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
          Salt: 3005
 
Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.0
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: Not Installed
        Python: 3.9.13 (main, Aug 23 2022, 18:31:04)
  python-gnupg: 0.4.8
        PyYAML: 5.4.1
         PyZMQ: 23.2.0
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: ubuntu 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.4.0-125-generic
        system: Linux
       version: Ubuntu 20.04 focal

Additional context

Other code is using sys.executable, too. These other occurrences must be fixed, too. If they just open a Python file, they can use /opt/saltstack/salt/run/run python for that.

@d--j d--j added Bug broken, incorrect, or confusing behavior needs-triage labels Aug 29, 2022
@OrangeDog OrangeDog added Regression The issue is a bug that breaks functionality known to work in previous releases. Phosphorus v3005.0 Release code name and version labels Aug 30, 2022
@Ch3LL Ch3LL self-assigned this Aug 30, 2022
@Ch3LL Ch3LL added this to the Phosphorus v3005.0 milestone Aug 30, 2022
@Ch3LL
Copy link
Contributor

Ch3LL commented Sep 13, 2022

would you mind testing from these packages here: https://gitlab.com/saltstack/employees/development/ch3ll/salt-pkg/-/pipelines/639005216 this pipeline is building the onedir packages with my change in #62617

EDIT: I just realized you might not have permissions for that, so I will let you know when the PR is merged in and the nightly builds build off of the change and you can test those packages

@Ch3LL
Copy link
Contributor

Ch3LL commented Sep 14, 2022

I have verified this is now working with the latest nightly build of packages:

(heist-3.8-1)  ch3ll@megan-precision5550  ~/Downloads/artifacts/3005+32.gb271eab491-1/salt  sudo -E ./run/run call --local cmd.run id runas=ch3ll
local:
    uid=1000(ch3ll) gid=1000(ch3ll) groups=1000(ch3ll),3(sys),90(network),98(power),991(lp),998(wheel)
(heist-3.8-1)  ch3ll@megan-precision5550  ~/Downloads/artifacts/3005+32.gb271eab491-1/salt  

I will close for now, but you can test them as well:

https://gitlab.com/saltstack/open/salt-pkg/-/pipelines/639419421

@Ch3LL Ch3LL closed this as completed Sep 14, 2022
@d--j
Copy link
Contributor Author

d--j commented Sep 14, 2022

I have tested the latest nightly you linked to. It works ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage Phosphorus v3005.0 Release code name and version Regression The issue is a bug that breaks functionality known to work in previous releases.
Projects
None yet
Development

No branches or pull requests

3 participants