Actions seem to be processed in reverse order?

[stephenreay]

I'm trying to debug an issue we're seeing in a production environment: a csync2 group has several actions defined, in a specific order:

• action 1 sets the correct ownership of the file via chown;
• action 2 runs setfacl to ensure correct filesystem ACLs are in place;
• action 3 runs a custom script via sudo -u ... to do some processing on the file;

After running some sync's with -v -v it seems that csync2 is reading the actions in the reverse order, and thus executing them in reverse order, which fails because the script is run as a user who doesn't yet own the file.

Is this intended behaviour, or have we discovered some kind of bug? Is there an explicit way to specify the order the actions should be performed?

I looked quickly in action.c and there's nothing obvious there to my eyes, about why it would be reversing the order, but I also don't really know enough about how the config parser works to know that it isn't being parsed backwards.

Lastly - if the order is not deterministic currently, would you be open to adding the ability to make it deterministic? I'm happy to try and help with a patch for this, but C really isn't my day job, so it's quite possible that even a patch which "works" will need to be fixed to make it usable/safe/what have you...

[stephenreay]

Ok having done more testing, and looking again at the config parser with fresh eyes, I see in https://github.com/LINBIT/csync2/blob/master/cfgfile_parser.y#L290, and my understanding would be that "->next" is set to the existing "action" on the current group, essentially meaning that "next" really means "previous", and thus a config file like

action {
    exec "chown foo %%"
}

action {
    exec "setfacl group:bar:rwX" %%
}

action {
    exec "sudo -u foo do-something %%"
}

Iteratively produces a config structure that first looks like:

group {
    ...
    action: {
        exec: "chown foo %%"
    }
}

then looks like:

group {
    ...
    action: {
        exec: "setfacl group:bar:rwX" %%
        next: {
            exec: "sudo -u foo do-something %%"
        }
    }
}

and finally looks like:

group {
    ...
    action: {
        exec: "sudo -u foo do-something %%"
        next: {
            exec: "setfacl group:bar:rwX" %%
            next: {
                exec: "sudo -u foo do-something %%"
                next: null
            }
        }
    }
}

Am I reading this correctly? Is the solution simply to put the actions in reverse order?

Yes I also noticed that when fetching the actions to run it groups by the log file used, which would further complicate the ordering of things - I'm just focussed on the case where they all use the same (or no) log file for now.

Even if nothing is done to change how this behaves, I think the behaviour ought to be documented, so it's clearer to end-users how the actions get executed.

[stephenreay]

Huh.. I forgot to take into account when csync2 queries the values back out of the sync database, which introduces two surprising affects:

Because it does an ORDER BY command, logfile (I assume to remove duplicate entries, because the INSERT... of action blocks is called multiple times per file for some reason), it implicitly sorts by the command too, but then we also get into the scenario of how the result is handled: csync apparently converts the result into a nested structure, again using "->next" to signify the previous entry it found.

So it turns out that the initial order is essentially meaningless - the actions will be executed in reverse order after sorting on exec, logfile.

So to me this is extremely unintuitive, and problematic - a common solution for "file is owned by root" is "add a chown action", but that doesn't necessarily work if you need to run other commands on the file not as root.

I don't really think documentation is enough here. It can be worked around e.g. prefixing with a dummy env-var set so the command sorting is quasi-predictable, but the fact that it has to be done in reverse makes that even less obvious what's going on.

I'm going to try to make a patch to have it maintain the order as presented in the config file.. unfortunately it won't be right away, but hopefully I'll get to it sometime next month (April '24).