Bug 2217079 - Not possible to configure the temporary directory to be used on clients by remote execution in pull mode
Summary: Not possible to configure the temporary directory to be used on clients by re...
Keywords:
Status: NEW
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: Remote Execution
Version: 6.12.4
Hardware: Unspecified
OS: Unspecified
high
medium
Target Milestone: Unspecified
Assignee: satellite6-bugs
QA Contact: Satellite QE Team
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-06-23 19:21 UTC by Joniel Pasqualetto
Modified: 2023-08-15 09:41 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github theforeman foreman_ygg_worker pull 13 0 None open Use default tmpdir for temporary files 2023-08-07 15:16:10 UTC
Red Hat Issue Tracker SAT-18608 0 None None None 2023-06-29 10:39:43 UTC
Red Hat Knowledge Base (Solution) 2419861 0 None None None 2023-06-26 10:03:42 UTC

Description Joniel Pasqualetto 2023-06-23 19:21:34 UTC
Description of problem:

When using rex in pull mode, foreman_ygg_worker always use /tmp as the temporary directory for the script that will be executed.

For environments when /tmp is mounted with noexec, this is a blocker.

Currently, no configuration option is available to change this path.

Version-Release number of selected component (if applicable):

Satellite 6.12
Satellite 6.13

How reproducible:

Always, when /tmp is mounted with noexec

Steps to Reproduce:

1. Configure REX in pull mode on satellite
2. Configure a client to consume the REX queue
3. Mount /tmp on the client with "noexec"
4. Try running any job

Actual results:

~~~
 1:
/bin/sh: /tmp/ygg_rex857528743: Permission denied
   2:
/bin/sh: line 0: exec: /tmp/ygg_rex857528743: cannot execute: Permission denied
   3:
Exit status: 126
   4:
StandardError: Job execution failed
~~~

Error to execute the script, due to noexec set on /tmp. No way to modify the path

Expected results:

A way to use another directory as workdir for the worker, so jobs can run.

Additional info:

/tmp appears to be hardcoded on the foreman_ygg_worker code. See https://github.com/theforeman/foreman_ygg_worker/blob/main/src/runner.go#L62

Having a variable there would be ideal.

Comment 2 Ewoud Kohl van Wijngaarden 2023-06-26 09:11:56 UTC
Reading https://pkg.go.dev/io/ioutil#TempFile the ioutil.TempFile function is deprecated and os.CreateTemp is now the new thing. https://pkg.go.dev/os#CreateTemp states:

> If dir is the empty string, CreateTemp uses the default directory for temporary files, as returned by TempDir.

https://pkg.go.dev/os#TempDir states:

On Unix systems, it returns $TMPDIR if non-empty, else /tmp.

So that should be used instead of hardcoding /tmp. Then it can be configured using a commonly used environment variable.

Comment 4 Adam Ruzicka 2023-06-26 11:25:00 UTC
What is the expectation? Is it that it should honor the remote_working_dir tunable as described in KCS 2419861 or that one should be able to configure this on each client separately?

Comment 5 Sayan Das 2023-06-26 11:30:24 UTC
From my point of view ( or the case engineers point of view ):

* foreman_ygg_worker/yggdrasil should allow setting a different temporary directory than /tmp ( if CU has hardened their client systems ) [ Perhaps something via the config.toml ]

* At satellite side:
    * remote_working_dir should be honored by pull_mqtt mode 
    * Using the value of remote_working_dir, Something additional should happen to generate and deploy a new config.toml on the client systems and restart yggdrasild on them.

    If this has to be a manual work, we will need to document that in the same KB or somewhere in the docs itself.

Comment 6 Adam Ruzicka 2023-06-26 13:08:44 UTC
> Using the value of remote_working_dir, Something additional should happen to generate and deploy a new config.toml on the client systems and restart yggdrasild on them.

I'd do one or the other. Changing a config file on a capsule and then having to propagate the changes to all the clients, which would also mean the clients would have to modify their own configuration and restart themselves feels overly complicated and fragile. Especially when considering you don't have to do anything else for non-pull rex.

Which means if remote_working_dir should be honored then it would get sent to the client as a parameter of the job and then the client would honor it.

Comment 7 Joniel Pasqualetto 2023-06-27 12:10:14 UTC
My expectation is to have one way to configure the tmp dir used by the client.

I really like this approach of sending the remote_working_dir as a parameter of the job and the client honoring it.

Comment 8 nicholas.j.teodosio.ctr 2023-07-28 15:04:43 UTC
(In reply to Sayan Das from comment #5)
> * At satellite side:
>     * remote_working_dir should be honored by pull_mqtt mode 
>     * Using the value of remote_working_dir, Something additional should
> happen to generate and deploy a new config.toml on the client systems and
> restart yggdrasild on them.
> 
>     If this has to be a manual work, we will need to document that in the
> same KB or somewhere in the docs itself.

It wouldn't be the smoothest thing, but if pull_mqtt mode honors remote_working_dir, can't you force a re-register of the clients to update the config.toml? I know this isn't an option for everyone, just a thought.


Just to chip in my two cents this is affecting a hardened system I manage. In order to use the pull_mqtt mode currently we actually have to adjust our settings on every system we want to manage. Fixing this would be extremely helpful.

Comment 9 Ewoud Kohl van Wijngaarden 2023-07-28 15:18:41 UTC
I'm not sure remote_working_dir even makes sense. In SSH context it may make sense but with the ygg worker you have a daemon that runs persistent. I'd prefer that it respects TMPDIR and be hardened using systemd's PrivateTmp=yes. I think https://github.com/theforeman/foreman_ygg_worker/pull/13 would address at least the TMPDIR part. As for hardening I think that needs to happen in https://github.com/RedHatInsights/yggdrasil/blob/main/data/systemd/yggdrasil.service.in but I can't really oversee the implications.

Comment 10 Adam Ruzicka 2023-07-28 16:12:42 UTC
I'm afraid your patch will not really change anything, apart from us not using a now deprecated function. Yggdrasil severely reduces the environment[1] before it kicks of the workers so it won't really matter what you set the TMPDIR env var to for the service as it will not reach the worker.

[1] - https://github.com/RedHatInsights/yggdrasil/blob/0.2.0/cmd/yggd/main.go#L319-L322


Note You need to log in before you can comment on or make changes to this bug.