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.