out_logrotate: Validate max_files configuration in out_logrotate and improve directory creation and rotation logic.

Signed-off-by: SagiROosto <[email protected]>

Author: SagiROosto

plugins/out_logrotate/logrotate.c

@@ -200,6 +200,12 @@ static int cb_logrotate_init(struct flb_output_instance *ins,
                   ctx->gzip == FLB_TRUE ? "true" : "false",
                   ctx->out_path ? ctx->out_path : "not set");
 
+    if (ctx->max_files <= 0) {
+        flb_plg_error(ctx->ins, "invalid max_files=%d; must be >= 1", ctx->max_files);
+        flb_free(ctx);
+        return -1;
+    }
+
     return 0;
 }
 
@@ -511,6 +517,11 @@ static int mkpath(struct flb_output_instance *ins, const char *dir)
     }
     flb_plg_debug(ins, "creating directory %s", dup_dir);
     ret = mkdir(dup_dir, 0755);
+    if (ret == -1 && errno == EEXIST) {
+        if (stat(dup_dir, &st) == 0 && S_ISDIR(st.st_mode)) {
+            ret = 0;
+        }
+    }
     flb_free(dup_dir);
     return ret;
 #else
@@ -524,7 +535,13 @@ static int mkpath(struct flb_output_instance *ins, const char *dir)
         return ret;
     }
     flb_plg_debug(ins, "creating directory %s", dir);
-    return mkdir(dir, 0755);
+    ret = mkdir(dir, 0755);
+    if (ret == -1 && errno == EEXIST) {
+        if (stat(dir, &st) == 0 && S_ISDIR(st.st_mode)) {
+            ret = 0;
+        }
+    }
+    return ret;
 #endif
 }
 
@@ -875,6 +892,13 @@ static int rotate_file(struct flb_logrotate_conf *ctx, const char *filename)
             return -1;
         }
         lock_acquired = 1;
+
+        /* Check if rotation is still needed (race condition check) */
+        if (file_size < ctx->max_size) {
+            flb_lock_release(&entry->lock, FLB_LOCK_DEFAULT_RETRY_LIMIT,
+                             FLB_LOCK_DEFAULT_RETRY_DELAY);
+            return 0;
+        }
     }
 
     /* Log rotation event */

tests/runtime/out_logrotate.c

@@ -23,6 +23,7 @@
 void flb_test_logrotate_basic_rotation(void);
 void flb_test_logrotate_gzip_compression(void);
 void flb_test_logrotate_max_files_cleanup(void);
+void flb_test_logrotate_max_files_validation(void);
 void flb_test_logrotate_format_csv(void);
 void flb_test_logrotate_format_ltsv(void);
 void flb_test_logrotate_format_plain(void);
@@ -40,6 +41,7 @@ TEST_LIST = {
     {"basic_rotation",        flb_test_logrotate_basic_rotation},
     {"gzip_compression",      flb_test_logrotate_gzip_compression},
     {"max_files_cleanup",     flb_test_logrotate_max_files_cleanup},
+    {"max_files_validation",  flb_test_logrotate_max_files_validation},
     {"format_csv",            flb_test_logrotate_format_csv},
     {"format_ltsv",           flb_test_logrotate_format_ltsv},
     {"format_plain",          flb_test_logrotate_format_plain},
@@ -1272,3 +1274,54 @@ void flb_test_logrotate_max_files_cleanup(void)
     /* Clean up directory and all contents */
     recursive_delete_directory(TEST_LOGPATH);
 }
+
+void flb_test_logrotate_max_files_validation(void)
+{
+    flb_ctx_t *ctx;
+    int out_ffd;
+    char logfile[512];
+
+    /* Clean up any existing directory and contents */
+    recursive_delete_directory(TEST_LOGPATH);
+    mkdir(TEST_LOGPATH, 0755);
+    snprintf(logfile, sizeof(logfile), "%s/%s", TEST_LOGPATH, TEST_LOGFILE);
+
+    ctx = flb_create();
+    TEST_ASSERT(flb_service_set(ctx, "Flush", "1", "Grace", "1",
+                                 "Log_Level", "error", NULL) == 0);
+
+    /* Test with max_files = 0 */
+    out_ffd = flb_output(ctx, (char *) "logrotate", NULL);
+    TEST_CHECK(out_ffd >= 0);
+    TEST_ASSERT(flb_output_set(ctx, out_ffd,
+        "match", "test",
+        "file", logfile,
+        "max_files", "0",
+        NULL) == 0);
+
+    /* Start should fail */
+    TEST_CHECK(flb_start(ctx) == -1);
+    
+    flb_destroy(ctx);
+
+    /* Test with max_files = -1 */
+    ctx = flb_create();
+    TEST_ASSERT(flb_service_set(ctx, "Flush", "1", "Grace", "1",
+                                 "Log_Level", "error", NULL) == 0);
+
+    out_ffd = flb_output(ctx, (char *) "logrotate", NULL);
+    TEST_CHECK(out_ffd >= 0);
+    TEST_ASSERT(flb_output_set(ctx, out_ffd,
+        "match", "test",
+        "file", logfile,
+        "max_files", "-1",
+        NULL) == 0);
+
+    /* Start should fail */
+    TEST_CHECK(flb_start(ctx) == -1);
+
+    flb_destroy(ctx);
+    
+    /* Clean up directory */
+    recursive_delete_directory(TEST_LOGPATH);
+}