From 782e199270fa51f1cb6590236415c8d4f10c68c2 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Wed, 21 Jan 2026 11:26:41 +0100 Subject: [PATCH 1/2] ignore temporary test key --- t/.gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/.gitignore b/t/.gitignore index 262011c4..dd6046b3 100644 --- a/t/.gitignore +++ b/t/.gitignore @@ -1 +1,4 @@ snclient + +# Temporary test key generated during "make citest" +test.key.tmp From f7da066df270d1679e0793b4271047fdcf2c75d2 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Thu, 22 Jan 2026 11:34:30 +0100 Subject: [PATCH 2/2] check_files , fix 'size' metric and other functions fix the sum of file sizes metric by inserting it as 'total_size' instead of 'size'. This fixes the problem where that condition collides with the file attribute 'size'. Three 5mb files would in total take up 15mb space, triggering a critical='size > 10mb' condition fix checkData if a pattern is used. we use the standard library walkDir function is used to add every directory, but files are checked and skipped if a pattern is present and file does not match it. this can lead to directories that dont have any matching files under them. add a function to trim them, and call it if a pattern is specified. add argument 'calculate-subdirectory-sizes'. the traversal functions add directories with size "0", and only add the files with their true size. this argument enables calculating the subdirectory function sizes and adding them as an attibute. implements the addSubdirectorySizes function add attribute "check_path". if user specified multiple search paths, each one of them add files/directories to the same listData. It is unclear which search paths traversal led to the entry being added. add that as an attribute. add search path metrics. by default, total_size is calculated from all files added to listData. add a new metric for each search path, it calculates the size sum of file sizes found for that search path. uses the "check_path" attribute. add subdir metrics. this will add metrics with the name "fullname size" , just like it does for the files "filename size" . currently it is toggled only if subdirectory sizes are calculated and a pattern is specified. --- docs/checks/commands/check_files.md | 64 +++---- pkg/snclient/check_files.go | 261 +++++++++++++++++++++++----- pkg/snclient/checkdata.go | 19 +- 3 files changed, 264 insertions(+), 80 deletions(-) diff --git a/docs/checks/commands/check_files.md b/docs/checks/commands/check_files.md index 43673415..09d3300d 100644 --- a/docs/checks/commands/check_files.md +++ b/docs/checks/commands/check_files.md @@ -58,14 +58,15 @@ Naemon Config ## Check Specific Arguments -| Argument | Description | -| --------- | --------------------------------------------------------------------------------------------------------- | -| file | Alias for path | -| max-depth | Maximum recursion depth. Default: no limit. '0' and '1' disable recursion and only include files/directories directly under path., '2' starts to include files/folders of subdirectories with given depth. | -| path | Path in which to search for files | -| paths | A comma separated list of paths | -| pattern | Pattern of files to search for | -| timezone | Sets the timezone for time metrics (default is local time) | +| Argument | Description | +| ---------------------------- | -------------------------------------------------------------------------------------- | +| calculate-subdirectory-sizes | For subdirectories that are found under the search paths, calculate the subdirectory sizes based on found files. This calculation may be expensive. Default: false | +| file | Alias for path | +| max-depth | Maximum recursion depth. Default: no limit. '0' and '1' disable recursion and only include files/directories directly under path., '2' starts to include files/directories of subdirectories with given depth. | +| path | Path in which to search for files | +| paths | A comma separated list of paths | +| pattern | Pattern of files to search for | +| timezone | Sets the timezone for time metrics (default is local time) | ## Attributes @@ -73,26 +74,27 @@ Naemon Config these can be used in filters and thresholds (along with the default attributes): -| Attribute | Description | -| --------------- | ------------------------------------------------- | -| path | Path to the file | -| filename | Name of the file | -| name | Alias for filename | -| file | Alias for filename | -| fullname | Full name of the file including path | -| type | Type of item (file or dir) | -| access | Unix timestamp of last access time | -| creation | Unix timestamp when file was created | -| size | File size in bytes | -| written | Unix timestamp when file was last written to | -| write | Alias for written | -| age | Seconds since file was last written | -| version | Windows exe/dll file version (windows only) | -| line_count | Number of lines in the files (text files) | -| total_bytes | Total size over all files in bytes | -| total_size | Total size over all files as human readable bytes | -| md5_checksum | MD5 checksum of the file | -| sha1_checksum | SHA1 checksum of the file | -| sha256_checksum | SHA256 checksum of the file | -| sha384_checksum | SHA384 checksum of the file | -| sha512_checksum | SHA512 checksum of the file | +| Attribute | Description | +| --------------- | -------------------------------------------------------------------- | +| path | Path to the file | +| filename | Name of the file | +| name | Alias for filename | +| file | Alias for filename | +| fullname | Full name of the file including path | +| type | Type of item (file or dir) | +| check_path | Check path argument whose search led to finding this file/directory. | +| access | Unix timestamp of last access time | +| creation | Unix timestamp when file was created | +| size | File size in bytes | +| written | Unix timestamp when file was last written to | +| write | Alias for written | +| age | Seconds since file was last written | +| version | Windows exe/dll file version (windows only) | +| line_count | Number of lines in the files (text files) | +| total_bytes | Total size over all files in bytes | +| total_size | Total size over all files as human readable bytes | +| md5_checksum | MD5 checksum of the file | +| sha1_checksum | SHA1 checksum of the file | +| sha256_checksum | SHA256 checksum of the file | +| sha384_checksum | SHA384 checksum of the file | +| sha512_checksum | SHA512 checksum of the file | diff --git a/pkg/snclient/check_files.go b/pkg/snclient/check_files.go index b2aa80f7..57cdda01 100644 --- a/pkg/snclient/check_files.go +++ b/pkg/snclient/check_files.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "runtime" + "slices" "strings" "syscall" "time" @@ -29,17 +30,19 @@ type FileInfoUnified struct { } type CheckFiles struct { - paths []string - pathList CommaStringList - pattern string - maxDepth int64 + paths []string + pathList CommaStringList + pattern string // constructor NewCheckFiles sets this as '*' + maxDepth int64 // constructor NewCheckFiles sets this as -1 + calculateSubdirectorySizes bool // constructor NewCheckFiles sets this as false } func NewCheckFiles() CheckHandler { return &CheckFiles{ - pathList: CommaStringList{}, - pattern: "*", - maxDepth: int64(-1), + pathList: CommaStringList{}, + pattern: "*", + maxDepth: int64(-1), + calculateSubdirectorySizes: false, } } @@ -57,8 +60,10 @@ func (l *CheckFiles) Build() *CheckData { "paths": {value: &l.pathList, description: "A comma separated list of paths", isFilter: true}, "pattern": {value: &l.pattern, description: "Pattern of files to search for", isFilter: true}, "max-depth": {value: &l.maxDepth, description: "Maximum recursion depth. Default: no limit. '0' and '1' disable recursion and only include files/directories directly under path." + - ", '2' starts to include files/folders of subdirectories with given depth. "}, + ", '2' starts to include files/directories of subdirectories with given depth. "}, "timezone": {description: "Sets the timezone for time metrics (default is local time)"}, + "calculate-subdirectory-sizes": {value: &l.calculateSubdirectorySizes, description: "For subdirectories that are found under the search paths, " + + "calculate the subdirectory sizes based on found files. This calculation may be expensive. Default: false"}, }, detailSyntax: "%(name)", okSyntax: "%(status) - All %(count) files are ok: (%(total_size))", @@ -72,6 +77,7 @@ func (l *CheckFiles) Build() *CheckData { {name: "file", description: "Alias for filename"}, {name: "fullname", description: "Full name of the file including path"}, {name: "type", description: "Type of item (file or dir)"}, + {name: "check_path", description: "Check path argument whose search led to finding this file/directory."}, {name: "access", description: "Unix timestamp of last access time", unit: UDate}, {name: "creation", description: "Unix timestamp when file was created", unit: UDate}, {name: "size", description: "File size in bytes", unit: UByte}, @@ -121,48 +127,36 @@ func (l *CheckFiles) Check(_ context.Context, _ *Agent, check *CheckData, _ []Ar } } - totalSize := uint64(0) - for _, data := range check.listData { - totalSize += convert.UInt64(data["size"]) + // Cleanup the listData if a filter is used + if l.pattern != "*" { + l.removeDirectoriesWithoutFilesUnder(check) } - if len(check.listData) > 0 || check.emptySyntax == "" { - check.details = map[string]string{ - "total_bytes": fmt.Sprintf("%d", totalSize), - "total_size": humanize.IBytesF(convert.UInt64(totalSize), 2), - } + if l.calculateSubdirectorySizes { + l.addSubdirectorySizes(check) } - if check.HasThreshold("count") { - check.result.Metrics = append(check.result.Metrics, - &CheckMetric{ - Name: "count", - Value: int64(len(check.listData)), - Warning: check.warnThreshold, - Critical: check.critThreshold, - Min: &Zero, - }) - } - if check.HasThreshold("size") || check.HasThreshold("total_size") { - check.result.Metrics = append(check.result.Metrics, - &CheckMetric{ - ThresholdName: "total_size", - Name: "size", - Value: totalSize, - Unit: "B", - Warning: check.warnThreshold, - Critical: check.critThreshold, - Min: &Zero, - }) + // file metrics are always added + l.addFileMetrics(check) + + // general metrics are always added + l.addGeneralMetrics(check) + + if len(l.paths) > 2 { + l.addSearchPathMetrics(check) } - l.addFileMetrics(check) + if l.calculateSubdirectorySizes && l.pattern != "*" { + l.addSubDirMetrics(check) + } return check.Finalize() } func (l *CheckFiles) addFile(check *CheckData, path, checkPath string, dirEntry fs.DirEntry, err error) error { - // if its a directory, checkPath is never added to the entry list + // if the search path is a directory e.g '/usr/bin' , the program assumes you are looking for files/subdirectories under it + // therefore it does not add the search path directory to the entry list + // if it is a file like /usr/bin/bash however, it will add that if dirEntry != nil && dirEntry.IsDir() && path == checkPath { return nil } @@ -170,12 +164,13 @@ func (l *CheckFiles) addFile(check *CheckData, path, checkPath string, dirEntry path = l.normalizePath(path) filename := filepath.Base(path) entry := map[string]string{ - "file": filename, - "filename": filename, - "name": filename, - "path": filepath.Dir(path), - "fullname": path, - "type": "file", + "file": filename, + "filename": filename, + "name": filename, + "path": filepath.Dir(path), + "fullname": path, + "type": "file", + "check_path": checkPath, } matchAndAdd := func() { @@ -193,7 +188,7 @@ func (l *CheckFiles) addFile(check *CheckData, path, checkPath string, dirEntry entry["type"] = "dir" if err != nil { - // silently skip failed sub folder. + // silently skip failed subdirectory. // If you continue on and the error is checked later, it will add error to the entry // This will make tests fail. return fs.SkipDir @@ -335,6 +330,180 @@ func checkSlowFileOperations(check *CheckData, entry map[string]string, path str return nil } +// The WalkDir normally adds every directory and files under the search path. +// If a pattern is specified, this prevents files that dont match the pattern to be skipped. +// This can lead to some directories being in the listData, while not having any matched files under them. +// This function cleans those directories up. +func (l *CheckFiles) removeDirectoriesWithoutFilesUnder(check *CheckData) { + fileFilepaths := make([]string, 0) + + for _, data := range check.listData { + if data["type"] == "file" { + fileFilepaths = append(fileFilepaths, data["fullname"]) + } + } + + newListData := make([]map[string]string, 0) + + for _, data := range check.listData { + // only filter the directories, files are automatically added + if data["type"] == "dir" { + hasFilesUnder := false + for _, fileFilepath := range fileFilepaths { + prefixToMatch := fmt.Sprintf("%s%c", data["fullname"], os.PathSeparator) + rest, found := strings.CutPrefix(fileFilepath, prefixToMatch) + if found && rest != "" { + hasFilesUnder = true + + break + } + } + if hasFilesUnder { + newListData = append(newListData, data) + } else { + log.Debugf("Skipping directory from the new listData, as it does not have any files found under it: %s", data["fullname"]) + } + } else { + newListData = append(newListData, data) + } + } + + check.listData = newListData +} + +// Files are checked by their individual attributes, with directories we have to count and size them up +// This function should be called with the final check.listData +func (l *CheckFiles) addGeneralMetrics(check *CheckData) { + // totalSize is always calculated, even if there are one/multiple search paths, and they point to files/directories + totalSize := uint64(0) + for _, data := range check.listData { + // directory entries could have their "size" set. + // this can lead to including a file multiple times in the count. only add files to totalSize + if data["type"] == "file" { + totalSize += convert.UInt64(data["size"]) + } + } + + // this is added to check.details and not a metric + if len(check.listData) > 0 || check.emptySyntax == "" { + check.details = map[string]string{ + "total_bytes": fmt.Sprintf("%d", totalSize), + "total_size": humanize.IBytesF(convert.UInt64(totalSize), 2), + } + } + + // files do not have a 'count' atrribute, so this wont collide like 'size' would. No need for 'totalCount' + if check.HasThreshold("count") { + check.result.Metrics = append(check.result.Metrics, + &CheckMetric{ + Name: "count", + Value: int64(len(check.listData)), + Warning: check.warnThreshold, + Critical: check.critThreshold, + Min: &Zero, + }) + } + + // entries in listData have a 'size' attribute. This is filled for files directly, with folders they have to be calculated after the walk has ended. + // total_size argument is the recommended way for thresholds, if they want to work with size summation of matched entries + if check.HasThreshold("total_size") { + check.result.Metrics = append(check.result.Metrics, + &CheckMetric{ + ThresholdName: "total_size", + Name: "total_size", + Value: totalSize, + Unit: "B", + Warning: check.warnThreshold, + Critical: check.critThreshold, + Min: &Zero, + }) + } + + if check.HasThreshold("size") { + log.Warn("check_files - Using 'size' in a threshold argument meant to mean \"summation of all found files sizes\" is wrong. " + + "This collides with each file entry 'size' attribute during checks. Using 'size' in a condition will check each files 'size' attribute. " + + "If you want to check for the sum of sizes, use 'total_size' in your condition instead. ") + } +} + +// this check might be called with multiple paths. calculate their sizes individually and add as a metric +func (l *CheckFiles) addSearchPathMetrics(check *CheckData) { + // this calculations are not accurate, as we are not including the directories sizes themselves + for _, checkPath := range l.paths { + checkPathNormalized := l.normalizePath(checkPath) + pathSize := uint64(0) + for _, data := range check.listData { + // 1. if we look at the paths, a file might be found under multiple search paths + // instead we save and check the path that led to this file being found + // 2. directory entries could have their "size" set. + // this can lead to including a file multiple times in the count. only add files to totalSize + if data["type"] == "file" && checkPathNormalized == data["check_path"] { + pathSize += convert.UInt64(data["size"]) + } + } + + pathMetricName := "size " + checkPath + check.result.Metrics = append(check.result.Metrics, + &CheckMetric{ + ThresholdName: pathMetricName, + Name: pathMetricName, + Value: pathSize, + Unit: "B", + Warning: check.warnThreshold, + Critical: check.critThreshold, + Min: &Zero, + }) + } +} + +// if specified, calculate the sizes of the subdirectories, that are not exactly search paths +// this function modifies the entries in the listData. It does not add metrics +// the sizes it calculcates are not accurate. it is just a sum of files under them. +// there are logical/physical sizes, disk block size, indexing, compression etc. to consider. +func (l *CheckFiles) addSubdirectorySizes(check *CheckData) { + for _, subDirData := range check.listData { + if subDirData["type"] != "dir" { + continue + } + if slices.Contains(l.paths, subDirData["fullname"]) { + continue + } + subDirSize := uint64(0) + for _, data := range check.listData { + if data["type"] != "file" { + continue + } + prefixToMatch := fmt.Sprintf("%s%c", subDirData["fullname"], os.PathSeparator) + rest, found := strings.CutPrefix(data["fullname"], prefixToMatch) + if found && rest != "" { + subDirSize += convert.UInt64(data["size"]) + } + } + + subDirData["size"] = fmt.Sprintf("%d", subDirSize) + } +} + +// if specified, calculate the sizes of the directories, that are not exactly search paths +// the entries should have a valid "size" attributes. populate them beforehand. +func (l *CheckFiles) addSubDirMetrics(check *CheckData) { + for _, data := range check.listData { + if data["type"] == "dir" { + subDirMetricName := data["fullname"] + " size" + check.result.Metrics = append(check.result.Metrics, + &CheckMetric{ + ThresholdName: subDirMetricName, + Name: subDirMetricName, + Value: data["size"], + Unit: "B", + Warning: check.warnThreshold, + Critical: check.critThreshold, + Min: &Zero, + }) + } + } +} + func (l *CheckFiles) addFileMetrics(check *CheckData) { needSize := check.HasThreshold("size") needAge := check.HasThreshold("age") diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index a8403756..d1d82c11 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -146,6 +146,8 @@ func (cd *CheckData) Finalize() (*CheckResult, error) { log.Debugf("condition warning: %s", cd.warnThreshold.String()) log.Debugf("condition critical: %s", cd.critThreshold.String()) log.Debugf("condition ok: %s", cd.okThreshold.String()) + // Run thresholds once on cd.details. This is done separately than metrics or entries + // This can possibly set a value to cd.details[_state] cd.Check(cd.details, cd.warnThreshold, cd.critThreshold, cd.okThreshold) log.Tracef("details: %#v", cd.details) @@ -188,8 +190,10 @@ func (cd *CheckData) finalizeOutput() (*CheckResult, error) { return nil, fmt.Errorf("%s", errMsg) } + // each entry in the list data is individually checked + // This may set "_state" of each entry cd.Check(entry, cd.warnThreshold, cd.critThreshold, cd.okThreshold) - log.Tracef(" - %#v", entry) + log.Tracef("Checking conditions for listData entry - %#v", entry) } } @@ -207,8 +211,10 @@ func (cd *CheckData) finalizeOutput() (*CheckResult, error) { cd.result.ApplyPerfSyntax(cd.perfSyntax, cd.timezone) + // Run a separate check on the macros cd.Check(finalMacros, cd.warnThreshold, cd.critThreshold, cd.okThreshold) cd.setStateFromMaps(finalMacros) + // Metrics are checked last, which also sets the final state cd.CheckMetrics(cd.warnThreshold, cd.critThreshold, cd.okThreshold) switch { @@ -389,7 +395,7 @@ func (cd *CheckData) buildCountMetrics(listLen, critLen, warnLen int) { } } -// setStateFromMaps sets main state from _state or list counts. +// setStateFromMaps sets main state from _state or list counts. main state is saved under cd.details["_state"] func (cd *CheckData) setStateFromMaps(macros map[string]string) { switch macros["_state"] { case "1": @@ -417,23 +423,27 @@ func (cd *CheckData) setStateFromMaps(macros map[string]string) { } // Check tries warn/crit/ok conditions against given data and sets result state. +// The data argument can be anything that has the correct keys that conditions use func (cd *CheckData) Check(data map[string]string, warnCond, critCond, okCond ConditionList) { data["_state"] = fmt.Sprintf("%d", CheckExitOK) for i := range warnCond { if res, ok := warnCond[i].Match(data); res && ok { + log.Debug("This data '%s' matched the WARNING CONDITION", warnCond[i].original) data["_state"] = fmt.Sprintf("%d", CheckExitWarning) } } for i := range critCond { if res, ok := critCond[i].Match(data); res && ok { + log.Debug("This data '%s' matched the CRITICAL Condition", critCond[i].original) data["_state"] = fmt.Sprintf("%d", CheckExitCritical) } } for i := range okCond { if res, ok := okCond[i].Match(data); res && ok { + log.Debug("This data '%s' matched the OK Condition", okCond[i].original) data["_state"] = fmt.Sprintf("%d", CheckExitOK) } } @@ -441,8 +451,10 @@ func (cd *CheckData) Check(data map[string]string, warnCond, critCond, okCond Co // CheckMetrics tries warn/crit/ok conditions against given metrics and sets final state accordingly func (cd *CheckData) CheckMetrics(warnCond, critCond, okCond ConditionList) { + // each metric is ran through conditions individually for _, metric := range cd.result.Metrics { state := CheckExitOK + // build up a data map[string]string as condition.Match function requires it as an argument data := map[string]string{ metric.Name: fmt.Sprintf("%v", metric.Value), } @@ -466,8 +478,9 @@ func (cd *CheckData) CheckMetrics(warnCond, critCond, okCond ConditionList) { state = CheckExitOK } } + if state > CheckExitOK { - log.Debugf("metric %s is %s", metric.Name, convert.StateString(state)) + log.Debugf("metric.Name: '%s', metric.ThresoldName: '%s', metric.Value: '%v', gave non-ok state: %s", metric.Name, metric.ThresholdName, metric.Value, convert.StateString(state)) cd.result.EscalateStatus(state) } }