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

Fix config load when HOME="/" #2173

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

robert-fraczek
Copy link

There is an issue i found on FreeBSD systems with starting choria-server via "service choria-server start" command causing global configuration doesn`t load. Same problem occur after OS reboot. Moreover i discover that starting via /usr/local/etc/rc.d/choria-server work properly.
Problem is with env HOME set to "/" when starting with service command ( rc.d set HOME="/root" ).

There is an issue with logic discovering local "home" configuration that prevent global configs to load.
Matching $HOME as prefix to config path when $HOME is "/" will always be true preventing to load global configs - as a quick fix i propose to make an exception for that.

@@ -410,7 +410,7 @@ func (c *Config) UnParsedOptions() map[string]string {
func (c *Config) dotdDir() string {
if !forceDotParse {
home, err := iu.HomeDir()
if err == nil {
if err == nil && home != "/" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like quite a big special case here that just doesnt seem like it belong. Should we fix FreeBSD rc scripts instead @smortex ?

@smortex
Copy link
Member

smortex commented May 17, 2024

@robert-fraczek, I am not sure to follow, and I can't reproduce your problem.

Are you saying that the home directory of the root user is the root of the filesystem, i.e. / instead of the usual /root?

Forcing HOME to / does not seems to cause any trouble as far as I can see:

# HOME=/ choria server run --config /usr/local/etc/choria/server.conf

Maybe you can describe the whole configuration files you have because my setup is probably quite different than yours?

@robert-fraczek
Copy link
Author

Some additional comments and obervations:

  • Problem is replicable also on other OS, eg: Ubuntu 22:
HOME="/" choria server --debug --config=/etc/choria/server.conf
{"component":"server","identity":"robert-lab.ams.creativecdn.net","level":"error","msg":"Initial Choria Broker connection failed: could not create connector: context canceled","time":"2024-05-20T04:23:11Z"}
{"level":"fatal","msg":"Could not run Choria: initial Choria Broker connection failed: could not create connector: context canceled","time":"2024-05-20T04:23:11Z"}
{"component":"server","connection":"robert-lab.ams.creativecdn.net","identity":"robert-lab.ams.creativecdn.net","level":"warning","msg":"Initial connection to the Broker failed on try 1: dial tcp: lookup **puppet** on 127.0.0.53:53: server misbehaving","time":"2024-05-20T04:23:17Z"}

choria trying to connect to "puppet" server which doesnt exist

HOME="/root" choria server --debug --config=/etc/choria/server.conf
{"component":"server","identity":"robert-lab.ams.creativecdn.net","level":"info","msg":"Choria Network Brokers: \"nats://choria-satellite.ams.rtbhouse.net:4222\"","time":"2024-05-20T05:03:24Z"}
{"component":"server","connection":"robert-lab.ams.creativecdn.net","identity":"robert-lab.ams.creativecdn.net","level":"info","msg":"Connected to nats://choria-satellite.ams.rtbhouse.net:4222","time":"2024-05-20T05:03:24Z"}

And everything is ok, broker url is taken from:

# cat /etc/choria/plugin.d/choria.cfg 
middleware_hosts = choria-satellite.ams.rtbhouse.net:4222
  • I`m not saying that root homedir is "/" - i just found that default FreeBSD 14 configuration setup HOME as "/" when starting choria-server via service command
  • IMHO there are two things
    • HOME rather shouldnt be set "/", but i saw that many times in unix world
    • solution in choria matching prefix of HOME as part of config file is also wrong as it leads to wrong assumpion in case of HOME="/"
    • i think that both: rc.d script in FreeBSD should be set to /root to be consistent with /usr/local/etc/rc.d/choria-server start and also choria should be fixed either by excluding "/" like i propose or by changing discovery method of loading config from home dir

@smortex
Copy link
Member

smortex commented May 21, 2024

Thank you for the details, I think I better understand what is going on, and as you say, I think it is the combination of multiple issues that break your use case.

I have no issue with my configuration where I only rely on plugin.choria.srv_domain in the system-wide configuration file /usr/local/etc/choria/server.conf. The fact that the server capacity of loading your plugin.d/choria.cfg depends on the $HOME environment variable feels wrong, especially when the path in this variable does not appear in the path to the configuration file.

About HOME=/ when using service(8) on FreeBSD

This is documented in the service(8) man page:

When used to run rc.d scripts the service command sets HOME to /

It was introduced in:
freebsd/freebsd-src@ee55fdb

It is unlikely to change, and I would not expect a system-wide service like the choria server to rely on the value of $HOME for anything. Changing the behavior of the choria rc.d script is of course possible, but that does not feel the right fix: the program should rather work regardless of the value of the HOME environment variable.

How HOME=/ affects choria

When run as a client (choria req ...), choria can load additional configuration files form the current working directory and its parent directories up to the root of the file system. All the loaded configuration files are listed in the output of choria tool config, and indeed this list unexpectedly change when HOME is set to something that match the beginning of the path of the "User Config" file:

romain@zappy ~/Opus-Codium % echo $HOME
/home/romain/Opus-Codium
romain@zappy ~/Opus-Codium % choria tool config | awk '/Configuration Files/,/Security Configuration/'
Configuration Files: 

           User Config: /usr/local/etc/choria/client.conf
          Loaded Files: /usr/local/etc/choria/client.conf
                        /usr/local/etc/choria/plugin.d/actionpolicy.cfg
                        /usr/local/etc/choria/plugin.d/choria.cfg
                        /usr/local/etc/choria/plugin.d/puppet.cfg
                        /home/romain/choria.conf
                        /home/romain/Opus-Codium/choria.conf

Security Configuration: 
romain@zappy ~/Opus-Codium % HOME=/ choria tool config | awk '/Configuration Files/,/Security Configuration/'
Configuration Files: 

           User Config: /usr/local/etc/choria/client.conf
          Loaded Files: /usr/local/etc/choria/client.conf
                        /home/romain/choria.conf
                        /home/romain/Opus-Codium/choria.conf

Security Configuration: 
romain@zappy ~ % HOME=/usr choria tool config | awk '/Configuration Files/,/Security Configuration/'
Configuration Files: 

           User Config: /usr/local/etc/choria/client.conf
          Loaded Files: /usr/local/etc/choria/client.conf
                        /home/romain/choria.conf
                        /home/romain/Opus-Codium/choria.conf

Security Configuration: 
romain@zappy ~/Opus-Codium % 

I recall we did not want to load plugins.d/* config from all the user directories when loading extra configuration files, but the code you change predate that and is from #1105.

Giving it a try, if I write a file ~/.choriarc, this one is loaded instead of the system-wide client.conf file, but ~/plugin.d/* config files are not loaded. Because the system-wide config is not loaded either, that hardly make sense to me.

If we remove the whole conditional block, the code become independent of the value of $HOME, and one can totally ignore system-wide config with his own config files in ~/.choriarc and ~/plugins.d. Do you know if this would cause some kind of issue @ripienaar?

@ripienaar
Copy link
Member

The problem is we cant totally ignore /etc/.../plugins.d as many of the module plugins set their settings there and some are even application settings. Thus if we skip them we'd skip important things. This led to undefined behaviour simply by trying to override some settings in your home dir.

Essentially think of /etc/.../plugins.d being puppet managed settings that should be enabled for the system to work. But puppet wouldnt manage them anywhere else so the current behaviour kind of achieves that.

The behaviour of the choria.conf you added @smortex is much better and more useful, but we're kind of stuck with this mess around the other files.

smortex added a commit that referenced this pull request May 22, 2024
Some plugins need to load configuration that is managed system-wide by
Puppet even when the user is using their own configuration file.
Regardless of the configuration file loaded (system one or user one), we
want to load these plugins configuration from the system.

We previously inferred the plugins configuration directory relatively to
the rest of the configuration, but ignored it if it was located in the
user HOME directory.  This was not correct, and even caused more trouble
when the HOME environment variable was '/' as it is commonly on some
systems for services, because plugins configuration was never loaded on
these systems.

Fix this by detecting the location of the system configuration and
unconditionally loading plugins configuration from this directory.
@smortex
Copy link
Member

smortex commented May 22, 2024

I struggled with go to try to implement what I understand to be the correct behavior in #2174: determine the system-wide location for choria config, and use this to load plugins regardless of the configuration being used.

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

Successfully merging this pull request may close these issues.

3 participants