Enhance rotatelogs-compress.sh script, fix for symlinked log directories - #385
Enhance rotatelogs-compress.sh script, fix for symlinked log directories#385pastakhov wants to merge 1 commit into
rotatelogs-compress.sh script, fix for symlinked log directories#385Conversation
pastakhov
commented
Feb 25, 2026
pastakhov
commented
- improve string comparison
- add validation for environment variables
- refine log file cleanup and compression logic.
- Ensure proper handling of empty prefixes and trailing slashes in directory paths.
…ories * improve string comparison * add validation for environment variables * refine log file cleanup and compression logic. * Ensure proper handling of empty prefixes and trailing slashes in directory paths.
|
🐳 The image based on 33e2c0c8 commit has been built with |
| fi | ||
|
|
||
| # Fallback: derive prefix from file_to_compress basename to avoid empty prefix ("*") in find filters. | ||
| if [ -z "$commonFilePrefix" ] && [ -n "$file_to_compress" ]; then |
There was a problem hiding this comment.
Shouldn't we fail if the common prefix can not be determined?
It seems there must always be a common prefix as the /rotatelogs-compress.sh hook always gets two arguments as input, and the pattern we use here is a common prefix:
Taqasta/_sources/configs/mediawiki.conf
Line 39 in 2525572
So absence of a common prefix must be an indicator of something not working/configured as expected
| DELAY=0 | ||
| else | ||
| # Use random delay only when env value is a valid positive integer. | ||
| if [[ "${LOG_FILES_COMPRESS_DELAY:-0}" =~ ^[0-9]+$ ]] && [ "${LOG_FILES_COMPRESS_DELAY:-0}" -gt 0 ]; then |
There was a problem hiding this comment.
This logic (both old and new) around LOG_FILES_COMPRESS_DELAY looks confusing to me:
Use random delay only when env value is a valid positive integer.
The default value for the LOG_FILES_COMPRESS_DELAY is 3600
Taqasta/templates/final.Dockerfile
Line 42 in e67e99c
But the value we set to the ENV does not really make any sense as we use mod division by $RANDOM, instead we run a random delay whether the LOG_FILES_COMPRESS_DELAY is not equal 0, and that's quite confusing
--
I suggest removing the random part altogether and simply use the value of LOG_FILES_COMPRESS_DELAY for the delay
| fi | ||
| LOG_DIRECTORY="$(dirname -- "$file_to_compress")" | ||
| # Keep exactly one trailing slash (needed for symlink directory handling). | ||
| LOG_DIRECTORY="${LOG_DIRECTORY%/}/" |
There was a problem hiding this comment.
I don't think dirname -- "$file" may ever return a trailing slash, so this transformation seems unnecessary
| LOG_DIRECTORY="${LOG_DIRECTORY%/}/" | ||
|
|
||
| # Empty prefix would expand to "*" and affect unrelated files. | ||
| if [ -n "$commonFilePrefix" ]; then |
There was a problem hiding this comment.
Is there a scenario the prefix could be empty here? Also, see my comment above about failing if the common prefix could not be determined. If we'd like to account for a possible empty prefix (even after we tried a fallback) this check better to be around the same place where we do the fallback rather than here
| # remove old log files | ||
| if [ -n "${LOG_FILES_REMOVE_OLDER_THAN_DAYS:-}" ] && [ "${LOG_FILES_REMOVE_OLDER_THAN_DAYS}" != false ]; then | ||
| # Validate mtime value before passing it to find. | ||
| if [[ "${LOG_FILES_REMOVE_OLDER_THAN_DAYS}" =~ ^[0-9]+$ ]]; then |
There was a problem hiding this comment.
I suggest moving all input checks like that to the beginning of the file, and failing the script run if the inputs are invalid, sending the errors to stderr
2 participants
