naemon / naemon-core

Networks, Applications and Event Monitor
http://www.naemon.io/
GNU General Public License v2.0
151 stars 63 forks source link

Naemon complains on reading directory member #433

Closed VladimirBilik closed 1 year ago

VladimirBilik commented 1 year ago

Hi, in my Naemon setup, I run Naemon with our own config generator, that generates some config files in dedicated directory. In the Naemon's main config file I have this directory configured with 'cfg_dir' directive. From time to time, Naemon reloads its config while the config generator is creating new set of config files. Although the generator creates the new files with '.tmp' extension, Naemon loads full directory content, first checks file's readability and next checks if the file has an '.cfg' extension. If the file is renamed while Naemon is checking the file readability, it stops reloading entirely.

In the xodtemplate.c source code:

        /* process this if it's a non-hidden config file... */
        if (stat(file, &stat_buf) == -1) {
            nm_log(NSLOG_RUNTIME_ERROR, "Error: Could not open config directory member '%s' for reading.\n", file);
            closedir(dirp);
            return ERROR;
        }

        switch (stat_buf.st_mode & S_IFMT) {

        case S_IFREG:
            x = strlen(dirfile->d_name);
            if (x <= 4 || strcmp(dirfile->d_name + (x - 4), ".cfg"))
                break;

            /* process the config file */
            result = xodtemplate_process_config_file(file);

            if (result == ERROR) {
                closedir(dirp);
                return ERROR;
            }

            break;
...

I suggest to narrow checking file readability just to files with the '.cfg' extension by moving the check inside the 'switch' statement. I don't consider it as a bug (I can modify our config generator that it creates temporary files outside the Naemon's working directory), but I think it could lesser the probability of reading wrong config content.

What do you think about it?

Thank you.

Vladimir

sni commented 1 year ago

that won't work, the switch is based on the stat result. Because in case it's a directory it does not check for a .cfg extension. I'd say it's easier to generate the new config into a separate folder (on the same filesystem) and then rename it. Or let the tmp file start with a dot, then it will be ignored as well.

nook24 commented 1 year ago

I run Naemon with our own config generator

Funny to see how many different config generators are out there. I would probably write all the new configuration files into a new directory like cfg-<current-timestamp>and then move a symlink when all configuration files are generated. I would also recommend that you config generator is execution a config verification before restarting Naemon.

VladimirBilik commented 1 year ago

ok, you are right, I need to work out a better strategy for generatig and testing config files. Thank you.