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

[3006.x] Run salt minion with salt group permissions #64174

Merged
merged 10 commits into from
May 3, 2023

Conversation

dwoz
Copy link
Contributor

@dwoz dwoz commented Apr 26, 2023

Move the salt user creation to the common package shared all other salt packages.

  • Move salt user creation to common package.
  • Do not add user or group when it already exists.
  • Create user in pre scripts

What does this PR do?

What issues does this PR fix or reference?

Fixes: #64141
Fixes: #64158

Previous Behavior

Remove this section if not relevant

New Behavior

Remove this section if not relevant

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes/No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@dwoz dwoz requested a review from a team as a code owner April 26, 2023 21:20
@dwoz dwoz requested review from cmcmarrow and removed request for a team April 26, 2023 21:20
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title [WIP] 64141 and 64158 [3006.x][WIP] 64141 and 64158 Apr 26, 2023
@dwoz dwoz temporarily deployed to ci April 26, 2023 22:53 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 26, 2023 22:53 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 26, 2023 23:12 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 27, 2023 01:39 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 27, 2023 01:39 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 27, 2023 01:39 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 27, 2023 02:20 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 27, 2023 02:20 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 27, 2023 02:20 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 08:32 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 08:32 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 08:32 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 09:57 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 09:57 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 09:57 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 11:27 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 11:27 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 11:27 — with GitHub Actions Inactive
@dwoz dwoz changed the title [3006.x][WIP] 64141 and 64158 [3006.x][WIP] Run salt minion with salt group permissions Apr 29, 2023
@dwoz dwoz temporarily deployed to ci April 29, 2023 20:40 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 20:40 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci April 29, 2023 20:40 — with GitHub Actions Inactive
@barneysowood
Copy link
Contributor

@dwoz - thanks for the heads up, I hadn't spotted this before working on #64194.

  • The improvements on not creating the user/group if they already exist look good
  • I'm not sure the fix for [BUG] salt user HOME is in /home #64141 is the right one - why does the user need a home directory? Setting homedir to /nonexistent on debian derived systems and / on RH derived systems is a valid thing to do for service users. Having the user's homedir set to something it can't write to will avoid unexpected behaviour that could happen if any state (dotfiles changing env etc) that could occur if it has a user writable homedir. Also makes it harder for a compromise to persist state.
  • If you're going to create a shared group makes sense to do in the salt-common pkg, but I'd argue that the user the salt-master runs as should be created in the salt-master package. I'd also say it's a good opportunity to change that user from salt to salt-master - I think it's clearer that way as it is just the user that service runs as.
  • Is creating a shared group the way to solve [BUG] 3006 Inconsistency with minion and master permissions on /etc/salt #64158? not sure the MasterMinion should be using config from /etc/salt/minion.d - it's separate from the normal minion that may be running on the master (which may not even connect to the local master).

@s0undt3ch s0undt3ch dismissed stale reviews from dmurphy18 and Ch3LL via 4627fd3 May 3, 2023 15:14
@dwoz dwoz temporarily deployed to ci May 3, 2023 15:48 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 15:48 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 15:48 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 16:41 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 16:41 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 16:41 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 17:17 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 17:17 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 17:17 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 18:26 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 18:27 — with GitHub Actions Inactive
@dwoz dwoz temporarily deployed to ci May 3, 2023 18:28 — with GitHub Actions Inactive
@tvb
Copy link

tvb commented May 26, 2023

@dwoz chipping in on this PR. I see the following while running 3006.1 with the user salt configured in the config:

2023-05-26 07:41:20,755 [salt.loaded.int.returner.local_cache:119 ][WARNING ][63] Could not write out jid file for job 20230526074119248612. Retrying.
2023-05-26 07:41:20,956 [salt.loaded.int.returner.local_cache:119 ][WARNING ][63] Could not write out jid file for job 20230526074119248612. Retrying.
2023-05-26 07:41:21,157 [salt.loaded.int.returner.local_cache:119 ][WARNING ][63] Could not write out jid file for job 20230526074119248612. Retrying.
2023-05-26 07:41:21,358 [salt.loaded.int.returner.local_cache:119 ][WARNING ][63] Could not write out jid file for job 20230526074119248612. Retrying.
2023-05-26 07:41:21,559 [salt.loaded.int.returner.local_cache:119 ][WARNING ][63] Could not write out jid file for job 20230526074119248612. Retrying.
2023-05-26 07:41:21,659 [salt.loaded.int.returner.local_cache:93  ][ERROR   ][63] prep_jid could not store a jid after 5 tries.
2023-05-26 07:41:21,660 [salt.utils.job   :78  ][CRITICAL][63] The specified 'local_cache' returner threw a stack trace
Traceback (most recent call last):
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/returners/local_cache.py", line 113, in prep_jid
    with salt.utils.files.fopen(os.path.join(jid_dir, "jid"), "wb+") as fn_:
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/utils/files.py", line 393, in fopen
    f_handle = open(*args, **kwargs)  # pylint: disable=resource-leakage
FileNotFoundError: [Errno 2] No such file or directory: '/var/cache/salt/master/jobs/8a/9e83fb30bd4f8d7deafa1b04b97105a51f992c919ea149fb10bb0afcc6672a/jid'
Traceback (most recent call last):
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/utils/job.py", line 72, in store_job
    mminion.returners[jidstore_fstr](False, passed_jid=load["jid"])
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1232, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1247, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/returners/local_cache.py", line 121, in prep_jid
    return prep_jid(
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/returners/local_cache.py", line 121, in prep_jid
    return prep_jid(
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/returners/local_cache.py", line 121, in prep_jid
    return prep_jid(
  [Previous line repeated 2 more times]
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/returners/local_cache.py", line 94, in prep_jid
    raise salt.exceptions.SaltCacheError(err)
salt.exceptions.SaltCacheError: prep_jid could not store a jid after 5 tries.
2023-05-26 07:41:21,660 [salt.utils.job   :143 ][CRITICAL][63] The specified 'local_cache' returner threw a stack trace
Traceback (most recent call last):
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/utils/job.py", line 141, in store_job
    mminion.returners[fstr](load)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 149, in __call__
    return self.loader.run(run_func, *args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1232, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1247, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/returners/local_cache.py", line 144, in returner
    os.makedirs(hn_dir)
  File "/opt/saltstack/salt/lib/python3.10/os.py", line 215, in makedirs
    makedirs(head, exist_ok=exist_ok)
  File "/opt/saltstack/salt/lib/python3.10/os.py", line 225, in makedirs
    mkdir(name, mode)
PermissionError: [Errno 13] Permission denied: '/var/cache/salt/master/jobs/8a/9e83fb30bd4f8d7deafa1b04b97105a51f992c919ea149fb10bb0afcc6672a'

Looking at the permissions of the directories, the are all set to salt:salt:

sh-4.2# ls -la /var/cache/salt/master/
total 72
drwxr-x--- 1 salt salt 4096 May 26 07:42 .
drwxr-xr-x 1 salt salt 4096 May 25 07:50 ..
-r-------- 1 salt salt    0 May 26 07:42 .dfn
-rw------- 1 root root   76 May 26 07:35 .root_key
-rw------- 1 salt root   76 May 26 07:35 .salt_key
drwxr-xr-x 1 root root 4096 May 25 07:51 extmods
drwxr-xr-x 3 salt salt 4096 May 26 07:37 file_lists
drwx------ 3 salt salt 4096 May 26 07:36 files
drwxr-xr-x 5 salt salt 4096 May 26 07:36 gitfs
drwxr-x--- 1 salt salt 4096 May 26 07:42 jobs
drwxr-xr-x 3 salt salt 4096 May 26 07:37 minions
drwxr-x--- 2 salt salt 4096 May  5 18:05 proc
drwxr-x--- 2 salt salt 4096 May  5 18:05 queues
drwxr-x--- 2 salt salt 4096 May  5 18:05 roots
drwxr-x--- 2 salt salt 4096 May  5 18:05 syndics
drwxr-x--- 2 salt salt 4096 May  5 18:05 tokens
sh-4.2# ls -la /var/cache/salt/master/jobs/
total 572
drwxr-x--- 1 salt salt 4096 May 26 07:45 .
drwxr-x--- 1 salt salt 4096 May 26 07:46 ..
drwxr-xr-x 4 salt salt 4096 May 26 07:41 00
drwxr-xr-x 3 salt salt 4096 May 26 07:43 01
drwxr-xr-x 4 salt salt 4096 May 26 07:42 02
drwxr-xr-x 3 salt salt 4096 May 26 07:41 05
drwxr-xr-x 3 salt salt 4096 May 26 07:42 07
drwxr-xr-x 3 salt salt 4096 May 26 07:40 0b
drwxr-xr-x 3 salt salt 4096 May 26 07:37 0c
drwxr-xr-x 6 salt salt 4096 May 26 07:42 0e
drwxr-xr-x 3 salt salt 4096 May 26 07:37 0f
drwxr-xr-x 4 salt salt 4096 May 26 07:45 14

Is this PR present in 3006.1 already?

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

Successfully merging this pull request may close these issues.

[BUG] 3006 Inconsistency with minion and master permissions on /etc/salt [BUG] salt user HOME is in /home
7 participants