An uncaught exception when add_copy_spec adds big files twice and reaches sizelimit

[pmoravec]

Under specific circumstances, collecting files from self.add_copy_spec(["/var/log/foo.*", "/var/log/foo.bar"])raises plugin's uncaught exception ValueError: path .. exists and is not a symbolic link.

Reproducer:

• create 11M /var/log/foo.bar file
• then create 55M /var/log/foo.txt file
• have a plugin having self.add_copy_spec(["/var/log/foo.*", "/var/log/foo.bar"])
• call sos report -o YOURPLUGIN --batch --build --log-size=60

The problem is, collecting /var/log/foo.* will collect whole /var/log/foo.txt and tail of /var/log/foo.bar, which creates the symlink var/log/foo.bar -> sos_strings/..... BUT then sos report attempts to collect whole /var/log/foo.bar that is under the limit alone, and fails..

The easiest approach is to ensure no such "duplicit copy_spec entries" can exist (see #3685), what is another requirement to plugin authors. Better approach would be overwrite symlinks to sos_strings (which means we collected tails instead of whole file) by full files - and dont raise exception "I want to create symlink (for a shorter file) but a real file (with bigger content) exists".

The "better approach" can have some gotchas I dont oversee. But it would resolve the problem of different plugins collecting same file and stepping on each other's toes (cf. https://github.com/sosreport/sos/blob/main/sos/report/plugins/apache.py#L64 as our "reaction").

[pierg75]

Would something like this be ok?
It fixes the exception in my tests, I did few tests and didn't see anything bad with it.

[pmoravec]

That will work only for a standalone plugin trying to grab the same file "twice". Original problem would be fixed, while the other issue behind apache.py#L64 would remain there.

No strong preference..

[pierg75]

I had some time to look at that "better approach". I'm probably missing a lot, but I think that the entries added by add_forbidden_path are excluded in add_copy_spec. Now, if I understood the gist of it, add_copy_spec will either add the files to tail_files_list or copy_paths. In case of copy_paths, this is a set, so there should be no duplicates, even if more plugins try to copy the same file. In case of the tailed files, this could be checked by the patch above (and I guess it could be made a set as well).
I guess I'm missing some steps.

[pmoravec]

copy_paths is a set per plugin, as well as forbidden_paths (which is a list, but still per plugin).

I feel the requirement to have a all-plugins-wide solution is too strong when we respect plugin's independence. So resolving the problem inside a plugin seems reasonable compromise to me.

The main...pierg75:pier-sosreport:do_not_copy_again_tailed_files will result in collecting just shorter content of the "conflicting" file, right? That can be OK, or maybe we can collect whole file (which would mean replacing symlink by full file, removing sos_strings file and updating self._tail_files_list at least - which sounds more fragile approach.

I would vote for the proposed patch as a sufficient solution.