Bug 2217079

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.

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?

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.

> 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.

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.

(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.

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.

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