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] orchestration does not propagate user name #63148

Closed
amalaguti opened this issue Nov 30, 2022 · 9 comments
Closed

[BUG] orchestration does not propagate user name #63148

amalaguti opened this issue Nov 30, 2022 · 9 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Runners VMware

Comments

@amalaguti
Copy link

amalaguti commented Nov 30, 2022

Description
when a runner is invoked from an orchestration state it does not pass the "user", generating a "user": UNKNOWN.

Here you go a very simple example

# ORCH/test.sls
show:
  test.configurable_test_state:
    - changes: False
    - comment: this is a test

test_runner_metasyntetic:
  salt.runner:
    - name: test.metasyntactic
    - locality: uk
    - 
If you follow the events in the event bus, the salt.runner test_runner_metasyntetic invoked by the orchestration shows user: UNKNOWN.
salt/run/20221130134354984855/new	{
    ...
    "fun": "runner.state.orch",
    "fun_args": [
        "ORCH/test"
    ],
    "user": "root"
}
salt/run/20221130134414856176/args	{
    "_stamp": "2022-11-30T13:44:15.471747",
    "args": [],
    "kwargs": {
        "locality": "uk"
    },
    "name": "test.metasyntactic",
    "type": "runner"
}
salt/run/20221130134415911510/new	{
    "_stamp": "2022-11-30T13:44:15.940837",
    "fun": "runner.test.metasyntactic",
    "fun_args": [
        {
            "locality": "uk"
        }
    ],
    "jid": "20221130134415911510",
    "user": "UNKNOWN"
}
salt/run/20221130134415911510/ret	{
    "_stamp": "2022-11-30T13:44:16.154564",
    "fun": "runner.test.metasyntactic",
    "fun_args": [
        {
            "locality": "uk"
        }
    ],
    "jid": "20221130134415911510",
    "return": [
        "wibble",
        "wobble",
        "wubble",
        "flob"
    ],
    "success": true,
    "user": "UNKNOWN"
}
salt/run/20221130134354984855/ret	{
    "_stamp": "2022-11-30T13:44:16.380148",
    "fun": "runner.state.orch",
    "fun_args": [
        "ORCH/test"
    ],
 ...
    "success": true,
    "user": "root"
}

Setup
Salt Open, but affecting SSE also due it shows jobs activity by UNKNOWN user, and given it's really hard to distinguish if a job was part of an orchestration it generates some concerns.

Steps to Reproduce the behavior
Run the orch example included here and watch the event bus.

Expected behavior
Pass the user from orchestration to invoked runners.

Screenshots
Screen Shot 2022-11-30 at 10 56 22 AM

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
          Salt: 3004.2

Dependency Versions:
          cffi: 1.13.2
      cherrypy: Not Installed
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.10.3
       libgit2: Not Installed
      M2Crypto: 0.38.0
          Mako: Not Installed
       msgpack: 1.0.4
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.19
      pycrypto: Not Installed
  pycryptodome: 3.15.0
        pygit2: Not Installed
        Python: 3.8.13 (default, Apr  5 2022, 17:15:15)
  python-gnupg: Not Installed
        PyYAML: 5.3.1
         PyZMQ: 21.0.2
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.3

Salt Extensions:
        SSEAPE: 8.8.2.4

System Versions:
          dist: rhel 7.9 Maipo
        locale: utf-8
       machine: x86_64
       release: 3.10.0-1160.71.1.el7.x86_64
        system: Linux
       version: Red Hat Enterprise Linux Server 7.9 Maipo

Additional context
Add any other context about the problem here.

@amalaguti amalaguti added Bug broken, incorrect, or confusing behavior needs-triage labels Nov 30, 2022
@amalaguti
Copy link
Author

Same with 3005.1

@anilsil anilsil added this to the Sulphur v3006.0 milestone Jan 18, 2023
@dmurphy18 dmurphy18 self-assigned this Jan 23, 2023
@dmurphy18
Copy link
Contributor

@amalaguti Looking, thanks for the info on 3005.1 also has the issue

@dmurphy18 dmurphy18 assigned whytewolf and unassigned Ch3LL and dmurphy18 Feb 3, 2023
@whytewolf
Copy link
Collaborator

Alright. looking at it and it looks like the issue is that salt.runner is using saltutil.runner to run the runner stuff. and saltutil.runner is somehow stripping user. if i run saltutil.runner directly in salt-call or through salt the user also comes as UNKNOWN. but if i run through salt-run it keeps the user.

Looking deeper to see if i can find how to get saltutil.runner to preserve the user.

@whytewolf
Copy link
Collaborator

Still working on this. the problem runs much deeper than originally thought. user is getting striped in multiple places. by multiple different systems. saltutil.runner is just one level of the pub_data being stripped.

@dmurphy18
Copy link
Contributor

dmurphy18 commented Feb 9, 2023

Have a partial solution which has proved workable for some test cases but other test cases still elude us.
Need further time to ensure have covered all corner cases and to fully check that the implementation has not broken other parts of Salt.
Have a working fix for calling saltutil.runner through Salt, but calling salt.runner still is giving the wrong user.

A WIP PR will be pushed later tonight with the the saltutil.runner work to date

@dmurphy18
Copy link
Contributor

Have a fix for salt.runner, writing test case to ensure the fix can be tested by CI/CD

@dmurphy18
Copy link
Contributor

Have the test written for checking user is not UNKNOWN, but currently after review the fix for salt.runner is insufficient, since it reports the user as root (which is running the runner on the master) but the correct solution would be to report the user who instigated the runner being run, for example: 'amalaguti' rather than generic root.

Further work to be done on the fix

@dmurphy18
Copy link
Contributor

PR passing tests, and out for review

@garethgreenaway
Copy link
Contributor

Closed by #64030

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 Runners VMware
Projects
None yet
7 participants