From aac431a49ff497dc78406b7d7501af4942846af7 Mon Sep 17 00:00:00 2001 From: Adam Chalkley Date: Sat, 16 Nov 2019 22:24:06 -0600 Subject: [PATCH] Fix config struct processing, add (much needed) tests Added: - Tests (partial implementation/coverage) - validate the most common code-paths (much more TODO) - config handling - logging setup - run tests as part of GitHub Actions Workflow - recursively - after getting deps, but before linting steps in order to allow early failure - update Makefile to run tests recursively, verbosely - Fix configuration precedence handling - Add string slice equality check from StackOverflow Changed: - Fix configuration precedence handling - Config file loses to everything except default config settings - `config.logBuffer` moved to `logging.LogBuffer` - internal detail exposed for general use - may change this later - Updated code (where apparent) to be more test friendly - e.g., use `io.Reader` instead of filename so that we can use an in-memory TOML config file for testing config parsing and precedence validation - Split config package into smaller files in an effort to make related code easier to manage - Fail if requested config file not found - previously an error was logged, but execution was allowed to continue. - Move default values to Getter methods - use those methods instead of directly dereferencing config struct fields in the majority of the code - use Getter methods to guard against nil dereferences - Partial work to de-couple reliance on `Config{}` (see #170) Deprecated: Both of these functions from the `config` package do not appear to be needed any longer, but are being kept for a cycle in case I change my mind: - `Config.SetDefaultConfig()` - `Config.GetStructTag()` Fixed: - Fix configuration precedence handling - Config settings are merged properly, so that even default settings are allowed to override lower precedence config sources - NumFilesToKeep default (didn't match v0.5.1 change) - Add missing `default` switch statements along with error return codes for `Set` functions with specific valid option values - README updates - cover config file flag, environment variables - config precedence corrections - Multiple linting errors (with more that needs to be evaluated) - Add missing exit if IgnoreErrors is false - Handle unintended nil assignment - Update GoDoc doc file - reflect config file support, including updated Help text TODO: - Update multiple tests to use "tables" instead of hard-coding specific checks (which ends up being very lengthy) - e.g., `TestValidate()` - Various goconst linting errors refs #161, #156, #170, #176 --- .github/workflows/lint-and-build-code.yml | 13 +- .gitignore | 5 + Makefile | 8 +- NOTICE.txt | 40 ++ README.md | 87 +-- config/compare.go | 227 ++++++++ config/config.go | 626 +++++++++------------- config/config_test.go | 224 ++++++++ config/deprecated.go | 76 +++ config/get.go | 235 ++++++++ config/logger.go | 181 +++++++ config/merge.go | 172 ++++++ config/merge_complete_configs_test.go | 348 ++++++++++++ config/merge_incomplete_configs_test.go | 468 ++++++++++++++++ config/validate.go | 160 ++++++ config/validate_test.go | 608 +++++++++++++++++++++ doc.go | 34 +- logging/logging.go | 62 ++- logging/logging_test.go | 241 +++++++++ logging/logging_unix.go | 7 +- main.go | 102 ++-- main_test.go | 49 +- matches/matches.go | 40 +- paths/paths.go | 21 +- 24 files changed, 3493 insertions(+), 541 deletions(-) create mode 100644 config/compare.go create mode 100644 config/config_test.go create mode 100644 config/deprecated.go create mode 100644 config/get.go create mode 100644 config/logger.go create mode 100644 config/merge.go create mode 100644 config/merge_complete_configs_test.go create mode 100644 config/merge_incomplete_configs_test.go create mode 100644 config/validate.go create mode 100644 config/validate_test.go create mode 100644 logging/logging_test.go diff --git a/.github/workflows/lint-and-build-code.yml b/.github/workflows/lint-and-build-code.yml index a2219cd6..4926d792 100644 --- a/.github/workflows/lint-and-build-code.yml +++ b/.github/workflows/lint-and-build-code.yml @@ -51,6 +51,15 @@ jobs: - name: Check out code into the Go module directory uses: actions/checkout@v1 + - name: Get dependencies + run: | + go get -v -t -d ./... + + # Force tests to run early as it isn't worth doing much else if the + # tests fail to run properly. + - name: Run all tests + run: go test -v ./... + - name: Install Go linting tools run: | # add executables installed with go get to PATH @@ -59,10 +68,6 @@ jobs: export PATH=${PATH}:$(go env GOPATH)/bin make lintinstall - - name: Get dependencies - run: | - go get -v -t -d ./... - - name: Install Ubuntu packages if: contains(matrix.os, 'ubuntu') run: sudo apt update && sudo apt install -y --no-install-recommends make gcc diff --git a/.gitignore b/.gitignore index 9cd1e7da..0b26415a 100644 --- a/.gitignore +++ b/.gitignore @@ -39,3 +39,8 @@ elbow.upx # Ignore local copy of config file config.toml + + +# Ignore local "scatch" directory that may +# contain temporary files that should not be included in the repo +scratch/ diff --git a/Makefile b/Makefile index 4ebddf0b..7f10bf11 100644 --- a/Makefile +++ b/Makefile @@ -53,7 +53,7 @@ LINTINSTALLCMD = bash testing/install_linting_tools.sh # Targets will not work properly if a file with the same name is ever created # in this directory. We explicitly declare our targets to be phony by # making them a prerequisite of the special target .PHONY -.PHONY: help clean goclean gitclean pristine all windows linux testenv testrun linting lintinstall +.PHONY: help clean goclean gitclean pristine all windows linux testenv testrun linting lintinstall gotests # WARNING: Make expects you to use tabs to introduce recipe lines help: @@ -67,6 +67,7 @@ help: @echo " testrun use wrapper script to call binary with test settings" @echo " lintinstall use wrapper script to install common linting tools" @echo " linting use wrapper script to run common linting checks" + @echo " gotests go test recursively, verbosely" testenv: @echo "Setting up test environment in \"$(TESTENVDIR1)\" and \"$(TESTENVDIR2)\"" @@ -88,6 +89,11 @@ linting: @$(LINTINGCMD) @echo "Finished running linting checks" +gotests: + @echo "Running go tests ..." + @go test ./... + @echo "Finished running go tests" + goclean: @echo "Removing object files and cached files ..." @$(GOCLEANCMD) diff --git a/NOTICE.txt b/NOTICE.txt index 321bb2b8..13b6a914 100644 --- a/NOTICE.txt +++ b/NOTICE.txt @@ -42,6 +42,46 @@ SOFTWARE. + +Stephen Weinberg +https://stackoverflow.com/a/15312097 +https://stackoverflow.com/questions/15311969/checking-the-equality-of-two-slices + +YOU ARE FREE TO: + +Share - copy and redistribute the material in any medium or format +Adapt - remix, transform, and build upon the material for any purpose, even +commercially. + +This license is acceptable for Free Cultural Works. The licensor cannot revoke +these freedoms as long as you follow the license terms. + +UNDER THE FOLLOWING TERMS: + +Attribution - You must give appropriate credit, provide a link to the license, +and indicate if changes were made. You may do so in any reasonable manner, but +not in any way that suggests the licensor endorses you or your use. + +ShareAlike - If you remix, transform, or build upon the material, you must +distribute your contributions under the same license as the original. + +No additional restrictions - You may not apply legal terms or technological +measures that legally restrict others from doing anything the license permits. + +NOTICES: + +You do not have to comply with the license for elements of the material in the +public domain or where your use is permitted by an applicable exception or +limitation. + +No warranties are given. The license may not give you all of the permissions +necessary for your intended use. For example, other rights such as publicity, +privacy, or moral rights may limit how you use the material. + + + + + Stefan Nilsson https://yourbasic.org/golang/formatting-byte-size-to-human-readable-format/ https://creativecommons.org/licenses/by/3.0/ diff --git a/README.md b/README.md index 88c069df..2d295970 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ Elbow, Elbow grease. - [Multiple paths](#multiple-paths) - [JSON format](#json-format) - [Help Output](#help-output) - - [Prune `.war` files from each branch recursively, keep newest 2](#prune-war-files-from-each-branch-recursively-keep-newest-2) + - [Prune .war files from each branch recursively, keep newest 2](#prune-war-files-from-each-branch-recursively-keep-newest-2) - [Keep oldest 1, debug logging, ignore errors, use syslog](#keep-oldest-1-debug-logging-ignore-errors-use-syslog) - [Log to a file in JSON format](#log-to-a-file-in-json-format) - [References](#references) @@ -157,22 +157,27 @@ against these newly created test files. The priority order is (mostly): 1. Command line flags (highest priority) -1. Configuration file 1. Environment variables 1. Environment variables loaded from `.env` files (lowest priority) - **Not supported yet** +1. Configuration file +1. Default settings Configuration sources lower in the list are loaded first, with configuration -sources above loaded sequentially (if enabled) after. Any non-default values -specified for later sources (higher in the list) override default values -specified previously. This means that a non-default value specified in the -configuration file (which has to be intentionally loaded) can *survive* a -default value explicitly provided via a command-line option. +sources above loaded sequentially (if enabled) after. Settings are *merged*, +with settings specifically defined in sources with higher precedence +overriding values set by configuration sources with lower precedence. + +For example, if the configuration file defines `/tmp/elbow/path1` as the path +to process, an environment variable defines `/tmp/elbow/path2` and the +command-line flag for that setting specifies `/tmp/elbow/path3`, the +command-line flag will win and `/tmp/elbow/path3` will be used. The intent of this behavior is to provide a *feathered* layering of -configuration settings; multiple configuration sources can be used to provide -overrides for default values, but not to override non-default values set -previously by another configuration source. +configuration settings; if a configuration file provides all settings that you +want other than one, you can use the configuration file for the other settings +and specify the settings that you wish to override via environment variable or +command-line flag. **Note: This behavior is subject to change based on feedback.** @@ -180,28 +185,30 @@ previously by another configuration source. Aside from the built-in `-h`, short flag names are currently not supported. -| Long | Required | Default | Repeat | Possible | Description | -| ---------------- | -------- | -------------- | ------ | ------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | -| `keep` | No | `0` | No | `0+` | Keep specified number of matching files. | -| `paths` | Yes | N/A | No | *one or more valid directory paths* | List of comma or space-separated paths to process. | -| `pattern` | No | *empty string* | No | *valid file name characters* | Substring pattern to compare filenames against. Wildcards are not supported. | -| `extensions` | No | *empty list* | No | *valid file extensions* | Limit search to specified file extension. Specify as needed to match multiple required extensions. | -| `recurse` | No | `false` | No | `true`, `false` | Perform recursive search into subdirectories. | -| `keep-old` | No | `false` | No | `true`, `false` | Keep oldest files instead of newer. | -| `age` | No | `0` | No | `0+` | Limit search to files that are the specified number of days old or older. | -| `remove` | Maybe | `false` | No | `true`, `false` | Remove matched files. The default behavior is to only note what matching files *would* be removed. | -| `ignore-errors` | No | `false` | No | `true`, `false` | Ignore errors encountered during file removal. | -| `log-format` | No | `text` | No | `text`, `json` | Log formatter used by logging package. | -| `log-file` | No | *empty string* | No | *writable directory path* | Optional log file used to hold logged messages. If set, log messages are not displayed on the console. | -| `console-output` | No | `stdout` | No | `stdout`, `stderr` | Specify how log messages are logged to the console. | -| `log-level` | No | `info` | No | `emergency`, `alert`, `critical`, `panic`, `fatal`, `error`, `warn`, `info`, `notice`, `debug`, `trace` | Maximum log level at which messages will be logged. Log messages below this threshold will be discarded. | -| `use-syslog` | No | `false` | No | `true`, `false` | Log messages to syslog in addition to other ouputs. Not supported on Windows. | +| Long | Required | Default | Repeat | Possible | Description | +| ---------------- | -------- | -------------- | ------ | ------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | +| `keep` | No | `0` | No | `0+` | Keep specified number of matching files. | +| `paths` | Yes | N/A | No | *one or more valid directory paths* | List of comma or space-separated paths to process. | +| `pattern` | No | *empty string* | No | *valid file name characters* | Substring pattern to compare filenames against. Wildcards are not supported. | +| `extensions` | No | *empty list* | No | *valid file extensions* | Limit search to specified file extension. Specify as needed to match multiple required extensions. | +| `recurse` | No | `false` | No | `true`, `false` | Perform recursive search into subdirectories. | +| `keep-old` | No | `false` | No | `true`, `false` | Keep oldest files instead of newer. | +| `age` | No | `0` | No | `0+` | Limit search to files that are the specified number of days old or older. | +| `remove` | Maybe | `false` | No | `true`, `false` | Remove matched files. The default behavior is to only note what matching files *would* be removed. | +| `ignore-errors` | No | `false` | No | `true`, `false` | Ignore errors encountered during file removal. | +| `log-format` | No | `text` | No | `text`, `json` | Log formatter used by logging package. | +| `log-file` | No | *empty string* | No | *writable directory path* | Optional log file used to hold logged messages. If set, log messages are not displayed on the console. | +| `console-output` | No | `stdout` | No | `stdout`, `stderr` | Specify how log messages are logged to the console. | +| `log-level` | No | `info` | No | `emergency`, `alert`, `critical`, `panic`, `fatal`, `error`, `warn`, `info`, `notice`, `debug`, `trace` | Maximum log level at which messages will be logged. Log messages below this threshold will be discarded. | +| `use-syslog` | No | `false` | No | `true`, `false` | Log messages to syslog in addition to other ouputs. Not supported on Windows. | +| `config-file` | No | *empty string* | No | *valid path to config file* | Full path to optional TOML-formatted configuration file. See `config.example.toml` for a starter template. | ### Environment Variables -If set, command-line arguments override the equivalent environment variables -listed below. See the [Command-line Arguments](#command-line-arguments) table -for more information. +If set, environment variables override settings provided by a configuration +file. If used, command-line arguments override the equivalent environment +variables listed below. See the [Command-line +Arguments](#command-line-arguments) table for more information. | Flag Name | Environment Variable Name | Notes | Example | | ---------------- | ------------------------- | ---------------------------- | ----------------------------------------------------------------------------------- | @@ -219,13 +226,15 @@ for more information. | `console-output` | `ELBOW_CONSOLE_OUTPUT` | | `ELBOW_CONSOLE_OUTPUT="stdout"` | | `log-level` | `ELBOW_LOG_LEVEL` | | `ELBOW_LOG_LEVEL="debug"` | | `use-syslog` | `ELBOW_USE_SYSLOG` | | `ELBOW_USE_SYSLOG="true"` | +| `config-file` | `ELBOW_CONFIG_FILE` | | `ELBOW_CONFIG_FILE="/usr/local/elbow/config.toml"` | ### Configuration File -If set, configuration file settings override equivalent environment variables, -but "lose" to command-line flags. See the [Command-line -Arguments](#command-line-arguments) table for more information, including the -available values for the listed configuration settings. +Configuration file settings have the lowest priority and are overridden by +settings specified in other configuration sources, except for default values. +See the [Command-line Arguments](#command-line-arguments) table for more +information, including the available values for the listed configuration +settings. | Flag Name | Config file Setting Name | Section Name | Notes | | ---------------- | ------------------------ | -------------- | ------------------------------------------------------------------------ | @@ -257,14 +266,14 @@ within an Ubuntu Linux Subsystem for Windows (WSL) instance. The `t` volume is present on the Windows host. The file extension used in the examples below is for `WAR` files that are -generated on a build system that our group maintains. The idea is that `elbow` -could be run as a cron job to help ensure that only X copies (the most recent -in our case) for each of three branches remain on the build box. +generated on a build system that our group used to maintain. The idea is that +`elbow` could be run as a cron job to help ensure that only X copies (the most +recent in our case) for each of three branches remain on the build box. -There are better approaches to managing build artifacts (e.g., containers), but -that is the problem that this tool seeks to solve in a simple, "low tech" way. +There are better approaches to managing build artifacts (e.g., containers); +this tool seeks to solve in a simple, "low tech" way. -The particular repo that the build system processes has three branches: +The particular repo that the build system processed has three branches: | Branch Name | Type of build | | ----------- | ------------- | diff --git a/config/compare.go b/config/compare.go new file mode 100644 index 00000000..bf93e156 --- /dev/null +++ b/config/compare.go @@ -0,0 +1,227 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "testing" +) + +// https://stackoverflow.com/questions/15311969/checking-the-equality-of-two-slices +// https://stackoverflow.com/a/15312097 +// +// TODO: Not sure what file/package to place this. +func testStringSliceEqual(a []string, b []string) bool { + + // If one is nil, the other must also be nil. + if (a == nil) != (b == nil) { + return false + } + + if len(a) != len(b) { + return false + } + + for i := range a { + if a[i] != b[i] { + return false + } + } + + return true +} + +// CompareConfig receives two Config objects and compares exported field +// values to determine equality. +func CompareConfig(got Config, wanted Config, t *testing.T) { + + // FIXME: How can we get all field names programatically so we don't have to + // manually reference each field? + + t.Logf("got config (raw): %+v", got) + t.Logf("got config (string): %s", got.String()) + + t.Logf("wanted config (raw): %+v", wanted) + t.Logf("wanted config (string): %s", wanted.String()) + + // Validate both config structs first before attempting to do anything + // useful with them. + if err := got.Validate(); err != nil { + t.Error("Validation failed for got config", err) + } + + if err := wanted.Validate(); err != nil { + t.Error("Validation failed for wanted config", err) + } + + // Fail each field that fails to validate, but allow testing to continue + // until all fields have been evaluated + + if got.AppName != wanted.AppName { + t.Errorf("AppName: got (%v) does not equal wanted (%v)", + got.AppName, wanted.AppName) + } else { + t.Logf("AppName: got (%v) == wanted (%v)", + got.AppName, wanted.AppName) + } + + if got.AppDescription != wanted.AppDescription { + t.Errorf("AppDescription: got (%v) does not equal wanted (%v)", + got.AppDescription, wanted.AppDescription) + } else { + t.Logf("AppDescription: got (%v) == wanted (%v)", + got.AppDescription, wanted.AppDescription) + } + + if got.AppURL != wanted.AppURL { + t.Errorf("AppURL: got (%v) does not equal wanted (%v)", + got.AppURL, wanted.AppURL) + } else { + t.Logf("AppURL: got (%v) == wanted (%v)", + got.AppURL, wanted.AppURL) + } + + if got.AppVersion != wanted.AppVersion { + t.Errorf("AppVersion: got (%v) does not equal wanted (%v)", + got.AppVersion, wanted.AppVersion) + } else { + t.Logf("AppVersion: got (%v) == wanted (%v)", + got.AppVersion, wanted.AppVersion) + } + + if !testStringSliceEqual(got.Paths, wanted.Paths) { + t.Errorf("Paths: got (%q) does not equal wanted (%q)", + got.Paths, wanted.Paths) + } else { + t.Logf("Paths: got (%q) == wanted (%q)", + got.Paths, wanted.Paths) + } + + if !testStringSliceEqual(got.FileExtensions, wanted.FileExtensions) { + t.Errorf("FileExtensions: got (%q) does not equal wanted (%q)", + got.FileExtensions, wanted.FileExtensions) + } else { + t.Logf("FileExtensions: got (%q) == wanted (%q)", + got.FileExtensions, wanted.FileExtensions) + } + + if *got.FilePattern != *wanted.FilePattern { + t.Errorf("FilePattern: got (%v) does not equal wanted (%v)", + *got.FilePattern, *wanted.FilePattern) + } else { + t.Logf("FilePattern: got (%v) == wanted (%v)", + *got.FilePattern, *wanted.FilePattern) + } + + if *got.FileAge != *wanted.FileAge { + t.Errorf("FileAge: got (%v) does not equal wanted (%v)", + *got.FileAge, *wanted.FileAge) + } else { + t.Logf("FileAge: got (%v) == wanted (%v)", + *got.FileAge, *wanted.FileAge) + } + + if *got.NumFilesToKeep != *wanted.NumFilesToKeep { + t.Errorf("NumFilesToKeep: got (%v) does not equal wanted (%v)", + *got.NumFilesToKeep, *wanted.NumFilesToKeep) + } else { + t.Logf("NumFilesToKeep: got (%v) == wanted (%v)", + *got.NumFilesToKeep, *wanted.NumFilesToKeep) + } + + if *got.KeepOldest != *wanted.KeepOldest { + t.Errorf("KeepOldest: got (%v) does not equal wanted (%v)", + *got.KeepOldest, *wanted.KeepOldest) + } else { + t.Logf("KeepOldest: got (%v) == wanted (%v)", + *got.KeepOldest, *wanted.KeepOldest) + } + + if *got.Remove != *wanted.Remove { + t.Errorf("Remove: got (%v) does not equal wanted (%v)", + *got.Remove, *wanted.Remove) + } else { + t.Logf("Remove: got (%v) == wanted (%v)", + *got.Remove, *wanted.Remove) + } + + if *got.IgnoreErrors != *wanted.IgnoreErrors { + t.Errorf("IgnoreErrors: got (%v) does not equal wanted (%v)", + *got.IgnoreErrors, *wanted.IgnoreErrors) + } else { + t.Logf("IgnoreErrors: got (%v) == wanted (%v)", + *got.IgnoreErrors, *wanted.IgnoreErrors) + } + + if *got.RecursiveSearch != *wanted.RecursiveSearch { + t.Errorf("RecursiveSearch: got (%v) does not equal wanted (%v)", + *got.RecursiveSearch, *wanted.RecursiveSearch) + } else { + t.Logf("RecursiveSearch: got (%v) == wanted (%v)", + *got.RecursiveSearch, *wanted.RecursiveSearch) + } + + if *got.LogLevel != *wanted.LogLevel { + t.Errorf("LogLevel: got (%v) does not equal wanted (%v)", + *got.LogLevel, *wanted.LogLevel) + } else { + t.Logf("LogLevel: got (%v) == wanted (%v)", + *got.LogLevel, *wanted.LogLevel) + } + + if *got.LogFormat != *wanted.LogFormat { + t.Errorf("LogFormat: got (%v) does not equal wanted (%v)", + *got.LogFormat, *wanted.LogFormat) + } else { + t.Logf("LogFormat: got (%v) == wanted (%v)", + *got.LogFormat, *wanted.LogFormat) + } + + if *got.LogFilePath != *wanted.LogFilePath { + t.Errorf("LogFilePath: got (%v) does not equal wanted (%v)", + *got.LogFilePath, *wanted.LogFilePath) + } else { + t.Logf("LogFilePath: got (%v) == wanted (%v)", + *got.LogFilePath, *wanted.LogFilePath) + } + + if *got.ConsoleOutput != *wanted.ConsoleOutput { + t.Errorf("ConsoleOutput: got (%v) does not equal wanted (%v)", + *got.ConsoleOutput, *wanted.ConsoleOutput) + } else { + t.Logf("ConsoleOutput: got (%v) == wanted (%v)", + *got.ConsoleOutput, *wanted.ConsoleOutput) + } + + if *got.UseSyslog != *wanted.UseSyslog { + t.Errorf("UseSyslog: got (%v) does not equal wanted (%v)", + *got.UseSyslog, *wanted.UseSyslog) + } else { + t.Logf("UseSyslog: got (%v) == wanted (%v)", + *got.UseSyslog, *wanted.UseSyslog) + } + + if *got.ConfigFile != *wanted.ConfigFile { + t.Errorf("ConfigFile: got (%v) does not equal wanted (%v)", + *got.ConfigFile, *wanted.ConfigFile) + } else { + t.Logf("ConfigFile: got (%v) == wanted (%v)", + *got.ConfigFile, *wanted.ConfigFile) + } + + t.Log("CompareConfig complete") + +} diff --git a/config/config.go b/config/config.go index c026f247..6731025c 100644 --- a/config/config.go +++ b/config/config.go @@ -20,10 +20,9 @@ package config import ( "fmt" + "io" "io/ioutil" "os" - "reflect" - "runtime" "strings" "github.com/atc0005/elbow/logging" @@ -33,8 +32,6 @@ import ( "github.com/sirupsen/logrus" ) -var logBuffer logging.LogBuffer - // AppMetadata represents data about this application that may be used in Help // output, error messages and potentially log messages (e.g., AppVersion) type AppMetadata struct { @@ -47,30 +44,30 @@ type AppMetadata struct { // FileHandling represents options specific to how this application // handles files. type FileHandling struct { - FilePattern string `toml:"pattern" arg:"--pattern,env:ELBOW_FILE_PATTERN" help:"Substring pattern to compare filenames against. Wildcards are not supported."` + FilePattern *string `toml:"pattern" arg:"--pattern,env:ELBOW_FILE_PATTERN" help:"Substring pattern to compare filenames against. Wildcards are not supported."` FileExtensions []string `toml:"file_extensions" arg:"--extensions,env:ELBOW_EXTENSIONS" help:"Limit search to specified file extensions. Specify as space separated list to match multiple required extensions."` - FileAge int `toml:"file_age" arg:"--age,env:ELBOW_FILE_AGE" help:"Limit search to files that are the specified number of days old or older."` - NumFilesToKeep int `toml:"files_to_keep" arg:"--keep,env:ELBOW_KEEP" help:"Keep specified number of matching files per provided path."` - KeepOldest bool `toml:"keep_oldest" arg:"--keep-old,env:ELBOW_KEEP_OLD" help:"Keep oldest files instead of newer per provided path."` - Remove bool `toml:"remove" arg:"--remove,env:ELBOW_REMOVE" help:"Remove matched files per provided path."` - IgnoreErrors bool `toml:"ignore_errors" arg:"--ignore-errors,env:ELBOW_IGNORE_ERRORS" help:"Ignore errors encountered during file removal."` + FileAge *int `toml:"file_age" arg:"--age,env:ELBOW_FILE_AGE" help:"Limit search to files that are the specified number of days old or older."` + NumFilesToKeep *int `toml:"files_to_keep" arg:"--keep,env:ELBOW_KEEP" help:"Keep specified number of matching files per provided path."` + KeepOldest *bool `toml:"keep_oldest" arg:"--keep-old,env:ELBOW_KEEP_OLD" help:"Keep oldest files instead of newer per provided path."` + Remove *bool `toml:"remove" arg:"--remove,env:ELBOW_REMOVE" help:"Remove matched files per provided path."` + IgnoreErrors *bool `toml:"ignore_errors" arg:"--ignore-errors,env:ELBOW_IGNORE_ERRORS" help:"Ignore errors encountered during file removal."` } // Search represents options specific to controlling how this application // performs searches in the filesystem type Search struct { Paths []string `toml:"paths" arg:"--paths,env:ELBOW_PATHS" help:"List of comma or space-separated paths to process."` - RecursiveSearch bool `toml:"recursive_search" arg:"--recurse,env:ELBOW_RECURSE" help:"Perform recursive search into subdirectories per provided path."` + RecursiveSearch *bool `toml:"recursive_search" arg:"--recurse,env:ELBOW_RECURSE" help:"Perform recursive search into subdirectories per provided path."` } // Logging represents options specific to how this application handles // logging. type Logging struct { - LogLevel string `toml:"log_level" arg:"--log-level,env:ELBOW_LOG_LEVEL" help:"Maximum log level at which messages will be logged. Log messages below this threshold will be discarded."` - LogFormat string `toml:"log_format" arg:"--log-format,env:ELBOW_LOG_FORMAT" help:"Log formatter used by logging package."` - LogFilePath string `toml:"log_file_path" arg:"--log-file,env:ELBOW_LOG_FILE" help:"Optional log file used to hold logged messages. If set, log messages are not displayed on the console."` - ConsoleOutput string `toml:"console_output" arg:"--console-output,env:ELBOW_CONSOLE_OUTPUT" help:"Specify how log messages are logged to the console."` - UseSyslog bool `toml:"use_syslog" arg:"--use-syslog,env:ELBOW_USE_SYSLOG" help:"Log messages to syslog in addition to other outputs. Not supported on Windows."` + LogLevel *string `toml:"log_level" arg:"--log-level,env:ELBOW_LOG_LEVEL" help:"Maximum log level at which messages will be logged. Log messages below this threshold will be discarded."` + LogFormat *string `toml:"log_format" arg:"--log-format,env:ELBOW_LOG_FORMAT" help:"Log formatter used by logging package."` + LogFilePath *string `toml:"log_file_path" arg:"--log-file,env:ELBOW_LOG_FILE" help:"Optional log file used to hold logged messages. If set, log messages are not displayed on the console."` + ConsoleOutput *string `toml:"console_output" arg:"--console-output,env:ELBOW_CONSOLE_OUTPUT" help:"Specify how log messages are logged to the console."` + UseSyslog *bool `toml:"use_syslog" arg:"--use-syslog,env:ELBOW_USE_SYSLOG" help:"Log messages to syslog in addition to other outputs. Not supported on Windows."` } // Config represents a collection of configuration settings for this @@ -87,76 +84,122 @@ type Config struct { // Embedded to allow for easier carrying of "handles" between functions // TODO: Confirm that this is both needed and that it doesn't violate // best practices. - LogFileHandle *os.File `toml:"-" arg:"-"` - Logger *logrus.Logger `toml:"-" arg:"-"` - FlagParser *arg.Parser `toml:"-" arg:"-"` + // TODO: Should these be exposed or kept private? + logFileHandle *os.File `toml:"-" arg:"-"` + logger *logrus.Logger `toml:"-" arg:"-"` + flagParser *arg.Parser `toml:"-" arg:"-"` // Path to (optional) configuration file - ConfigFile string `toml:"config_file" arg:"--config-file,env:ELBOW_CONFIG_FILE" help:"Full path to optional TOML-formatted configuration file. See config.example.toml for a starter template."` + ConfigFile *string `toml:"config_file" arg:"--config-file,env:ELBOW_CONFIG_FILE" help:"Full path to optional TOML-formatted configuration file. See config.example.toml for a starter template."` } -// DefaultConfig returns a configuration object with baseline settings applied -// for further extension by the caller. -func DefaultConfig(appName, appDescription, appURL, appVersion string) Config { - - // Our baseline. The majority of the default settings were previously - // supplied via struct tags - - var defaultConfig Config - - // Common metadata - defaultConfig.AppName = appName - defaultConfig.AppDescription = appDescription - defaultConfig.AppURL = appURL - defaultConfig.AppVersion = appVersion - - // Apply default settings that other configuration sources will be allowed - // to (and for a few settings MUST) override - defaultConfig.FilePattern = "" - defaultConfig.FileAge = 0 - defaultConfig.NumFilesToKeep = -1 - defaultConfig.KeepOldest = false - defaultConfig.Remove = false - defaultConfig.IgnoreErrors = false - defaultConfig.RecursiveSearch = false - defaultConfig.LogLevel = "info" - defaultConfig.LogFormat = "text" - defaultConfig.LogFilePath = "" - defaultConfig.ConsoleOutput = "stdout" - defaultConfig.UseSyslog = false - defaultConfig.ConfigFile = "" +// NewDefaultConfig returns a newly constructed config object composed of +// default configuration settings. +func NewDefaultConfig(appVersion string) Config { + + // TODO: Is there a better way than creating a "throwaway" config object + // just to make use of its methods for retrieving default values? + c := Config{} + defaultAppName := c.GetAppName() + defaultAppDescription := c.GetAppDescription() + defaultAppURL := c.GetAppURL() + defaultFilePattern := c.GetFilePattern() + defaultFileAge := c.GetFileAge() + defaultNumFilesToKeep := c.GetNumFilesToKeep() + defaultKeepOldest := c.GetKeepOldest() + defaultRemove := c.GetRemove() + defaultIgnoreErrors := c.GetIgnoreErrors() + defaultRecursiveSearch := c.GetRecursiveSearch() + defaultLogLevel := c.GetLogLevel() + defaultLogFormat := c.GetLogFormat() + defaultLogFilePath := c.GetLogFilePath() + defaultConsoleOutput := c.GetConsoleOutput() + defaultUseSyslog := c.GetUseSyslog() + defaultConfigFile := c.GetConfigFile() + + defaultConfig := Config{ + AppMetadata: AppMetadata{ + AppName: defaultAppName, + AppDescription: defaultAppDescription, + AppURL: defaultAppURL, + AppVersion: appVersion, + }, + FileHandling: FileHandling{ + FilePattern: &defaultFilePattern, + //FileExtensions: &fileExtensions, + FileAge: &defaultFileAge, + NumFilesToKeep: &defaultNumFilesToKeep, + KeepOldest: &defaultKeepOldest, + Remove: &defaultRemove, + IgnoreErrors: &defaultIgnoreErrors, + }, + Logging: Logging{ + LogLevel: &defaultLogLevel, + LogFormat: &defaultLogFormat, + LogFilePath: &defaultLogFilePath, + ConsoleOutput: &defaultConsoleOutput, + UseSyslog: &defaultUseSyslog, + }, + Search: Search{ + //Paths: , + RecursiveSearch: &defaultRecursiveSearch, + }, + ConfigFile: &defaultConfigFile, + } return defaultConfig - } // NewConfig returns a pointer to a newly configured object representing a // collection of user-provided and default settings. -func NewConfig(appName, appDescription, appURL, appVersion string) *Config { +func NewConfig(appVersion string) (*Config, error) { - // Baseline collection of settings before loading custom config sources - defaultConfig := DefaultConfig(appName, appDescription, appURL, appVersion) + // fmt.Printf("os.Args quoted: %q\n", os.Args) + // fmt.Printf("os.Args bare: %v\n", os.Args) + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: "os.Args array contents", + Fields: logrus.Fields{ + "line": logging.GetLineNumber(), + "os_args": os.Args, + }, + }) - // The base configuration object that will be returned to the caller - baseConfig := DefaultConfig(appName, appDescription, appURL, appVersion) + // Apply default settings that other configuration sources will be allowed + // to (and for a few settings MUST) override + baseConfig := NewDefaultConfig(appVersion) - // Settings provided via config file - fileConfig := DefaultConfig(appName, appDescription, appURL, appVersion) + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Current baseConfig after NewDefaultConfig() call: %+v\n", baseConfig), + Fields: logrus.Fields{"line": logging.GetLineNumber()}, + }) + + // Settings provided via config file. Intentionally using uninitialized + // struct here so that we can check for nil pointers to indicate whether + // a field has been populated with configuration values. + fileConfig := Config{} // Settings provided via command-line flags and environment variables. // This object will always be set in some manner as either flags or env // vars will be needed to bootstrap the application. While we may support // using a configuration file to provide settings, it is not used by // default. - argsConfig := DefaultConfig(appName, appDescription, appURL, appVersion) + argsConfig := Config{} // Initialize logger "handle" for later use - baseConfig.Logger = logrus.New() + baseConfig.logger = logrus.New() // Bundle the returned `*.arg.Parser` for later use from `main()` so that // we can explicitly display usage or help details should the // user-provided settings fail validation. - baseConfig.FlagParser = arg.MustParse(&argsConfig) + baseConfig.flagParser = arg.MustParse(&argsConfig) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Current argsConfig after MustParse() call: %+v\n", argsConfig), + Fields: logrus.Fields{"line": logging.GetLineNumber()}, + }) /************************************************************************* At this point `baseConfig` is our baseline config object containing @@ -167,210 +210,165 @@ func NewConfig(appName, appDescription, appURL, appVersion string) *Config { *************************************************************************/ // If user specified a config file, let's try to use it - if argsConfig.ConfigFile != "" { + // TODO: Fail if not found, or continue using defaults in its place? + if argsConfig.ConfigFile != nil { // Check for a configuration file and load it if found. - if err := fileConfig.LoadConfigFile(argsConfig.ConfigFile); err != nil { - logBuffer.Add(logging.LogRecord{ + + // path not found + if _, err := os.Stat(*argsConfig.ConfigFile); os.IsNotExist(err) { + return nil, fmt.Errorf("requested config file not found: %v", err) + } + + fh, err := os.Open(*argsConfig.ConfigFile) + if err != nil { + return nil, fmt.Errorf("unable to open config file: %v", err) + } + defer fh.Close() + + if err := fileConfig.LoadConfigFile(fh); err != nil { + logging.Buffer.Add(logging.LogRecord{ Level: logrus.ErrorLevel, Message: fmt.Sprintf("Error loading config file: %s", err), Fields: logrus.Fields{"config_file": argsConfig.ConfigFile}, }) + + // Application failure codepath. Dump collected log messages and + // return control to the caller. + logging.Buffer.Flush(baseConfig.GetLogger()) + // TODO: Wrap errors and return so they can be unpacked in main() + return nil, fmt.Errorf("error loading configuration file: %s", err) } - logBuffer.Add(logging.LogRecord{ + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Current fileConfig after LoadConfigFile() call: %+v\n", fileConfig), + Fields: logrus.Fields{"line": logging.GetLineNumber()}, + }) + + logging.Buffer.Add(logging.LogRecord{ Level: logrus.DebugLevel, Message: "Processing fileConfig object with MergeConfig func", + Fields: logrus.Fields{"line": logging.GetLineNumber()}, }) - if err := MergeConfig(&baseConfig, fileConfig, defaultConfig); err != nil { - _, _, line, _ := runtime.Caller(0) - logBuffer.Add(logging.LogRecord{ + if err := MergeConfig(&baseConfig, fileConfig); err != nil { + logging.Buffer.Add(logging.LogRecord{ Level: logrus.ErrorLevel, Message: fmt.Sprintf("Error merging config file settings with base config: %s", err), - Fields: logrus.Fields{"line": line}, + Fields: logrus.Fields{ + "line": logging.GetLineNumber(), + "base_config": fmt.Sprintf("%+v", baseConfig), + "file_config": fmt.Sprintf("%+v", fileConfig), + }, }) } - if ok, err := baseConfig.Validate(); !ok { - _, _, line, _ := runtime.Caller(0) - logBuffer.Add(logging.LogRecord{ - Level: logrus.ErrorLevel, + // Don't fail the new configuration due to fileConfig not providing + // all required values; we are *feathering* values, not replacing all + // existing values in the config struct with ones from the next + // configuration source. + if err := baseConfig.Validate(); err != nil { + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, Message: fmt.Sprintf("Error validating config after merging %s: %s", "fileConfig", err), Fields: logrus.Fields{ - "line": line, - "config_object": fmt.Sprintf("%+v", baseConfig), + "line": logging.GetLineNumber(), + "base_config": fmt.Sprintf("%+v", baseConfig), + "file_config": fmt.Sprintf("%+v", fileConfig), }, }) - } + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: "Proceeding with evaluation of argsConfig", + Fields: logrus.Fields{ + "line": logging.GetLineNumber(), + }, + }) + + } } - logBuffer.Add(logging.LogRecord{ + logging.Buffer.Add(logging.LogRecord{ Level: logrus.DebugLevel, Message: "Processing argsConfig object with MergeConfig func", }) - if err := MergeConfig(&baseConfig, argsConfig, defaultConfig); err != nil { - _, _, line, _ := runtime.Caller(0) - logBuffer.Add(logging.LogRecord{ + if err := MergeConfig(&baseConfig, argsConfig); err != nil { + logging.Buffer.Add(logging.LogRecord{ Level: logrus.ErrorLevel, Message: fmt.Sprintf("Error merging args config settings with base config: %s", err), - Fields: logrus.Fields{"line": line}, + Fields: logrus.Fields{ + "line": logging.GetLineNumber(), + "base_config": fmt.Sprintf("%+v", baseConfig), + "args_config": fmt.Sprintf("%+v", argsConfig), + }, }) } - if ok, err := baseConfig.Validate(); !ok { - _, _, line, _ := runtime.Caller(0) - logBuffer.Add(logging.LogRecord{ - Level: logrus.ErrorLevel, - Message: fmt.Sprintf("Error validating config after merging %s: %s", "argsConfig", err), - Fields: logrus.Fields{"line": line}, - }) - } - - // Apply logging configuration - baseConfig.SetLoggerConfig() - - logBuffer.Add(logging.LogRecord{ - Level: logrus.DebugLevel, - Message: fmt.Sprintf("The config object that we are returning: %+v", baseConfig), - }) - - logBuffer.Add(logging.LogRecord{ - Level: logrus.DebugLevel, - Message: "Empty queued up log messages from log buffer using user-specified logging settings", - }) - logBuffer.Flush(baseConfig.Logger) - - return &baseConfig - -} + if err := baseConfig.Validate(); err != nil { -// GetStructTag returns the requested struct tag value, if set, and an error -// value indicating whether any problems were encountered. -func GetStructTag(c Config, fieldname string, tagName string) (string, bool) { + // ################################################################### + // This code should only be reached if we were unable to properly + // apply the configuration. At this point we cannot trust that our + // settings are valid. We should ensure default settings are applied + // to our logger instance, flush all held messages and then exit + // immediately. + // ################################################################### - t := reflect.TypeOf(c) + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Error validating config after merging %s: %s", "argsConfig", err), + Fields: logrus.Fields{ + "line": logging.GetLineNumber(), + "base_config": fmt.Sprintf("%+v", baseConfig), + "args_config": fmt.Sprintf("%+v", argsConfig), + }, + }) - var field reflect.StructField - var ok bool - var tagValue string + // Application failure codepath. Dump collected log messages and + // return control to the caller. + logging.Buffer.Flush(baseConfig.GetLogger()) + // TODO: Wrap errors and return so they can be unpacked in main() + return nil, fmt.Errorf("configuration validation failed after merging argsConfig: %s", err) - if field, ok = t.FieldByName(fieldname); !ok { - return "", false } - if tagValue, ok = field.Tag.Lookup(tagName); !ok { - return "", false + // Apply logging configuration. If error is encountered, pass it back to + // caller to deal with. + if err := baseConfig.SetLoggerConfig(); err != nil { + return nil, err } - return tagValue, true - -} - -// MergeConfig receives source, destination and default Config objects and -// merges select, non-default field values from the source Config object to -// the destination config object, overwriting any field value already present. -// -// `source` and `destination` config structs already have usable default values -// upon creation using our `NewConfig()` constructor; only copy if source struct -// has a different value -// -// TODO: While this makes sense NOW, what is the best way to handle this if -// the default value becomes non-zero? -// -// The goal is to respect the current documented configuration precedence for -// multiple configuration sources (e.g., config file and command-line flags). -func MergeConfig(destination *Config, source Config, defaultConfig Config) error { - - // FIXME: How can we get all field names programatically so we don't have to - // manually reference each field? - - logBuffer.Add(logging.LogRecord{ - Level: logrus.DebugLevel, - Message: "MergeConfig called", - }) - logBuffer.Add(logging.LogRecord{ + logging.Buffer.Add(logging.LogRecord{ Level: logrus.DebugLevel, - Message: fmt.Sprintf("Source struct: %+v", source), + Message: fmt.Sprintf("The config object that we are returning (raw format): %+v", baseConfig), + Fields: logrus.Fields{"line": logging.GetLineNumber()}, }) - logBuffer.Add(logging.LogRecord{ + + logging.Buffer.Add(logging.LogRecord{ Level: logrus.DebugLevel, - Message: fmt.Sprintf("Destination struct: %+v", *destination), + Message: fmt.Sprint("The config object that we are returning (string format): ", baseConfig.String()), + Fields: logrus.Fields{"line": logging.GetLineNumber()}, }) - logBuffer.Add(logging.LogRecord{ + + logging.Buffer.Add(logging.LogRecord{ Level: logrus.DebugLevel, - Message: fmt.Sprintf("Default struct: %+v", defaultConfig), + Message: "Empty queued up log messages from log buffer using user-specified logging settings", + Fields: logrus.Fields{"line": logging.GetLineNumber()}, }) - if len(source.Paths) > len(defaultConfig.Paths) { - destination.Paths = source.Paths - } + logging.Buffer.Flush(baseConfig.GetLogger()) - if len(source.FileExtensions) > len(defaultConfig.FileExtensions) { - destination.FileExtensions = source.FileExtensions - } - - if source.FilePattern != defaultConfig.FilePattern { - destination.FilePattern = source.FilePattern - } - - if source.FileAge > defaultConfig.FileAge { - destination.FileAge = source.FileAge - } - - if source.NumFilesToKeep > defaultConfig.NumFilesToKeep { - destination.NumFilesToKeep = source.NumFilesToKeep - } - - // TODO: any reason to check this? Perhaps just direct copy for boolean - // variables? - if source.KeepOldest != defaultConfig.KeepOldest { - destination.KeepOldest = source.KeepOldest - } - - if source.Remove != defaultConfig.Remove { - destination.Remove = source.Remove - } - - if source.IgnoreErrors != defaultConfig.IgnoreErrors { - destination.IgnoreErrors = source.IgnoreErrors - } - - if source.RecursiveSearch != defaultConfig.RecursiveSearch { - destination.RecursiveSearch = source.RecursiveSearch - } - - if source.LogLevel != defaultConfig.LogLevel { - destination.LogLevel = source.LogLevel - } + return &baseConfig, nil - if source.LogFormat != defaultConfig.LogFormat { - destination.LogFormat = source.LogFormat - } - - if source.LogFilePath != defaultConfig.LogFilePath { - destination.LogFilePath = source.LogFilePath - } - - if source.ConsoleOutput != defaultConfig.ConsoleOutput { - destination.ConsoleOutput = source.ConsoleOutput - } - - if source.UseSyslog != defaultConfig.UseSyslog { - destination.UseSyslog = source.UseSyslog - } - - // FIXME: Placeholder - // FIXME: What useful error code would we return from this function? - return nil } -// LoadConfigFile reads and unmarshals a configuration file in TOML format -func (c *Config) LoadConfigFile(filename string) error { +// LoadConfigFile reads from an io.Reader and unmarshals a configuration file +// in TOML format into the associated Config struct. +func (c *Config) LoadConfigFile(fileHandle io.Reader) error { - // Read file to byte slice - configFile, err := ioutil.ReadFile(filename) + configFile, err := ioutil.ReadAll(fileHandle) if err != nil { return err } @@ -385,7 +383,7 @@ func (c *Config) LoadConfigFile(filename string) error { // Description provides an overview as part of the application Help output func (c Config) Description() string { - return fmt.Sprintf("%s %s", c.AppName, c.AppDescription) + return fmt.Sprintf("%s %s", c.GetAppName(), c.GetAppDescription()) } // Version provides a version string that appears at the top of the @@ -393,7 +391,7 @@ func (c Config) Description() string { func (c Config) Version() string { versionString := fmt.Sprintf("%s %s\n%s", - strings.ToTitle(c.AppName), c.AppVersion, c.AppURL) + strings.ToTitle(c.GetAppName()), c.GetAppVersion(), c.GetAppURL()) //divider := strings.Repeat("-", len(versionString)) @@ -405,164 +403,62 @@ func (c Config) Version() string { return "\n" + versionString + "\n" } -// Validate verifies all struct fields have been provided acceptable -func (c Config) Validate() (bool, error) { - - // FilePattern is optional - // FileExtensions is optional - // Discovered files are checked against FileExtensions later - - if len(c.Paths) == 0 { - return false, fmt.Errorf("one or more paths not provided") - } - - // RecursiveSearch is optional - - // NumFilesToKeep is optional, but if specified we should make sure it is - // a non-negative number. AFAIK, this is not currently enforced any other - // way. - if c.NumFilesToKeep < 0 { - return false, fmt.Errorf("invalid value provided for files to keep") - } +// String() satisfies the Stringer{} interface. This is intended for non-JSON +// formatting if using the TextFormatter logrus formatter. +func (c *Config) String() string { - // We only want to work with positive file modification times 0 is - // acceptable as it is the default value and indicates that the user has - // not chosen to use the flag (or has chosen improperly and it will be - // ignored). - if c.FileAge < 0 { - return false, fmt.Errorf("negative number for file age not supported") + // If the AppMetadata fields are empty, note that to aid in + // troubleshooting elsewhere in the codebase + if c.AppName == "" { + c.AppName = "NotSet" } - - // KeepOldest is optional - // Remove is optional - // IgnoreErrors is optional - - switch c.LogFormat { - case "text": - case "json": - default: - return false, fmt.Errorf("invalid option %q provided for log format", c.LogFormat) + if c.AppDescription == "" { + c.AppDescription = "NotSet" } - - // LogFilePath is optional - // TODO: String validation if it is set? - - // Do nothing for valid choices, return false if invalid value specified - switch c.ConsoleOutput { - case "stdout": - case "stderr": - default: - return false, fmt.Errorf("invalid option %q provided for console output destination", c.ConsoleOutput) + if c.AppVersion == "" { + c.AppVersion = "NotSet" } - - switch c.LogLevel { - case "emergency": - case "alert": - case "critical": - case "panic": - case "fatal": - case "error": - case "warn": - case "info": - case "notice": - case "debug": - case "trace": - default: - return false, fmt.Errorf("invalid option %q provided for log level", c.LogLevel) + if c.AppURL == "" { + c.AppURL = "NotSet" } - // UseSyslog is optional - - // Optimist - return true, nil - -} - -// String() satisfies the Stringer{} interface. This is intended for non-JSON -// formatting if using the TextFormatter logrus formatter. -func (c *Config) String() string { - return fmt.Sprintf("AppName=%q, AppDescription=%q, AppVersion=%q, AppURL=%q, FilePattern=%q, FileExtensions=%q, Paths=%v, RecursiveSearch=%t, FileAge=%d, NumFilesToKeep=%d, KeepOldest=%t, Remove=%t, IgnoreErrors=%t, LogFormat=%q, LogFilePath=%q, LogFileHandle=%v, ConsoleOutput=%q, LogLevel=%q, UseSyslog=%t, Logger=%v, FlagParser=%v), ConfigFile=%q, EndOfStringMethod", - - c.AppName, - c.AppDescription, - c.AppVersion, - c.AppURL, - c.FilePattern, - c.FileExtensions, - c.Paths, - c.RecursiveSearch, - c.FileAge, - c.NumFilesToKeep, - c.KeepOldest, - c.Remove, - c.IgnoreErrors, - c.LogFormat, - c.LogFilePath, - c.LogFileHandle, - c.ConsoleOutput, - c.LogLevel, - c.UseSyslog, - c.Logger, - c.FlagParser, - c.ConfigFile, + return fmt.Sprintf("AppName=%q, AppDescription=%q, AppVersion=%q, AppURL=%q, FilePattern=%q, FileExtensions=%q, Paths=%v, RecursiveSearch=%t, FileAge=%d, NumFilesToKeep=%d, KeepOldest=%t, Remove=%t, IgnoreErrors=%t, LogFormat=%q, LogFilePath=%q, ConfigFile=%q, ConsoleOutput=%q, LogLevel=%q, UseSyslog=%t, logger=%v, flagParser=%v, logFileHandle=%v", + + c.GetAppName(), + c.GetAppDescription(), + c.GetAppVersion(), + c.GetAppURL(), + c.GetFilePattern(), + c.GetFileExtensions(), + c.GetPaths(), + c.GetRecursiveSearch(), + c.GetFileAge(), + c.GetNumFilesToKeep(), + c.GetKeepOldest(), + c.GetRemove(), + c.GetIgnoreErrors(), + c.GetLogFormat(), + c.GetLogFilePath(), + c.GetConfigFile(), + c.GetConsoleOutput(), + c.GetLogLevel(), + c.GetUseSyslog(), + c.GetLogger(), + c.GetFlagParser(), + c.GetLogFileHandle(), ) } -// SetLoggerConfig applies chosen configuration settings that control logging -// output. -func (c *Config) SetLoggerConfig() { - - logging.SetLoggerFormatter(c.Logger, c.LogFormat) - logging.SetLoggerConsoleOutput(c.Logger, c.ConsoleOutput) - - if fileHandle, err := logging.SetLoggerLogFile(c.Logger, c.LogFilePath); err == nil { - c.LogFileHandle = fileHandle - } else { - // Need to collect the error for display later - logBuffer.Add(logging.LogRecord{ - Level: logrus.ErrorLevel, - Message: fmt.Sprintf("%s", err), - Fields: logrus.Fields{"log_file": c.LogFilePath}, - }) - } - - logging.SetLoggerLevel(c.Logger, c.LogLevel) - - // https://godoc.org/github.com/sirupsen/logrus#New - // https://godoc.org/github.com/sirupsen/logrus#Logger - - // make sure that the user actually requested syslog logging as it is - // currently supported on UNIX only. - if c.UseSyslog { - logBuffer.Add(logging.LogRecord{ - Level: logrus.InfoLevel, - Message: "Syslog logging requested, attempting to enable it", - Fields: logrus.Fields{"use_syslog": c.UseSyslog}, - }) - - if err := logging.EnableSyslogLogging(c.Logger, &logBuffer, c.LogLevel); err != nil { - // TODO: Is this sufficient cause for failing? Perhaps if a local - // log file is not also set consider it a failure? - - logBuffer.Add(logging.LogRecord{ - Level: logrus.ErrorLevel, - Message: fmt.Sprintf("Failed to enable syslog logging: %s", err), - Fields: logrus.Fields{"use_syslog": c.UseSyslog}, - }) - - logBuffer.Add(logging.LogRecord{ - Level: logrus.WarnLevel, - Message: "Proceeding without syslog logging", - Fields: logrus.Fields{"use_syslog": c.UseSyslog}, - }) - } - } else { - logBuffer.Add(logging.LogRecord{ - Level: logrus.DebugLevel, - Message: "Syslog logging not requested, not enabling", - Fields: logrus.Fields{"use_syslog": c.UseSyslog}, - }) - +// WriteDefaultHelpText is a helper function used to output Help text for +// situations where the Config object cannot be trusted to be in a usable +// state. +// TODO: Reconsider this; this feels fragile. +func WriteDefaultHelpText(appName string) { + config := arg.Config{Program: appName} + defaultConfig := Config{} + parser, err := arg.NewParser(config, &defaultConfig) + if err != nil { + panic("failed to build go-arg parser for Help text generation") } - + parser.WriteUsage(os.Stdout) } diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 00000000..d8053afb --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,224 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "bytes" + "os" + "path" + "runtime" + "testing" + + "github.com/atc0005/elbow/units" + "github.com/sirupsen/logrus" +) + +func GetBaseProjectDir(t *testing.T) string { + + // https://stackoverflow.com/questions/23847003/golang-tests-and-working-directory + _, filename, _, _ := runtime.Caller(1) + // The ".." reflects the path above the current working directory + dir := path.Join(path.Dir(filename), "..") + return dir + +} + +// TODO: Lots of variations here +func TestNewConfigFlagsOnly(t *testing.T) { + + // https://stackoverflow.com/questions/33723300/how-to-test-the-passing-of-arguments-in-golang + + // Save old command-line arguments so that we can restore them later + oldArgs := os.Args + + // Defer restoring original command-line arguments + defer func() { os.Args = oldArgs }() + + // TODO: A useful way to automate retrieving the app name? + appName := "elbow" + if runtime.GOOS == "windows" { + appName += ".exe" + } + + // Note to self: Don't add/escape double-quotes here. The shell strips + // them away and the application never sees them. + os.Args = []string{ + appName, + "--paths", "/tmp/elbow/path1", + "--keep", "1", + "--recurse", + "--keep-old", + "--log-level", "info", + "--use-syslog", + "--log-format", "text", + "--console-output", "stdout", + } + + // TODO: Flesh this out + _, err := NewConfig("x.y.z") + if err != nil { + t.Errorf("Error encountered when instantiating configuration: %s", err) + } else { + t.Log("No errors encountered when instantiating configuration") + } + +} + +func TestLoadConfigFileBaseline(t *testing.T) { + + // TODO: This currently mirrors the example config file. Replace with a read + // against that file? + var defaultConfigFile = []byte(` + [filehandling] + + pattern = "reach-masterdev-" + file_extensions = [ + ".war", + ".tmp", + ] + + file_age = 1 + files_to_keep = 2 + keep_oldest = false + remove = false + ignore_errors = true + + [search] + + paths = [ + "/tmp/elbow/path1", + "/tmp/elbow/path2", + ] + + recursive_search = true + + + [logging] + + log_level = "debug" + log_format = "text" + + # If set, all output to the console will be muted and sent here instead + log_file_path = "/tmp/log.json" + + console_output = "stdout" + use_syslog = false`) + + // Construct a mostly empty config struct to load our config settings into. + // We only define values for settings that we have no intention of using + // from the config file, such as a logger object and an empty path to + // the log file that we already know the path to. + defaultConfigFilePath := "" + c := Config{ + ConfigFile: &defaultConfigFilePath, + logger: logrus.New(), + } + + // Use our default in-memory config file settings + r := bytes.NewReader(defaultConfigFile) + + if err := c.LoadConfigFile(r); err != nil { + t.Error("Unable to load in-memory configuration:", err) + } else { + t.Log("Loaded in-memory configuration file") + } + + t.Log("LoadConfigFile wiped the existing struct, reconstructing AppMetadata fields to pass validation checks") + c.AppName = c.GetAppName() + c.AppVersion = c.GetAppVersion() + c.AppURL = c.GetAppURL() + c.AppDescription = c.GetAppDescription() + + t.Logf("Current config settings: %s", c.String()) + + if err := c.Validate(); err != nil { + t.Error("Unable to validate configuration:", err) + } else { + t.Log("Validation successful") + } + +} + +// This function is intended to test the example config file included in the +// repo. That example config file is a template of valid settings, so we +// should run tests against it to verify everything checks out. +func TestLoadConfigFileTemplate(t *testing.T) { + + // Construct a mostly empty config struct to load our config settings into. + // We only define values for settings that we have no intention of using + // from the config file, such as a logger object and an empty path to + // the log file that we already know the path to. + defaultConfigFilePath := "" + c := Config{ + ConfigFile: &defaultConfigFilePath, + logger: logrus.New(), + } + + cwd, err := os.Getwd() + if err != nil { + t.Fatalf(err.Error()) + } + t.Log("Current working directory:", cwd) + + // this file is located in the base of the repo + exampleConfigFile := "config.example.toml" + + // Get path above cwd in order to load config file (from base path of repo) + baseDir := GetBaseProjectDir(t) + if err := os.Chdir(baseDir); err != nil { + t.Error(err) + t.FailNow() + } else { + cwd, err := os.Getwd() + if err != nil { + t.Fatalf(err.Error()) + } + t.Log("New working directory:", cwd) + } + + if fileDetails, err := os.Stat(exampleConfigFile); os.IsNotExist(err) { + t.Errorf("requested config file not found: %v", err) + } else { + t.Log("config file found") + t.Log("name:", fileDetails.Name()) + t.Log("size:", units.ByteCountSI(fileDetails.Size())) + t.Log("date/time stamp:", fileDetails.ModTime()) + + fh, err := os.Open(exampleConfigFile) + if err != nil { + t.Errorf("Unable to open config file: %v", err) + } else { + t.Log("Successfully opened config file", exampleConfigFile) + } + defer fh.Close() + + if err := c.LoadConfigFile(fh); err != nil { + t.Error("Unable to load configuration file:", err) + } else { + t.Log("Loaded configuration file") + } + + t.Logf("Current config settings: %s", c.String()) + + if err := c.Validate(); err != nil { + t.Error("Unable to validate configuration:", err) + } else { + t.Log("Validation successful") + } + + } +} diff --git a/config/deprecated.go b/config/deprecated.go new file mode 100644 index 00000000..77813ead --- /dev/null +++ b/config/deprecated.go @@ -0,0 +1,76 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// NOTE: This file is a "box on the shelf" for potential later use, but should +// be considered earmarked for retirement. Content in this file is subject to +// removal at any time. + +package config + +import ( + "reflect" +) + +// GetStructTag returns the requested struct tag value, if set, and an error +// value indicating whether any problems were encountered. +func GetStructTag(c Config, fieldname string, tagName string) (string, bool) { + + t := reflect.TypeOf(c) + + var field reflect.StructField + var ok bool + var tagValue string + + if field, ok = t.FieldByName(fieldname); !ok { + return "", false + } + + if tagValue, ok = field.Tag.Lookup(tagName); !ok { + return "", false + } + + return tagValue, true + +} + +// SetDefaultConfig applies application default values to Config object fields +// TODO: Is this still needed? NewDefaultConfig() is handling this now? +func (c *Config) SetDefaultConfig() { + + // These fields are intentionally ignored + // FileExtensions + // Paths + + // TODO: Create default logger object? + + c.AppName = c.GetAppName() + c.AppDescription = c.GetAppDescription() + c.AppURL = c.GetAppURL() + c.AppVersion = c.GetAppVersion() + *c.FilePattern = c.GetFilePattern() + *c.FileAge = c.GetFileAge() + *c.NumFilesToKeep = c.GetNumFilesToKeep() + *c.KeepOldest = c.GetKeepOldest() + *c.Remove = c.GetRemove() + *c.IgnoreErrors = c.GetIgnoreErrors() + *c.RecursiveSearch = c.GetRecursiveSearch() + *c.LogLevel = c.GetLogLevel() + *c.LogFormat = c.GetLogFormat() + *c.LogFilePath = c.GetLogFilePath() + *c.ConsoleOutput = c.GetConsoleOutput() + *c.UseSyslog = c.GetUseSyslog() + *c.ConfigFile = c.GetConfigFile() +} diff --git a/config/get.go b/config/get.go new file mode 100644 index 00000000..b580987f --- /dev/null +++ b/config/get.go @@ -0,0 +1,235 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "os" + + "github.com/alexflint/go-arg" + "github.com/sirupsen/logrus" +) + +// GetAppName returns the appName field if it's non-nil, app default value +// otherwise +func (c *Config) GetAppName() string { + if c == nil || c.AppName == "" { + return "Elbow" + } + return c.AppName +} + +// GetAppDescription returns the appDescription field if it's non-nil, app +// default value otherwise +func (c *Config) GetAppDescription() string { + if c == nil || c.AppDescription == "" { + return "prunes content matching specific patterns, either in a single directory or recursively through a directory tree." + } + return c.AppDescription + +} + +// GetAppVersion returns the appVersion field if it's non-nil, app default +// value otherwise +func (c *Config) GetAppVersion() string { + if c == nil || c.AppVersion == "" { + return "dev" + } + return c.AppVersion +} + +// GetAppURL returns the appURL field if it's non-nil, app default value +// otherwise +func (c *Config) GetAppURL() string { + if c == nil || c.AppURL == "" { + return "https://github.com/atc0005/elbow" + } + return c.AppURL +} + +// GetFilePattern returns the filePattern field if it's non-nil, app default +// value otherwise +func (c *Config) GetFilePattern() string { + if c == nil || c.FilePattern == nil { + return "" + } + return *c.FilePattern +} + +// GetFileExtensions returns the fileExtensions field if it's non-nil, zero value +// otherwise. +// TODO: Double check this one; how should we safely handle returning an +// empty/zero value? +// As an example, the https://github.com/google/go-github package has a +// `Issue.GetAssignees()` method that returns nil if the `Issue.Assignees` +// field is nil. This seems to suggest that this is all we really can do here? +// +func (c *Config) GetFileExtensions() []string { + if c == nil || c.FileExtensions == nil { + // FIXME: Isn't the goal to avoid returning nil? + return nil + } + return c.FileExtensions +} + +// GetFileAge returns the fileAge field if it's non-nil, app default value +// otherwise +func (c *Config) GetFileAge() int { + if c == nil || c.FileAge == nil { + return 0 + } + return *c.FileAge +} + +// GetNumFilesToKeep returns the numFilesToKeep field if it's non-nil, zero +// value otherwise. +func (c *Config) GetNumFilesToKeep() int { + if c == nil || c.NumFilesToKeep == nil { + return 0 + } + return *c.NumFilesToKeep +} + +// GetKeepOldest returns the keepOldest field if it's non-nil, zero value +// otherwise. +func (c *Config) GetKeepOldest() bool { + if c == nil || c.KeepOldest == nil { + return false + } + return *c.KeepOldest +} + +// GetRemove returns the remove field if it's non-nil, app default value +// otherwise +func (c *Config) GetRemove() bool { + if c == nil || c.Remove == nil { + return false + } + return *c.Remove +} + +// GetIgnoreErrors returns the ignoreErrors field if it's non-nil, zero value +// otherwise. +func (c *Config) GetIgnoreErrors() bool { + if c == nil || c.IgnoreErrors == nil { + return false + } + return *c.IgnoreErrors +} + +// GetPaths returns the paths field if it's non-nil, app default value +// otherwise +func (c *Config) GetPaths() []string { + if c == nil || c.Paths == nil { + return nil + } + return c.Paths +} + +// GetRecursiveSearch returns the recursiveSearch field if it's non-nil, zero +// value otherwise. +func (c *Config) GetRecursiveSearch() bool { + if c == nil || c.RecursiveSearch == nil { + return false + } + return *c.RecursiveSearch +} + +// GetLogLevel returns the logLevel field if it's non-nil, app default value +// otherwise +func (c *Config) GetLogLevel() string { + if c == nil || c.LogLevel == nil { + return "info" + } + return *c.LogLevel +} + +// GetLogFormat returns the logFormat field if it's non-nil, app default value +// otherwise +func (c *Config) GetLogFormat() string { + if c == nil || c.LogFormat == nil { + return "text" + } + return *c.LogFormat +} + +// GetLogFilePath returns the logFilePath field if it's non-nil, zero value +// otherwise. +func (c *Config) GetLogFilePath() string { + if c == nil || c.LogFilePath == nil { + return "" + } + return *c.LogFilePath +} + +// GetConsoleOutput returns the consoleOutput field if it's non-nil, zero +// value otherwise. +func (c *Config) GetConsoleOutput() string { + if c == nil || c.ConsoleOutput == nil { + return "stdout" + } + return *c.ConsoleOutput +} + +// GetUseSyslog returns the useSyslog field if it's non-nil, zero +// value otherwise. +func (c *Config) GetUseSyslog() bool { + if c == nil || c.UseSyslog == nil { + return false + } + return *c.UseSyslog +} + +// GetConfigFile returns the configFile field if it's non-nil, zero value +// otherwise. +func (c *Config) GetConfigFile() string { + if c == nil || c.ConfigFile == nil { + return "" + } + return *c.ConfigFile +} + +// GetLogger returns the logger field if it's non-nil, app default value +// otherwise +func (c *Config) GetLogger() *logrus.Logger { + if c == nil || c.logger == nil { + //return nil + + // FIXME: Is this the best logic? + c.logger = logrus.New() + //c.logger.Out = os.Stderr + + } + return c.logger +} + +// GetFlagParser returns the flagParser field if it's non-nil, app default +// value otherwise +func (c *Config) GetFlagParser() *arg.Parser { + if c == nil || c.flagParser == nil { + return nil + } + return c.flagParser +} + +// GetLogFileHandle returns the logFileHandle field if it's non-nil, app +// default value otherwise +func (c *Config) GetLogFileHandle() *os.File { + if c == nil || c.logFileHandle == nil { + return nil + } + return c.logFileHandle +} diff --git a/config/logger.go b/config/logger.go new file mode 100644 index 00000000..3bbae112 --- /dev/null +++ b/config/logger.go @@ -0,0 +1,181 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "fmt" + + "github.com/atc0005/elbow/logging" + "github.com/sirupsen/logrus" +) + +// SetLoggerConfig applies chosen configuration settings that control logging +// output. +func (c *Config) SetLoggerConfig() error { + + var err error + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: "Calling SetLoggerConfig()", + Fields: logrus.Fields{"line": logging.GetLineNumber()}, + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Current state of config object: %+v\n", c), + Fields: logrus.Fields{"line": logging.GetLineNumber()}, + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("The address of the logger SetLoggerConfig received: %p\n", c.GetLogger()), + Fields: logrus.Fields{"line": logging.GetLineNumber()}, + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: "Current state of individual logging related fields", + Fields: logrus.Fields{ + "line": logging.GetLineNumber(), + "log_format": c.GetLogFormat(), + "console_output": c.GetConsoleOutput(), + "use_syslog": c.GetUseSyslog(), + }, + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("logger.Out field at start of SetLoggerFormatter(): %p\n", c.GetLogger().Out), + Fields: logrus.Fields{ + "line": logging.GetLineNumber(), + }, + }) + + if err = logging.SetLoggerFormatter(c.logger, c.GetLogFormat()); err != nil { + return err + } + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("logger.Out field after SetLoggerFormatter: %p\n", c.GetLogger().Out), + Fields: logrus.Fields{ + "line": logging.GetLineNumber(), + }, + }) + + if err = logging.SetLoggerConsoleOutput(c.logger, c.GetConsoleOutput()); err != nil { + return err + } + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("logger.Out field after SetLoggerConsoleOutput(): %p\n", c.GetLogger().Out), + Fields: logrus.Fields{ + "line": logging.GetLineNumber(), + }, + }) + + // FIXME: This seems like a pretty ugly tradeoff just to avoid golint + // complaining about the use of an else block; we now have `err` declared + // at function scope instead of per block scope in order to directly + // assign to struct field. + c.logFileHandle, err = logging.SetLoggerLogFile(c.logger, c.GetLogFilePath()) + if err != nil { + + // Need to collect the error for display later + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.ErrorLevel, + Message: fmt.Sprintf("%s", err), + Fields: logrus.Fields{ + "log_file": c.GetLogFilePath(), + "line": logging.GetLineNumber(), + "log_file_handle": c.GetLogFileHandle(), + }, + }) + + return err + } + + if err = logging.SetLoggerLevel(c.logger, c.GetLogLevel()); err != nil { + return err + } + + // https://godoc.org/github.com/sirupsen/logrus#New + // https://godoc.org/github.com/sirupsen/logrus#Logger + + // make sure that the user actually requested syslog logging as it is + // currently supported on UNIX only. + if c.GetUseSyslog() { + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.InfoLevel, + Message: "Syslog logging requested, attempting to enable it", + Fields: logrus.Fields{ + "use_syslog": c.GetUseSyslog(), + "line": logging.GetLineNumber(), + }, + }) + + if err := logging.EnableSyslogLogging(c.logger, &logging.Buffer, c.GetLogLevel()); err != nil { + // TODO: Is this sufficient cause for failing? Perhaps if a local + // log file is not also set consider it a failure? + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.ErrorLevel, + Message: fmt.Sprintf("Failed to enable syslog logging: %s", err), + Fields: logrus.Fields{ + "use_syslog": c.GetUseSyslog(), + "line": logging.GetLineNumber(), + }, + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.WarnLevel, + Message: "Proceeding without syslog logging", + Fields: logrus.Fields{ + "use_syslog": c.GetUseSyslog(), + "line": logging.GetLineNumber(), + }, + }) + } + } else { + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: "Syslog logging not requested, not enabling", + Fields: logrus.Fields{ + "use_syslog": c.GetUseSyslog(), + "line": logging.GetLineNumber(), + }, + }) + } + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: "logging object details at end of SetLoggerConfig()", + Fields: logrus.Fields{ + "logger": fmt.Sprintf("%+v\n", c.GetLogger()), + // TODO: Re-evaluate potential for field ref on nil pointer + "logger_out": fmt.Sprintf("%+v\n", c.GetLogger().Out), + "line": logging.GetLineNumber(), + }, + }) + + // FIXME: Placeholder for now + + return nil + +} diff --git a/config/merge.go b/config/merge.go new file mode 100644 index 00000000..713afc42 --- /dev/null +++ b/config/merge.go @@ -0,0 +1,172 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "fmt" + + "github.com/atc0005/elbow/logging" + "github.com/sirupsen/logrus" +) + +// MergeConfig receives source, destination and default Config objects and +// merges select, non-nil field values from the source Config object to +// the destination config object, overwriting any field value already present. +// +// The goal is to respect the current documented configuration precedence for +// multiple configuration sources (e.g., config file and command-line flags). +func MergeConfig(destination *Config, source Config) error { + + // FIXME: How can we get all field names programatically so we don't have to + // manually reference each field? + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: "MergeConfig starting", + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Source struct (raw): %+v", source), + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Source struct (string): %s", source.String()), + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Destination struct (raw): %+v", destination), + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Destination struct (string): %s", destination.String()), + }) + + if source.AppName != "" { + destination.AppName = source.AppName + } + + if source.AppDescription != "" { + destination.AppDescription = source.AppDescription + } + + if source.AppURL != "" { + destination.AppURL = source.AppURL + } + + if source.AppVersion != "" { + destination.AppVersion = source.AppVersion + } + + if source.Paths != nil { + destination.Paths = source.Paths + } + + if source.FileExtensions != nil { + destination.FileExtensions = source.FileExtensions + } + + if source.FilePattern != nil { + *destination.FilePattern = *source.FilePattern + } + + if source.FileAge != nil { + *destination.FileAge = *source.FileAge + } + + if source.NumFilesToKeep != nil { + *destination.NumFilesToKeep = *source.NumFilesToKeep + } + + if source.KeepOldest != nil { + *destination.KeepOldest = *source.KeepOldest + } + + if source.Remove != nil { + *destination.Remove = *source.Remove + } + + if source.IgnoreErrors != nil { + *destination.IgnoreErrors = *source.IgnoreErrors + } + + if source.RecursiveSearch != nil { + *destination.RecursiveSearch = *source.RecursiveSearch + } + + if source.LogLevel != nil { + *destination.LogLevel = *source.LogLevel + } + + if source.LogFormat != nil { + *destination.LogFormat = *source.LogFormat + } + + if source.LogFilePath != nil { + *destination.LogFilePath = *source.LogFilePath + } + + // TODO: Was this intentionally left out in the past? + // This was added as part of the work on #161 to implement + // testing of the MergeConfig() function. + if source.ConfigFile != nil { + *destination.ConfigFile = *source.ConfigFile + } + + if source.ConsoleOutput != nil { + // TODO: Add debug output for each of these decisions, but enable it + // only for debug/troubleshooting builds. + // fmt.Printf("Using %q for ConsoleOutput\n", *source.ConsoleOutput) + *destination.ConsoleOutput = *source.ConsoleOutput + } + + if source.UseSyslog != nil { + *destination.UseSyslog = *source.UseSyslog + } + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: "MergeConfig ending", + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Source struct (raw): %+v", source), + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Source struct (string): %s", source.String()), + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Destination struct (raw): %+v", destination), + }) + + logging.Buffer.Add(logging.LogRecord{ + Level: logrus.DebugLevel, + Message: fmt.Sprintf("Destination struct (string): %s", destination.String()), + }) + + // FIXME: Placeholder + // FIXME: What useful error code would we return from this function? + return nil +} diff --git a/config/merge_complete_configs_test.go b/config/merge_complete_configs_test.go new file mode 100644 index 00000000..9ddd2977 --- /dev/null +++ b/config/merge_complete_configs_test.go @@ -0,0 +1,348 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "bytes" + "os" + "runtime" + "testing" + + "github.com/alexflint/go-arg" + "github.com/sirupsen/logrus" +) + +// TestMergeConfigUsingCompleteConfigObjects creates multiple Config structs +// and merges them in sequence, verifying that after each MergeConfig +// operation that the initial config struct has been updated to reflect the +// new state. +func TestMergeConfigUsingCompleteConfigObjects(t *testing.T) { + + // + // Base Config testing + // + + // Validation will fail if this is all we do since the default config + // doesn't contain any Paths to process. + baseConfig := NewDefaultConfig("x.y.z") + + testPaths := []string{"/tmp/elbow/path1"} + baseConfig.Paths = testPaths + baseConfig.logger = baseConfig.GetLogger() + + // TODO: Any reason to set this? The Validate() method does not currently + // enforce that the FileExtensions field be set. + // Answer: Yes, because we want to ensure that the final MergeConfig() + // result reflects our test case(s). + // + testFileExtensions := []string{".yaml", ".json"} + baseConfig.FileExtensions = testFileExtensions + + // Validate the base config settings before making further changes that + // could potentially break the configuration. + if err := baseConfig.Validate(); err != nil { + t.Error("Unable to validate base configuration before merge:", err) + } else { + t.Log("Validation of base config settings before merge successful") + } + + // + // File Config testing + // + + // ConfigFilePath for use with fileConfig struct tests + fcConfigFilePath := "/usr/local/etc/elbow/config.toml" + fileConfig := Config{ + + // This is required as well to pass validation checks + ConfigFile: &fcConfigFilePath, + + // Not going to merge this in, but we have to specify it in order to + // pass validation checks. + logger: logrus.New(), + } + + // TODO: This currently mirrors the example config file. Replace with a read + // against that file? + var defaultConfigFile = []byte(` + [filehandling] + + file_age = 1 + files_to_keep = 2 + keep_oldest = true + remove = true + ignore_errors = true + pattern = "reach-masterdev-" + file_extensions = [ + ".war", + ".tmp", + ] + + + [search] + + recursive_search = true + paths = [ + "/tmp/elbow/path1", + "/tmp/elbow/path2", + ] + + + [logging] + + log_level = "debug" + log_format = "json" + log_file_path = "/var/log/elbow.log" + console_output = "stderr" + use_syslog = true`) + + // Use our default in-memory config file settings + r := bytes.NewReader(defaultConfigFile) + + if err := fileConfig.LoadConfigFile(r); err != nil { + t.Error("Unable to load in-memory configuration:", err) + } else { + t.Log("Loaded in-memory configuration file") + } + + t.Log("LoadConfigFile wiped the existing struct, reconstructing AppMetadata fields to pass validation checks") + fileConfig.AppName = baseConfig.GetAppName() + fileConfig.AppVersion = baseConfig.GetAppVersion() + fileConfig.AppURL = baseConfig.GetAppURL() + fileConfig.AppDescription = baseConfig.GetAppDescription() + + // Validate the config file settings + if err := fileConfig.Validate(); err != nil { + t.Error("Unable to validate file config:", err) + } else { + t.Log("Validation of file config settings successful") + } + + if err := MergeConfig(&baseConfig, fileConfig); err != nil { + t.Errorf("Error merging config file settings with base config: %s", err) + } else { + t.Log("Merge of config file settings with base config successful") + } + + // Validate the base config settings after merging + if err := baseConfig.Validate(); err != nil { + t.Error("Unable to validate base configuration after merge:", err) + } else { + t.Log("Validation of base config settings after merge successful") + } + + // This is where we compare the field values of the baseConfig struct + // against the fileConfig struct to determine if any are different. In + // normal use of this application it is likely that the fields WOULD be + // different, but in our test case we have explicitly defined most fields + // of each config struct to have conflicting values. This allows us to + // simply our test case(s) so that we can assume each field has a value + // that should be compared and merged. + + CompareConfig(baseConfig, fileConfig, t) + + // + // Environment variables config testing + // + + // Setup environment variables for parsing with alexflint/go-arg package + + evConfigFilePath := "" + + // Bolt these on directly as we're likely going to abandon support for + // overriding these values anyway (haven't been able to come up with a + // legitimate reason why others would need or want to do so) + evAppName := "ElbowEnvVar" + evAppDescription := "something nifty here" + evAppURL := "https://example.com" + evAppVersion := "x.y.z" + + envConfig := Config{ + + // See earlier notes + AppMetadata: AppMetadata{ + AppName: evAppName, + AppDescription: evAppDescription, + AppURL: evAppURL, + AppVersion: evAppVersion, + }, + + // This is required as well to pass validation checks + ConfigFile: &evConfigFilePath, + + // Not going to merge this in, but we have to specify it in order to + // pass validation checks. + logger: logrus.New(), + } + + envVarTables := []struct { + envVar string + value string + }{ + {"ELBOW_FILE_PATTERN", "reach-masterqa-"}, + {"ELBOW_FILE_AGE", "3"}, + {"ELBOW_KEEP", "4"}, + {"ELBOW_KEEP_OLD", "false"}, + {"ELBOW_REMOVE", "false"}, + {"ELBOW_IGNORE_ERRORS", "false"}, + {"ELBOW_RECURSE", "false"}, + {"ELBOW_LOG_LEVEL", "warn"}, + {"ELBOW_LOG_FORMAT", "text"}, + {"ELBOW_LOG_FILE", "/var/log/elbow/env.log"}, + {"ELBOW_CONSOLE_OUTPUT", "stdout"}, + {"ELBOW_USE_SYSLOG", "false"}, + {"ELBOW_CONFIG_FILE", "/tmp/config.toml"}, + {"ELBOW_PATHS", "/tmp/elbow/path3"}, + {"ELBOW_EXTENSIONS", ".docx,.pptx"}, + } + + for _, table := range envVarTables { + t.Logf("Setting %q to %q", table.envVar, table.value) + os.Setenv(table.envVar, table.value) + } + + // https://stackoverflow.com/questions/33723300/how-to-test-the-passing-of-arguments-in-golang + // Save old command-line arguments so that we can restore them later + oldArgs := os.Args + + // Defer restoring original command-line arguments + defer func() { os.Args = oldArgs }() + + // Wipe existing command-line arguments so that the unexpected testing + // package flags don't trip alexflint/go-arg package logic for invalid + // flags. + // https://github.com/alexflint/go-arg/issues/97#issuecomment-561995206 + os.Args = nil + + t.Log("Parsing environment variables") + arg.MustParse(&envConfig) + t.Logf("Results of parsing environment variables: %v", envConfig.String()) + + // Validate the env vars settings + if err := envConfig.Validate(); err != nil { + t.Error("Unable to validate environment vars config:", err) + } else { + t.Log("Validation of environment vars config settings successful") + } + + if err := MergeConfig(&baseConfig, envConfig); err != nil { + t.Errorf("Error merging environment vars config settings with base config: %s", err) + } else { + t.Log("Merge of environment vars config settings with base config successful") + } + + // Validate the base config settings after merging + if err := baseConfig.Validate(); err != nil { + t.Error("Unable to validate base configuration after merge:", err) + } else { + t.Log("Validation of base config settings after merge successful") + } + + CompareConfig(baseConfig, envConfig, t) + + // Unset environment variables that we just set + for _, table := range envVarTables { + t.Logf("Unsetting %q\n", table.envVar) + os.Unsetenv(table.envVar) + } + + // + // Flags Config testing + // + + // Bolt these on directly as we're likely going to abandon support for + // overriding these values anyway (haven't been able to come up with a + // legitimate reason why others would need or want to do so) + flagsAppName := "ElbowFlagVar" + flagsAppDescription := "nothing fancy" + flagsAppURL := "https://example.org" + flagsAppVersion := "0.1.2" + + flagsConfigFilePath := "" + + flagsConfig := Config{ + + // See earlier notes + AppMetadata: AppMetadata{ + AppName: flagsAppName, + AppDescription: flagsAppDescription, + AppURL: flagsAppURL, + AppVersion: flagsAppVersion, + }, + + // This is required as well to pass validation checks + ConfigFile: &flagsConfigFilePath, + + // Not going to merge this in, but we have to specify it in order to + // pass validation checks. + logger: logrus.New(), + } + + // TODO: A useful way to automate retrieving the app name? + appName := "elbow" + if runtime.GOOS == "windows" { + appName += ".exe" + } + + // Note to self: Don't add/escape double-quotes here. The shell strips + // them away and the application never sees them. + os.Args = []string{ + appName, + "--paths", "/tmp/elbow/path4", + "--pattern", "reach-master-", + "--age", "5", + "--keep", "6", + "--remove", + "--ignore-errors", + "--recurse", + "--keep-old", + "--log-level", "info", + "--use-syslog", + "--log-format", "json", + "--console-output", "stderr", + "--log-file", "/var/log/elbow/flags.log", + "--config-file", "/tmp/configfile.toml", + "--extensions", ".java", ".class", + } + + t.Log("Parsing command-line flags") + arg.MustParse(&flagsConfig) + t.Logf("Results of parsing flags: %v", flagsConfig.String()) + + // Validate the config file settings + if err := flagsConfig.Validate(); err != nil { + t.Error("Unable to validate flags config:", err) + } else { + t.Log("Validation of flags config settings successful") + } + + if err := MergeConfig(&baseConfig, flagsConfig); err != nil { + t.Errorf("Error merging flags config settings with base config: %s", err) + } else { + t.Log("Merge of flags config settings with base config successful") + } + + // Validate the base config settings after merging + if err := baseConfig.Validate(); err != nil { + t.Error("Unable to validate base configuration after merge:", err) + } else { + t.Log("Validation of base config settings after merge successful") + } + + CompareConfig(baseConfig, flagsConfig, t) + +} diff --git a/config/merge_incomplete_configs_test.go b/config/merge_incomplete_configs_test.go new file mode 100644 index 00000000..8c0c0af1 --- /dev/null +++ b/config/merge_incomplete_configs_test.go @@ -0,0 +1,468 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "bytes" + "os" + "runtime" + "testing" + + "github.com/alexflint/go-arg" +) + +// TestMergeConfigUsingIncompleteConfigObjects creates multiple Config structs +// and merges them in sequence, verifying that after each MergeConfig +// operation that the initial config struct has been updated to reflect the +// new state. +func TestMergeConfigUsingIncompleteConfigObjects(t *testing.T) { + + // + // Base Config testing + // + + // Validation will fail if this is all we do since the default config + // doesn't contain any Paths to process. + baseConfig := NewDefaultConfig("x.y.z") + + testBaseConfigPaths := []string{"/tmp/elbow/path1"} + baseConfig.Paths = testBaseConfigPaths + baseConfig.logger = baseConfig.GetLogger() + + // TODO: Any reason to set this? The Validate() method does not currently + // enforce that the FileExtensions field be set. + // Answer: Yes, because we want to ensure that the final MergeConfig() + // result reflects our test case(s). + // + testBaseConfigFileExtensions := []string{".yaml", ".json"} + baseConfig.FileExtensions = testBaseConfigFileExtensions + + // Validate the base config settings before making further changes that + // could potentially break the configuration. + if err := baseConfig.Validate(); err != nil { + t.Error("Unable to validate base configuration before merge:", err) + } else { + t.Log("Validation of base config settings before merge successful") + } + + // + // File Config testing + // + + // TODO: This currently mirrors the example config file. Replace with a read + // against that file? + var defaultConfigFile = []byte(` + + [filehandling] + + file_age = 90 + files_to_keep = 1 + file_extensions = [ + ".war", + ] + + [search] + + recursive_search = true + paths = [ + "/tmp/elbow/path1", + ] + + [logging] + + log_level = "notice" + log_format = "text" + use_syslog = true`) + + // Use our default in-memory config file settings + r := bytes.NewReader(defaultConfigFile) + + fileConfig := Config{} + if err := fileConfig.LoadConfigFile(r); err != nil { + t.Error("Unable to load in-memory configuration:", err) + } else { + t.Log("Loaded in-memory configuration file") + } + + t.Log("LoadConfigFile wiped the existing struct, reconstructing AppMetadata fields to pass validation checks") + fileConfig.AppName = baseConfig.GetAppName() + fileConfig.AppVersion = baseConfig.GetAppVersion() + fileConfig.AppURL = baseConfig.GetAppURL() + fileConfig.AppDescription = baseConfig.GetAppDescription() + + // NOTE: We cannot validate the fileConfig object at this point because it + // is a partial object, missing the rest of the settings that are required + // for a full config validation check + + // Build EXPECTED baseConfig after merge so we can use Compare() against + // it and the actual baseConfig + + expectedAppNameAfterFileMerge := baseConfig.GetAppName() + expectedAppDescriptionAfterFileMerge := baseConfig.GetAppDescription() + expectedAppURLAfterFileMerge := baseConfig.GetAppURL() + expectedAppVersionAfterFileMerge := baseConfig.GetAppVersion() + expectedFilePatternAfterFileMerge := baseConfig.GetFilePattern() + + // Explicitly set these; we want to ensure the final merged config has + // the values we provided (incomplete fileConfig) and the prior baseConfig + // settings that we are not overriding + // NOTE: Paths and FileExtensions are set below after config struct is + // instantiated + expectedPathsAfterFileMerge := []string{"/tmp/elbow/path1"} + expectedFileExtensionsAfterFileMerge := []string{".war"} + expectedFileAgeAfterFileMerge := 90 + expectedNumFilesToKeepAfterFileMerge := 1 + expectedRecursiveSearchAfterFileMerge := true + expectedLogLevelAfterFileMerge := "notice" + expectedLogFormatAfterFileMerge := "text" + expectedUseSyslogAfterFileMerge := true + + expectedKeepOldestAfterFileMerge := baseConfig.GetKeepOldest() + expectedRemoveAfterFileMerge := baseConfig.GetRemove() + expectedIgnoreErrorsAfterFileMerge := baseConfig.GetIgnoreErrors() + expectedLogFilePathAfterFileMerge := baseConfig.GetLogFilePath() + expectedConsoleOutputAfterFileMerge := baseConfig.GetConsoleOutput() + expectedConfigFileAfterFileMerge := baseConfig.GetConfigFile() + + expectedBaseConfigAfterFileConfigMerge := Config{ + AppMetadata: AppMetadata{ + AppName: expectedAppNameAfterFileMerge, + AppDescription: expectedAppDescriptionAfterFileMerge, + AppURL: expectedAppURLAfterFileMerge, + AppVersion: expectedAppVersionAfterFileMerge, + }, + FileHandling: FileHandling{ + FilePattern: &expectedFilePatternAfterFileMerge, + FileAge: &expectedFileAgeAfterFileMerge, + NumFilesToKeep: &expectedNumFilesToKeepAfterFileMerge, + KeepOldest: &expectedKeepOldestAfterFileMerge, + Remove: &expectedRemoveAfterFileMerge, + IgnoreErrors: &expectedIgnoreErrorsAfterFileMerge, + }, + Logging: Logging{ + LogLevel: &expectedLogLevelAfterFileMerge, + LogFormat: &expectedLogFormatAfterFileMerge, + LogFilePath: &expectedLogFilePathAfterFileMerge, + ConsoleOutput: &expectedConsoleOutputAfterFileMerge, + UseSyslog: &expectedUseSyslogAfterFileMerge, + }, + Search: Search{ + //Paths: , + RecursiveSearch: &expectedRecursiveSearchAfterFileMerge, + }, + ConfigFile: &expectedConfigFileAfterFileMerge, + logger: baseConfig.GetLogger(), + } + + expectedBaseConfigAfterFileConfigMerge.Paths = expectedPathsAfterFileMerge + expectedBaseConfigAfterFileConfigMerge.FileExtensions = expectedFileExtensionsAfterFileMerge + + // Validate the expectedBaseConfigAfterFileConfigMerge config settings + // to ensure that we're not going to compare against a broken configuration + if err := expectedBaseConfigAfterFileConfigMerge.Validate(); err != nil { + t.Error("Unable to validate expectedBaseConfigAfterFileConfigMerge before merge:", err) + } else { + t.Log("Validation of expectedBaseConfigAfterFileConfigMerge settings before merge successful") + } + + if err := MergeConfig(&baseConfig, fileConfig); err != nil { + t.Errorf("Error merging config file settings with base config: %s", err) + } else { + t.Log("Merge of config file settings with base config successful") + } + + // Validate the base config settings after merging + if err := baseConfig.Validate(); err != nil { + t.Error("Unable to validate base configuration after merge:", err) + } else { + t.Log("Validation of base config settings after merge successful") + } + + // This is where we compare the field values of the baseConfig struct + // against the expectedBaseConfigAfterFileConfigMerge struct to determine + // if the merge results are as expected. + + CompareConfig(baseConfig, expectedBaseConfigAfterFileConfigMerge, t) + + // + // Environment variables config testing + // + + // Setup subset of total environment variables for parsing with + // alexflint/go-arg package. These values should override baseConfig + // settings + envVarTables := []struct { + envVar string + value string + }{ + {"ELBOW_FILE_PATTERN", "reach-masterqa-"}, + {"ELBOW_FILE_AGE", "3"}, + {"ELBOW_KEEP", "4"}, + {"ELBOW_KEEP_OLD", "false"}, + {"ELBOW_REMOVE", "true"}, + {"ELBOW_LOG_FORMAT", "text"}, + {"ELBOW_LOG_FILE", "/var/log/elbow/env.log"}, + {"ELBOW_PATHS", "/tmp/elbow/path3"}, + {"ELBOW_EXTENSIONS", ".docx,.pptx"}, + } + + envConfig := Config{} + + t.Log("Explictly setting AppMetadata fields to pass validation checks") + envConfig.AppName = baseConfig.GetAppName() + envConfig.AppVersion = baseConfig.GetAppVersion() + envConfig.AppURL = baseConfig.GetAppURL() + envConfig.AppDescription = baseConfig.GetAppDescription() + + for _, table := range envVarTables { + t.Logf("Setting %q to %q", table.envVar, table.value) + os.Setenv(table.envVar, table.value) + } + + // https://stackoverflow.com/questions/33723300/how-to-test-the-passing-of-arguments-in-golang + // Save old command-line arguments so that we can restore them later + oldArgs := os.Args + + // Defer restoring original command-line arguments + defer func() { os.Args = oldArgs }() + + // Wipe existing command-line arguments so that the unexpected testing + // package flags don't trip alexflint/go-arg package logic for invalid + // flags. + // https://github.com/alexflint/go-arg/issues/97#issuecomment-561995206 + os.Args = nil + + t.Log("Parsing environment variables") + arg.MustParse(&envConfig) + t.Logf("Results of parsing environment variables: %v", envConfig.String()) + + // NOTE: We cannot validate envConfig here since the set of options is + // incomplete. + + // Build EXPECTED baseConfig after env vars merge so we can use Compare() + // against it and the actual baseConfig + + expectedAppNameAfterEnvVarsMerge := baseConfig.GetAppName() + expectedAppDescriptionAfterEnvVarsMerge := baseConfig.GetAppDescription() + expectedAppURLAfterEnvVarsMerge := baseConfig.GetAppURL() + expectedAppVersionAfterEnvVarsMerge := baseConfig.GetAppVersion() + + // Explicitly set these; we want to ensure the final merged config has + // the values we provided (incomplete fileConfig) and the prior baseConfig + // settings that we are not overriding + // NOTE: Paths and FileExtensions are set below after config struct is + // instantiated + expectedPathsAfterEnvVarsMerge := []string{"/tmp/elbow/path3"} + expectedFileExtensionsAfterEnvVarsMerge := []string{".docx", ".pptx"} + expectedFilePatternAfterEnvVarsMerge := "reach-masterqa-" + expectedFileAgeAfterEnvVarsMerge := 3 + expectedNumFilesToKeepAfterEnvVarsMerge := 4 + expectedKeepOldestAfterEnvVarsMerge := false + expectedRemoveAfterEnvVarsMerge := true + expectedLogFormatAfterEnvVarsMerge := "text" + expectedLogFilePathAfterEnvVarsMerge := "/var/log/elbow/env.log" + + expectedRecursiveSearchAfterEnvVarsMerge := baseConfig.GetRecursiveSearch() + expectedLogLevelAfterEnvVarsMerge := baseConfig.GetLogLevel() + expectedUseSyslogAfterEnvVarsMerge := baseConfig.GetUseSyslog() + expectedIgnoreErrorsAfterEnvVarsMerge := baseConfig.GetIgnoreErrors() + expectedConsoleOutputAfterEnvVarsMerge := baseConfig.GetConsoleOutput() + expectedConfigFileAfterEnvVarsMerge := baseConfig.GetConfigFile() + + expectedBaseConfigAfterEnvVarsMerge := Config{ + AppMetadata: AppMetadata{ + AppName: expectedAppNameAfterEnvVarsMerge, + AppDescription: expectedAppDescriptionAfterEnvVarsMerge, + AppURL: expectedAppURLAfterEnvVarsMerge, + AppVersion: expectedAppVersionAfterEnvVarsMerge, + }, + FileHandling: FileHandling{ + FilePattern: &expectedFilePatternAfterEnvVarsMerge, + FileAge: &expectedFileAgeAfterEnvVarsMerge, + NumFilesToKeep: &expectedNumFilesToKeepAfterEnvVarsMerge, + KeepOldest: &expectedKeepOldestAfterEnvVarsMerge, + Remove: &expectedRemoveAfterEnvVarsMerge, + IgnoreErrors: &expectedIgnoreErrorsAfterEnvVarsMerge, + }, + Logging: Logging{ + LogLevel: &expectedLogLevelAfterEnvVarsMerge, + LogFormat: &expectedLogFormatAfterEnvVarsMerge, + LogFilePath: &expectedLogFilePathAfterEnvVarsMerge, + ConsoleOutput: &expectedConsoleOutputAfterEnvVarsMerge, + UseSyslog: &expectedUseSyslogAfterEnvVarsMerge, + }, + Search: Search{ + RecursiveSearch: &expectedRecursiveSearchAfterEnvVarsMerge, + }, + ConfigFile: &expectedConfigFileAfterEnvVarsMerge, + logger: baseConfig.GetLogger(), + } + + expectedBaseConfigAfterEnvVarsMerge.Paths = expectedPathsAfterEnvVarsMerge + expectedBaseConfigAfterEnvVarsMerge.FileExtensions = expectedFileExtensionsAfterEnvVarsMerge + + // Validate the env vars settings + if err := expectedBaseConfigAfterEnvVarsMerge.Validate(); err != nil { + t.Error("Unable to validate expectedBaseConfigAfterEnvVarsMerge before merge:", err) + } else { + t.Log("Validation of expectedBaseConfigAfterEnvVarsMerge before merge successful") + } + + if err := MergeConfig(&baseConfig, envConfig); err != nil { + t.Errorf("Error merging environment vars config settings with base config: %s", err) + } else { + t.Log("Merge of environment vars config settings with base config successful") + } + + // Validate the base config settings after merging + if err := baseConfig.Validate(); err != nil { + t.Error("Unable to validate base configuration after merge:", err) + } else { + t.Log("Validation of base config settings after merge successful") + } + + CompareConfig(baseConfig, expectedBaseConfigAfterEnvVarsMerge, t) + + // Unset environment variables that we just set + for _, table := range envVarTables { + t.Logf("Unsetting %q\n", table.envVar) + os.Unsetenv(table.envVar) + } + + // + // Flags Config testing + // + + flagsConfig := Config{} + + t.Log("Explictly setting AppMetadata fields to pass validation checks") + flagsConfig.AppName = baseConfig.GetAppName() + flagsConfig.AppVersion = baseConfig.GetAppVersion() + flagsConfig.AppURL = baseConfig.GetAppURL() + flagsConfig.AppDescription = baseConfig.GetAppDescription() + + // TODO: A useful way to automate retrieving the app name? + appName := "elbow" + if runtime.GOOS == "windows" { + appName += ".exe" + } + + // Note to self: Don't add/escape double-quotes here. The shell strips + // them away and the application never sees them. + os.Args = []string{ + appName, + "--pattern", "reach-master-", + "--age", "5", + "--keep", "6", + "--remove", + "--log-level", "panic", + "--log-format", "json", + "--console-output", "stderr", + "--extensions", ".java", ".class", + } + + t.Log("Parsing command-line flags") + arg.MustParse(&flagsConfig) + t.Logf("Results of parsing flags: %v", flagsConfig.String()) + + // NOTE: We cannot validate flagsConfig here since the set of options is + // incomplete. + + // Build EXPECTED baseConfig after flags merge so we can use Compare() + // against it and the actual baseConfig + + expectedAppNameAfterFlagsMerge := baseConfig.GetAppName() + expectedAppDescriptionAfterFlagsMerge := baseConfig.GetAppDescription() + expectedAppURLAfterFlagsMerge := baseConfig.GetAppURL() + expectedAppVersionAfterFlagsMerge := baseConfig.GetAppVersion() + expectedPathsAfterFlagsMerge := baseConfig.GetPaths() + expectedKeepOldestAfterFlagsMerge := baseConfig.GetKeepOldest() + expectedLogFilePathAfterFlagsMerge := baseConfig.GetLogFilePath() + expectedRecursiveSearchAfterFlagsMerge := baseConfig.GetRecursiveSearch() + expectedUseSyslogAfterFlagsMerge := baseConfig.GetUseSyslog() + expectedIgnoreErrorsAfterFlagsMerge := baseConfig.GetIgnoreErrors() + expectedConfigFileAfterFlagsMerge := baseConfig.GetConfigFile() + + // Explicitly set these; we want to ensure the final merged config has + // the values we provided (incomplete fileConfig) and the prior baseConfig + // settings that we are not overriding + // NOTE: Paths and FileExtensions are set below after config struct is + // instantiated + expectedFileExtensionsAfterFlagsMerge := []string{".java", ".class"} + expectedFilePatternAfterFlagsMerge := "reach-master-" + expectedFileAgeAfterFlagsMerge := 5 + expectedNumFilesToKeepAfterFlagsMerge := 6 + expectedRemoveAfterFlagsMerge := true + expectedLogFormatAfterFlagsMerge := "json" + expectedLogLevelAfterFlagsMerge := "panic" + expectedConsoleOutputAfterFlagsMerge := "stderr" + + expectedBaseConfigAfterFlagsMerge := Config{ + AppMetadata: AppMetadata{ + AppName: expectedAppNameAfterFlagsMerge, + AppDescription: expectedAppDescriptionAfterFlagsMerge, + AppURL: expectedAppURLAfterFlagsMerge, + AppVersion: expectedAppVersionAfterFlagsMerge, + }, + FileHandling: FileHandling{ + FilePattern: &expectedFilePatternAfterFlagsMerge, + FileAge: &expectedFileAgeAfterFlagsMerge, + NumFilesToKeep: &expectedNumFilesToKeepAfterFlagsMerge, + KeepOldest: &expectedKeepOldestAfterFlagsMerge, + Remove: &expectedRemoveAfterFlagsMerge, + IgnoreErrors: &expectedIgnoreErrorsAfterFlagsMerge, + }, + Logging: Logging{ + LogLevel: &expectedLogLevelAfterFlagsMerge, + LogFormat: &expectedLogFormatAfterFlagsMerge, + LogFilePath: &expectedLogFilePathAfterFlagsMerge, + ConsoleOutput: &expectedConsoleOutputAfterFlagsMerge, + UseSyslog: &expectedUseSyslogAfterFlagsMerge, + }, + Search: Search{ + RecursiveSearch: &expectedRecursiveSearchAfterFlagsMerge, + }, + ConfigFile: &expectedConfigFileAfterFlagsMerge, + logger: baseConfig.GetLogger(), + } + + expectedBaseConfigAfterFlagsMerge.Paths = expectedPathsAfterFlagsMerge + expectedBaseConfigAfterFlagsMerge.FileExtensions = expectedFileExtensionsAfterFlagsMerge + + // Validate the config file settings + if err := expectedBaseConfigAfterFlagsMerge.Validate(); err != nil { + t.Error("Unable to validate expectedBaseConfigAfterFlagsMerge before merging:", err) + } else { + t.Log("Validation of expectedBaseConfigAfterFlagsMerge before merging successful") + } + + if err := MergeConfig(&baseConfig, flagsConfig); err != nil { + t.Errorf("Error merging flags config settings with base config: %s", err) + } else { + t.Log("Merge of flags config settings with base config successful") + } + + // Validate the base config settings after merging + if err := baseConfig.Validate(); err != nil { + t.Error("Unable to validate base configuration after merge:", err) + } else { + t.Log("Validation of base config settings after merge successful") + } + + CompareConfig(baseConfig, expectedBaseConfigAfterFlagsMerge, t) + +} diff --git a/config/validate.go b/config/validate.go new file mode 100644 index 00000000..d9dcb229 --- /dev/null +++ b/config/validate.go @@ -0,0 +1,160 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "fmt" +) + +// Validate verifies all struct fields have been provided acceptable values +func (c Config) Validate() error { + + if c.AppName == "" { + return fmt.Errorf("field AppName not configured") + } + + if c.AppDescription == "" { + return fmt.Errorf("field AppDescription not configured") + } + + if c.AppVersion == "" { + return fmt.Errorf("field AppVersion not configured") + } + + if c.AppURL == "" { + return fmt.Errorf("field AppURL not configured") + } + + // FilePattern is optional, but since has an underlying string type with a + // default of empty string we can assert that the pointer isn't + if c.FilePattern == nil { + return fmt.Errorf("field FilePattern not configured") + } + + // FileExtensions is optional + // Discovered files are checked against FileExtensions later + // This isn't a pointer, but rather a string slice. The user may opt to + // not configure this setting, so having a `nil` state for this setting is + // normal? + // + // if c.FileExtensions == nil { + // return false, fmt.Errorf("file extensions option not configured") + // } + + if c.Paths == nil { + return fmt.Errorf("one or more paths not provided") + } + + // RecursiveSearch is optional + if c.RecursiveSearch == nil { + return fmt.Errorf("field RecursiveSearch not configured") + } + + // NumFilesToKeep is optional, but should be configured via + // if specified we should make sure it is a non-negative number. + switch { + case c.NumFilesToKeep == nil: + return fmt.Errorf("field NumFilesToKeep not configured") + case *c.NumFilesToKeep < 0: + return fmt.Errorf("negative number for files to keep not supported") + } + + // We only want to work with positive file modification times 0 is + // acceptable as it is the default value and indicates that the user has + // not chosen to use the flag (or has chosen improperly and it will be + // ignored). + switch { + case c.FileAge == nil: + return fmt.Errorf("field FileAge not configured") + case *c.FileAge < 0: + return fmt.Errorf("negative number for file age not supported") + } + + if c.KeepOldest == nil { + return fmt.Errorf("field KeepOldest not configured") + } + + if c.Remove == nil { + return fmt.Errorf("field Remove not configured") + } + + if c.IgnoreErrors == nil { + return fmt.Errorf("field IgnoreErrors not configured") + } + + switch { + case c.LogFormat == nil: + return fmt.Errorf("field LogFormat not configured") + case *c.LogFormat == "text": + case *c.LogFormat == "json": + default: + return fmt.Errorf("invalid option %q provided for log format", *c.LogFormat) + } + + // LogFilePath is optional, but should still have a non-nil value + if c.LogFilePath == nil { + return fmt.Errorf("field LogFilePath not configured") + } + + // Do nothing for valid choices, return false if invalid value specified + switch { + case c.ConsoleOutput == nil: + return fmt.Errorf("field ConsoleOutput not configured") + case *c.ConsoleOutput == "stdout": + case *c.ConsoleOutput == "stderr": + default: + return fmt.Errorf("invalid option %q provided for console output destination", *c.ConsoleOutput) + } + + switch { + case c.LogLevel == nil: + return fmt.Errorf("field LogLevel not configured") + case *c.LogLevel == "emergency": + case *c.LogLevel == "alert": + case *c.LogLevel == "critical": + case *c.LogLevel == "panic": + case *c.LogLevel == "fatal": + case *c.LogLevel == "error": + case *c.LogLevel == "warn": + case *c.LogLevel == "info": + case *c.LogLevel == "notice": + case *c.LogLevel == "debug": + case *c.LogLevel == "trace": + default: + return fmt.Errorf("invalid option %q provided for log level", *c.LogLevel) + } + + // UseSyslog is optional, but should still be initialized + if c.UseSyslog == nil { + return fmt.Errorf("field UseSyslog not configured") + } + + // Make sure that a valid logger has been created + if c.logger == nil { + return fmt.Errorf("field logger not configured") + } + + // Using a config file is optional, but should still be initialized so + // that user values can be stored later if specified. + if c.ConfigFile == nil { + return fmt.Errorf("field ConfigFile not configured") + } + + // Optimist + return nil + +} diff --git a/config/validate_test.go b/config/validate_test.go new file mode 100644 index 00000000..69ff2c69 --- /dev/null +++ b/config/validate_test.go @@ -0,0 +1,608 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "testing" +) + +// Fix linting error +// string `fakeValue` has 3 occurrences, make it a constant (goconst) +const fakeValue = "fakeValue" + +// This is *mostly* a default config struct with the addition of config.Paths +// and config.FileExtensions fields set to fill out the set. +func TestValidate(t *testing.T) { + + // Create and initialize struct + // One at a time, set test-target fields to nil + // Validate config struct + // Set field back to good value + + c := NewDefaultConfig("x.y.z") + + testPaths := []string{"/tmp/elbow/path1"} + testFileExtensions := []string{".tmp", ".war"} + + c.Paths = testPaths + c.FileExtensions = testFileExtensions + c.logger = c.GetLogger() + + t.Run("Validating mostly default config", func(t *testing.T) { + // This should pass + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config created from NewDefaultConfig(): %s", err) + } else { + t.Log("No errors encountered when instantiating configuration") + } + }) + + t.Run("AppName set to empty string", func(t *testing.T) { + tmpAppName := c.AppName + c.AppName = "" + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil AppName: %s", err) + } else { + t.Logf("Config failed as expected after setting AppName to nil: %s", err) + } + // Set back to prior value + c.AppName = tmpAppName + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring AppName: %s", err) + } else { + t.Log("Validation successful after restoring AppName field") + } + }) + + t.Run("AppDescription set to empty string", func(t *testing.T) { + tmpAppDescription := c.AppDescription + c.AppDescription = "" + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil AppDescription: %s", err) + } else { + t.Logf("Config failed as expected after setting AppDescription to nil: %s", err) + } + // Set back to prior value + c.AppDescription = tmpAppDescription + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring AppDescription: %s", err) + } else { + t.Log("Validation successful after restoring AppDescription field") + } + }) + + t.Run("AppVersion set to empty string", func(t *testing.T) { + tmpAppVersion := c.AppVersion + c.AppVersion = "" + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil AppVersion: %s", err) + } else { + t.Logf("Config failed as expected after setting AppVersion to nil: %s", err) + } + // Set back to prior value + c.AppVersion = tmpAppVersion + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring AppVersion: %s", err) + } else { + t.Log("Validation successful after restoring AppVersion field") + } + }) + + t.Run("AppURL set to empty string", func(t *testing.T) { + tmpAppURL := c.AppURL + c.AppURL = "" + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil AppURL: %s", err) + } else { + t.Logf("Config failed as expected after setting AppURL to nil: %s", err) + } + // Set back to prior value + c.AppURL = tmpAppURL + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring AppURL: %s", err) + } else { + t.Log("Validation successful after restoring AppURL field") + } + }) + + t.Run("FilePattern set to nil", func(t *testing.T) { + tmpFilePattern := c.FilePattern + //t.Logf("c.FilePattern before setting to nil: %p", c.FilePattern) + c.FilePattern = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil FilePattern: %s", err) + } else { + t.Logf("Config failed as expected after setting FilePattern to nil: %s", err) + } + // Set back to prior value + c.FilePattern = tmpFilePattern + //t.Logf("c.FilePattern address after resetting back to original value: %p", c.FilePattern) + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring FilePattern: %s", err) + } else { + t.Log("Validation successful after restoring FilePattern field") + } + }) + + t.Run("Paths set to nil", func(t *testing.T) { + tmpPaths := c.Paths + c.Paths = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil Paths: %s", err) + } else { + t.Logf("Config failed as expected after setting Paths to nil: %s", err) + } + // Set back to prior value + c.Paths = tmpPaths + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring Paths: %s", err) + } else { + t.Log("Validation successful after restoring Paths field") + } + }) + + t.Run("RecursiveSearch set to nil", func(t *testing.T) { + tmpRecursiveSearch := c.RecursiveSearch + c.RecursiveSearch = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil RecursiveSearch: %s", err) + } else { + t.Logf("Config failed as expected after setting RecursiveSearch to nil: %s", err) + } + // Set back to prior value + c.RecursiveSearch = tmpRecursiveSearch + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring RecursiveSearch: %s", err) + } else { + t.Log("Validation successful after restoring RecursiveSearch field") + } + }) + + t.Run("NumFilesToKeep set to nil", func(t *testing.T) { + tmpNumFilesToKeep := c.NumFilesToKeep + c.NumFilesToKeep = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil NumFilesToKeep: %s", err) + } else { + t.Logf("Config failed as expected after setting NumFilesToKeep to nil: %s", err) + } + // Set back to prior value + c.NumFilesToKeep = tmpNumFilesToKeep + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring NumFilesToKeep: %s", err) + } else { + t.Log("Validation successful after restoring NumFilesToKeep field") + } + }) + + t.Run("NumFilesToKeep set to invalid value", func(t *testing.T) { + tmpNumFilesToKeep := *c.NumFilesToKeep + *c.NumFilesToKeep = -1 + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on invalid value %d for NumFilesToKeep: %s", *c.NumFilesToKeep, err) + } else { + t.Logf("Config failed as expected after setting NumFilesToKeep to %d: %s", *c.NumFilesToKeep, err) + } + // Set back to prior value + *c.NumFilesToKeep = tmpNumFilesToKeep + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring NumFilesToKeep: %s", err) + } else { + t.Log("Validation successful after restoring NumFilesToKeep field") + } + }) + + t.Run("NumFilesToKeep set to valid value", func(t *testing.T) { + tmpNumFilesToKeep := *c.NumFilesToKeep + *c.NumFilesToKeep = 1 + if err := c.Validate(); err == nil { + t.Logf("Config passed as expected after setting NumFilesToKeep to %d: %v", *c.NumFilesToKeep, err) + } else { + t.Errorf("Config failed, but should have passed on valid value %d for NumFilesToKeep: %v", *c.NumFilesToKeep, err) + } + // Set back to prior value + *c.NumFilesToKeep = tmpNumFilesToKeep + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring NumFilesToKeep: %s", err) + } else { + t.Log("Validation successful after restoring NumFilesToKeep field") + } + }) + + t.Run("FileAge set to nil", func(t *testing.T) { + tmpFileAge := c.FileAge + c.FileAge = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil FileAge: %s", err) + } else { + t.Logf("Config failed as expected after setting FileAge to nil: %s", err) + } + // Set back to prior value + c.FileAge = tmpFileAge + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring FileAge: %s", err) + } else { + t.Log("Validation successful after restoring FileAge field") + } + }) + + t.Run("FileAge set to invalid value", func(t *testing.T) { + tmpFileAge := *c.FileAge + *c.FileAge = -1 + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on invalid value %d for FileAge: %s", *c.FileAge, err) + } else { + t.Logf("Config failed as expected after setting FileAge to %d: %s", *c.FileAge, err) + } + // Set back to prior value + *c.FileAge = tmpFileAge + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring FileAge: %s", err) + } else { + t.Log("Validation successful after restoring FileAge field") + } + }) + + t.Run("FileAge set to valid value", func(t *testing.T) { + tmpFileAge := *c.FileAge + *c.FileAge = 1 + if err := c.Validate(); err == nil { + t.Logf("Config passed as expected after setting FileAge to %d: %v", *c.FileAge, err) + } else { + t.Errorf("Config failed, but should have passed on valid value %d for FileAge: %v", *c.FileAge, err) + } + // Set back to prior value + *c.FileAge = tmpFileAge + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring FileAge: %s", err) + } else { + t.Log("Validation successful after restoring FileAge field") + } + }) + + t.Run("KeepOldest set to nil", func(t *testing.T) { + tmpKeepOldest := c.KeepOldest + c.KeepOldest = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil KeepOldest: %s", err) + } else { + t.Logf("Config failed as expected after setting KeepOldest to nil: %s", err) + } + // Set back to prior value + c.KeepOldest = tmpKeepOldest + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring KeepOldest: %s", err) + } else { + t.Log("Validation successful after restoring KeepOldest field") + } + }) + + t.Run("Remove set to nil", func(t *testing.T) { + tmpRemove := c.Remove + c.Remove = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil Remove: %s", err) + } else { + t.Logf("Config failed as expected after setting Remove to nil: %s", err) + } + // Set back to prior value + c.Remove = tmpRemove + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring Remove: %s", err) + } else { + t.Log("Validation successful after restoring Remove field") + } + }) + + t.Run("IgnoreErrors set to nil", func(t *testing.T) { + tmpIgnoreErrors := c.IgnoreErrors + c.IgnoreErrors = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil IgnoreErrors: %s", err) + } else { + t.Logf("Config failed as expected after setting IgnoreErrors to nil: %s", err) + } + // Set back to prior value + c.IgnoreErrors = tmpIgnoreErrors + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring IgnoreErrors: %s", err) + } else { + t.Log("Validation successful after restoring IgnoreErrors field") + } + }) + + t.Run("LogFormat set to nil", func(t *testing.T) { + tmpLogFormat := c.LogFormat + c.LogFormat = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil LogFormat: %s", err) + } else { + t.Logf("Config failed as expected after setting LogFormat to nil: %s", err) + } + // Set back to prior value + c.LogFormat = tmpLogFormat + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring LogFormat: %s", err) + } else { + t.Log("Validation successful after restoring LogFormat field") + } + }) + + t.Run("LogFormat set to invalid value", func(t *testing.T) { + tmpLogFormat := *c.LogFormat + *c.LogFormat = fakeValue + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on invalid value %q for LogFormat: %v", *c.LogFormat, err) + } else { + t.Logf("Config failed as expected after setting LogFormat to %q: %v", *c.LogFormat, err) + } + // Set back to prior value + *c.LogFormat = tmpLogFormat + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring LogFormat: %s", err) + } else { + t.Log("Validation successful after restoring LogFormat field") + } + }) + + t.Run("LogFormat set to valid values", func(t *testing.T) { + + tmpLogFormat := *c.LogFormat + tests := []string{"text", "json"} + for _, v := range tests { + *c.LogFormat = v + if err := c.Validate(); err == nil { + t.Logf("Config passed as expected after setting LogFormat to %q: %v", *c.LogFormat, err) + + } else { + t.Errorf("Config failed, but should have passed on valid value %q for LogFormat: %s", *c.LogFormat, err) + } + } + + // Set back to prior value + *c.LogFormat = tmpLogFormat + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring LogFormat: %s", err) + } else { + t.Log("Validation successful after restoring LogFormat field") + } + }) + + t.Run("LogFilePath set to nil", func(t *testing.T) { + tmpLogFilePath := c.LogFilePath + c.LogFilePath = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil LogFilePath: %s", err) + } else { + t.Logf("Config failed as expected after setting LogFilePath to nil: %s", err) + } + // Set back to prior value + c.LogFilePath = tmpLogFilePath + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring LogFilePath: %s", err) + } else { + t.Log("Validation successful after restoring LogFilePath field") + } + }) + + t.Run("ConsoleOutput set to nil", func(t *testing.T) { + tmpConsoleOutput := c.ConsoleOutput + c.ConsoleOutput = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil ConsoleOutput: %s", err) + } else { + t.Logf("Config failed as expected after setting ConsoleOutput to nil: %s", err) + } + // Set back to prior value + c.ConsoleOutput = tmpConsoleOutput + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring ConsoleOutput: %s", err) + } else { + t.Log("Validation successful after restoring ConsoleOutput field") + } + }) + + t.Run("ConsoleOutput set to invalid value", func(t *testing.T) { + tmpConsoleOutput := *c.ConsoleOutput + *c.ConsoleOutput = fakeValue + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on invalid value %q for ConsoleOutput: %v", *c.ConsoleOutput, err) + } else { + t.Logf("Config failed as expected after setting ConsoleOutput to %q: %v", *c.ConsoleOutput, err) + } + // Set back to prior value + *c.ConsoleOutput = tmpConsoleOutput + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring ConsoleOutput: %s", err) + } else { + t.Log("Validation successful after restoring ConsoleOutput field") + } + }) + + t.Run("ConsoleOutput set to valid values", func(t *testing.T) { + + tmpConsoleOutput := *c.ConsoleOutput + tests := []string{"stdout", "stderr"} + for _, v := range tests { + *c.ConsoleOutput = v + if err := c.Validate(); err == nil { + t.Logf("Config passed as expected after setting ConsoleOutput to %q: %v", *c.ConsoleOutput, err) + + } else { + t.Errorf("Config failed, but should have passed on valid value %q for ConsoleOutput: %s", *c.ConsoleOutput, err) + } + } + + // Set back to prior value + *c.ConsoleOutput = tmpConsoleOutput + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring ConsoleOutput: %s", err) + } else { + t.Log("Validation successful after restoring ConsoleOutput field") + } + }) + + t.Run("LogLevel set to nil", func(t *testing.T) { + tmpLogLevel := c.LogLevel + c.LogLevel = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil LogLevel: %s", err) + } else { + t.Logf("Config failed as expected after setting LogLevel to nil: %s", err) + } + // Set back to prior value + c.LogLevel = tmpLogLevel + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring LogLevel: %s", err) + } else { + t.Log("Validation successful after restoring LogLevel field") + } + }) + + t.Run("LogLevel set to invalid value", func(t *testing.T) { + tmpLogLevel := *c.LogLevel + *c.LogLevel = fakeValue + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on invalid value %q for LogLevel: %v", *c.LogLevel, err) + } else { + t.Logf("Config failed as expected after setting LogLevel to %q: %v", *c.LogLevel, err) + } + // Set back to prior value + *c.LogLevel = tmpLogLevel + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring LogLevel: %s", err) + } else { + t.Log("Validation successful after restoring LogLevel field") + } + }) + + t.Run("LogLevel set to valid values", func(t *testing.T) { + + tmpLogLevel := *c.LogLevel + tests := []string{ + "emergency", + "alert", + "critical", + "panic", + "fatal", + "error", + "warn", + "info", + "notice", + "debug", + "trace", + } + for _, v := range tests { + *c.LogLevel = v + if err := c.Validate(); err == nil { + t.Logf("Config passed as expected after setting LogLevel to %q: %v", *c.LogLevel, err) + + } else { + t.Errorf("Config failed, but should have passed on valid value %q for LogLevel: %s", *c.LogLevel, err) + } + } + + // Set back to prior value + *c.LogLevel = tmpLogLevel + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring LogLevel: %s", err) + } else { + t.Log("Validation successful after restoring LogLevel field") + } + }) + + t.Run("UseSyslog set to nil", func(t *testing.T) { + tmpUseSyslog := c.UseSyslog + c.UseSyslog = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil UseSyslog: %s", err) + } else { + t.Logf("Config failed as expected after setting UseSyslog to nil: %s", err) + } + // Set back to prior value + c.UseSyslog = tmpUseSyslog + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring UseSyslog: %s", err) + } else { + t.Log("Validation successful after restoring UseSyslog field") + } + }) + + t.Run("logger set to nil", func(t *testing.T) { + tmplogger := c.logger + c.logger = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil logger: %s", err) + } else { + t.Logf("Config failed as expected after setting logger to nil: %s", err) + } + // Set back to prior value + c.logger = tmplogger + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring logger: %s", err) + } else { + t.Log("Validation successful after restoring logger field") + } + }) + + t.Run("ConfigFile set to nil", func(t *testing.T) { + tmpConfigFile := c.ConfigFile + c.ConfigFile = nil + if err := c.Validate(); err == nil { + t.Errorf("Config passed, but should have failed on nil ConfigFile: %s", err) + } else { + t.Logf("Config failed as expected after setting ConfigFile to nil: %s", err) + } + // Set back to prior value + c.ConfigFile = tmpConfigFile + + if err := c.Validate(); err != nil { + t.Errorf("Validation failed for config after restoring ConfigFile: %s", err) + } else { + t.Log("Validation successful after restoring ConfigFile field") + } + }) + +} diff --git a/doc.go b/doc.go index adfb6a11..007b9c40 100644 --- a/doc.go +++ b/doc.go @@ -26,9 +26,11 @@ GOTCHAS FEATURES -• Extensive command-line flags with detailed help output - -• (Optional) Use environment variables instead of or in addition to command-line arguments +• Supports multiple (merged) sources for supplying configuration settings + Default settings + TOML format configuration file + Environment variables + Command-line flags (with detailed help output) • Match on specified file patterns @@ -64,27 +66,29 @@ Help output is below. See the README for examples. ELBOW x.y.z https://github.com/atc0005/elbow - Usage: elbow [--pattern PATTERN] [--extensions EXTENSIONS] [--age AGE] --keep KEEP [--keep-old] [--remove] [--ignore-errors] [--log-level LOG-LEVEL] [--log-format LOG-FORMAT] [--log-file LOG-FILE] [--console-output CONSOLE-OUTPUT] [--use-syslog] --paths PATHS [--recurse] + Usage: elbow [--pattern PATTERN] [--extensions EXTENSIONS] [--age AGE] [--keep KEEP] [--keep-old] [--remove] [--ignore-errors] [--log-level LOG-LEVEL] [--log-format LOG-FORMAT] [--log-file LOG-FILE] [--console-output CONSOLE-OUTPUT] [--use-syslog] [--paths PATHS] [--recurse] [--config-file CONFIG-FILE] Options: --pattern PATTERN Substring pattern to compare filenames against. Wildcards are not supported. --extensions EXTENSIONS - Limit search to specified file extensions. Specify as space separated list to match multiple required extensions. - --age AGE Limit search to files that are the specified number of days old or older. [default: 0] - --keep KEEP Keep specified number of matching files per provided path. - --keep-old Keep oldest files instead of newer per provided path. [default: false] - --remove Remove matched files per provided path. [default: false] - --ignore-errors Ignore errors encountered during file removal. [default: false] + Limit search to specified file extensions. Specify as space separated list to match multiple required extensions. + --age AGE Limit search to files that are the specified number of days old or older. + --keep KEEP Keep specified number of matching files per provided path. [default: -1] + --keep-old Keep oldest files instead of newer per provided path. + --remove Remove matched files per provided path. + --ignore-errors Ignore errors encountered during file removal. --log-level LOG-LEVEL - Maximum log level at which messages will be logged. Log messages below this threshold will be discarded. [default: info] + Maximum log level at which messages will be logged. Log messages below this threshold will be discarded. [default: info] --log-format LOG-FORMAT Log formatter used by logging package. [default: text] - --log-file LOG-FILE Optional log file used to hold logged messages. If set, log messages are not displayed on the console. + --log-file LOG-FILE Optional log file used to hold logged messages. If set, log messages are not displayed on the console. --console-output CONSOLE-OUTPUT - Specify how log messages are logged to the console. [default: stdout] - --use-syslog Log messages to syslog in addition to other outputs. Not supported on Windows. [default: false] + Specify how log messages are logged to the console. [default: stdout] + --use-syslog Log messages to syslog in addition to other outputs. Not supported on Windows. --paths PATHS List of comma or space-separated paths to process. - --recurse Perform recursive search into subdirectories per provided path. [default: false] + --recurse Perform recursive search into subdirectories per provided path. + --config-file CONFIG-FILE + Full path to optional TOML-formatted configuration file. See config.example.toml for a starter template. --help, -h display this help and exit --version display version and exit diff --git a/logging/logging.go b/logging/logging.go index 7d7c4352..a242ca33 100644 --- a/logging/logging.go +++ b/logging/logging.go @@ -22,11 +22,20 @@ package logging import ( "fmt" "os" + "runtime" "strings" "github.com/sirupsen/logrus" ) +// Buffer is a package global instance of LogBuffer intended to ease log +// message collection for later emission when all required logger settings +// have been applied. +// +// FIXME: Moved here from Config package What is the best approach to handling +// this instead of using a package global? +var Buffer LogBuffer + // LogRecord holds logrus.Field values along with additional metadata that can be // used later to complete the log message submission process. type LogRecord struct { @@ -44,7 +53,12 @@ func (lb *LogBuffer) Add(r LogRecord) { } // Flush LogRecord entries after applying user-provided logging settings -func (lb LogBuffer) Flush(logger *logrus.Logger) { +func (lb LogBuffer) Flush(logger *logrus.Logger) error { + + // Check for nil *logrus.Logger before attempting to use it. + if logger == nil { + return fmt.Errorf("nil logger received by LogBuffer.Flush()") + } for _, entry := range lb { @@ -72,42 +86,53 @@ func (lb LogBuffer) Flush(logger *logrus.Logger) { logger.WithFields(entry.Fields).Trace(entry.Message) default: - panic("This should not have been reachable") + return fmt.Errorf("unhandled codepath; invalid option provided for entry.Level: %v", entry.Level) } } - // TODO: Empty slice now that we're done processing all items + // Empty slice now that we're done processing all items + // https://yourbasic.org/golang/clear-slice/ + // lb = nil + // FIXME + // ineffectual assignment to `lb` (ineffassign) + // indicate no errors were encountered + return nil } // SetLoggerFormatter sets a user-specified logging format for the provided // logger object. -func SetLoggerFormatter(logger *logrus.Logger, format string) { +func SetLoggerFormatter(logger *logrus.Logger, format string) error { switch format { case "text": logger.SetFormatter(&logrus.TextFormatter{}) case "json": // Log as JSON instead of the default ASCII formatter. logger.SetFormatter(&logrus.JSONFormatter{}) + default: + return fmt.Errorf("invalid option provided: %v", format) } + + return nil } // SetLoggerConsoleOutput configures the chosen console output to one of // stdout or stderr. -func SetLoggerConsoleOutput(logger *logrus.Logger, consoleOutput string) { - var loggerOutput *os.File +func SetLoggerConsoleOutput(logger *logrus.Logger, consoleOutput string) error { + switch { case consoleOutput == "stdout": - loggerOutput = os.Stdout + logger.SetOutput(os.Stdout) case consoleOutput == "stderr": - loggerOutput = os.Stderr + logger.SetOutput(os.Stderr) + default: + return fmt.Errorf("invalid option provided: %v", consoleOutput) } - // Apply chosen output based on earlier checks - // Note: Can be any io.Writer - logger.SetOutput(loggerOutput) + return nil + } // SetLoggerLogFile configures a log file as the destination for all log @@ -136,7 +161,7 @@ func SetLoggerLogFile(logger *logrus.Logger, logFilePath string) (*os.File, erro // SetLoggerLevel applies the requested logger level to filter out messages // with a lower level than the one configured. -func SetLoggerLevel(logger *logrus.Logger, logLevel string) { +func SetLoggerLevel(logger *logrus.Logger, logLevel string) error { // https://godoc.org/github.com/sirupsen/logrus#Level // https://golang.org/pkg/log/syslog/#Priority @@ -156,6 +181,19 @@ func SetLoggerLevel(logger *logrus.Logger, logLevel string) { logger.SetLevel(logrus.DebugLevel) case "trace": logger.SetLevel(logrus.TraceLevel) + default: + return fmt.Errorf("invalid option provided: %v", logLevel) } + // signal that a case was triggered as expected + return nil + +} + +// GetLineNumber is a wrapper around runtime.Caller to return only the current +// line number from the point this function was called. +// TODO: Find a better location for this utility function +func GetLineNumber() int { + _, _, line, _ := runtime.Caller(1) + return line } diff --git a/logging/logging_test.go b/logging/logging_test.go new file mode 100644 index 00000000..725bde41 --- /dev/null +++ b/logging/logging_test.go @@ -0,0 +1,241 @@ +// Copyright 2019 Adam Chalkley +// +// https://github.com/atc0005/elbow +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package logging is intended mostly as a set of helper functions around +// configuring and using a common logger to provide structured, leveled +// logging. +package logging + +import ( + //"io/ioutil" + "fmt" + "testing" + + "github.com/sirupsen/logrus" +) + +// Fix linting error +// string `fakeValue` has 3 occurrences, make it a constant (goconst) +const fakeValue = "fakeValue" + +func TestLogBufferFlushNilLoggerShouldFail(t *testing.T) { + + var nilLogger *logrus.Logger + + var logBuffer LogBuffer + + if err := logBuffer.Flush(nilLogger); err == nil { + t.Error("passed nil *logrus.Logger without error") + } else { + t.Log("received error as expected:", err) + } +} + +func TestLogBufferFlushShouldSucceed(t *testing.T) { + + var testLogBuffer LogBuffer + + logger := logrus.New() + // Configure logger to throw everything away + //logger.SetOutput(ioutil.Discard) + logger.SetLevel(logrus.TraceLevel) + + type test struct { + entryLevel logrus.Level + result error + } + + tests := []test{ + // TODO: Need to add coverage for messages at these log levels: + //test{entryLevel: logrus.PanicLevel, result: nil}, + //test{entryLevel: logrus.FatalLevel, result: nil}, + + test{entryLevel: logrus.ErrorLevel, result: nil}, + test{entryLevel: logrus.WarnLevel, result: nil}, + test{entryLevel: logrus.InfoLevel, result: nil}, + test{entryLevel: logrus.DebugLevel, result: nil}, + test{entryLevel: logrus.TraceLevel, result: nil}, + } + + // Create test log buffer entries + for _, v := range tests { + + testLogBuffer.Add(LogRecord{ + Level: v.entryLevel, + Message: fmt.Sprintf("This is a message at level %v.", v.entryLevel), + }) + } + + // Verify that the number of entries matches up with the same number of + // active test entries + if len(testLogBuffer) != len(tests) { + t.Errorf("Expected %d log buffer entries, Got %d", + len(testLogBuffer), len(tests)) + } else { + t.Log("Number of log buffer entries matches test entries") + } + + if err := testLogBuffer.Flush(logger); err != nil { + t.Error("Failed to flush log entries:", err) + } else { + t.Log("Flushed log buffer entry as expected") + } +} + +func TestGetLineNumber(t *testing.T) { + got := GetLineNumber() + if got < 1 { + t.Errorf("Line number is incorrect, got: %d, want: greater than 0.", got) + } + +} + +func TestSetLoggerLevelShouldFail(t *testing.T) { + + logger := logrus.New() + + give := fakeValue + got := SetLoggerLevel(logger, give) + if got == nil { + t.Error("Expected error for", give, "Got", got) + } else { + t.Logf("Got error as expected for %v: %v", give, got) + } + +} + +// Pass in a valid logLevel string, call logger.GetLevel() +// and compare against the expected value +func TestSetLoggerLevelShouldSucceed(t *testing.T) { + + type test struct { + logLevel string + loggerLevel logrus.Level + } + + tests := []test{ + test{logLevel: "emerg", loggerLevel: logrus.PanicLevel}, + test{logLevel: "panic", loggerLevel: logrus.PanicLevel}, + test{logLevel: "alert", loggerLevel: logrus.FatalLevel}, + test{logLevel: "critical", loggerLevel: logrus.FatalLevel}, + test{logLevel: "fatal", loggerLevel: logrus.FatalLevel}, + test{logLevel: "error", loggerLevel: logrus.ErrorLevel}, + test{logLevel: "warn", loggerLevel: logrus.WarnLevel}, + test{logLevel: "notice", loggerLevel: logrus.WarnLevel}, + test{logLevel: "info", loggerLevel: logrus.InfoLevel}, + test{logLevel: "debug", loggerLevel: logrus.DebugLevel}, + test{logLevel: "trace", loggerLevel: logrus.TraceLevel}, + } + + logger := logrus.New() + + for _, v := range tests { + give := v.logLevel + if err := SetLoggerLevel(logger, give); err != nil { + t.Error("Error when calling SetLoggerLevel(): ", err) + } else { + t.Log("No error when calling SetLoggerLevel()") + } + want := v.loggerLevel + got := logger.GetLevel() + + if got != v.loggerLevel { + t.Error("Expected", want, "Got", got) + t.FailNow() + } else { + t.Log("Got", got, "as expected for requested level of", give) + } + } + +} + +func TestSetLoggerFormatterShouldFail(t *testing.T) { + + logger := logrus.New() + + give := fakeValue + got := SetLoggerFormatter(logger, give) + if got == nil { + t.Error("Expected error for", give, "Got", got) + } else { + t.Logf("Got error as expected for %v: %v", give, got) + } +} + +func TestSetLoggerFormatterShouldSucceed(t *testing.T) { + + type test struct { + format string + result error + } + + logger := logrus.New() + + tests := []test{ + test{format: "text", result: nil}, + test{format: "json", result: nil}, + } + + for _, give := range tests { + got := SetLoggerFormatter(logger, give.format) + if got != give.result { + t.Error("Expected", give.result, "Got", got) + } + } + +} + +func TestSetLoggerConsoleOutputShouldFail(t *testing.T) { + + logger := logrus.New() + + give := fakeValue + got := SetLoggerConsoleOutput(logger, give) + if got == nil { + t.Error("Expected error for", give, "Got", got) + } else { + t.Logf("Got error as expected for %v: %v", give, got) + } +} + +func TestSetLoggerConsoleOutputShouldSucceed(t *testing.T) { + + type test struct { + consoleOutput string + result error + } + + logger := logrus.New() + + tests := []test{ + test{consoleOutput: "stdout", result: nil}, + test{consoleOutput: "stderr", result: nil}, + } + + for _, give := range tests { + got := SetLoggerConsoleOutput(logger, give.consoleOutput) + if got != give.result { + t.Error("Expected", give.result, "Got", got) + } + } + +} + +func TestEnableSyslogLogging(t *testing.T) { + // TODO: Need to implement this + + t.Log("TODO: Need to implement this test.") +} diff --git a/logging/logging_unix.go b/logging/logging_unix.go index ffebc13b..9092903e 100644 --- a/logging/logging_unix.go +++ b/logging/logging_unix.go @@ -82,6 +82,8 @@ func EnableSyslogLogging(logger *logrus.Logger, logBuffer *LogBuffer, logLevel s // syslog: N/A // logrus: Finer-grained informational events than debug. syslogLogLevel = syslog.LOG_DEBUG + default: + return fmt.Errorf("invalid syslog log level: %q", logLevel) } logBuffer.Add(LogRecord{ @@ -98,12 +100,7 @@ func EnableSyslogLogging(logger *logrus.Logger, logBuffer *LogBuffer, logLevel s hook, err := lSyslog.NewSyslogHook("", "", syslogLogLevel, "") if err == nil { - // https://github.com/sirupsen/logrus#hooks - // https://github.com/sirupsen/logrus/blob/master/hooks/syslog/README.md - // Seems to require `log.AddHook(hook)`` vs `log.Hooks.Add(hook)` - // FIXME: Confirm that we can use Record{} without specifying the - // `Fields` struct file key/value pair. logBuffer.Add(LogRecord{ Level: logrus.InfoLevel, Message: "Connected to syslog socket", diff --git a/main.go b/main.go index 118b0353..ee239d27 100644 --- a/main.go +++ b/main.go @@ -34,68 +34,63 @@ import ( // build by our Makefile var version = "x.y.z" +// TODO: Move this elsewhere to a dedicated location. +// TODO: Create a metadata subpackage? +var defaultAppName = "Elbow" + func main() { - // Checked at the end to determine if any issues were encountered during - // app run. There is likely a much better way to handle this + // Checked at the end to determine if any non-fatal issues were + // encountered during app run problemsEncountered := false - appName := "Elbow" - appDescription := "prunes content matching specific patterns, either in a single directory or recursively through a directory tree." - appURL := "https://github.com/atc0005/elbow" - - //log.Debug("Constructing config object") - - // If this fails, the application will immediately exit. - appConfig := config.NewConfig(appName, appDescription, appURL, version) - // defaultConfig := config.NewConfig(appName, appDescription, appURL, version) - - // Validate configuration - if ok, err := appConfig.Validate(); !ok { + // If this fails, the application should immediately exit. + appConfig, err := config.NewConfig(version) + if err != nil { // NOTE: We're not using `log` here as the user-specified // configuration could be too botched to use reliably. - // Provide user with error and valid usage details - fmt.Printf("\nERROR: configuration validation failed\n%s\n\n", err) - appConfig.FlagParser.WriteUsage(os.Stdout) + // TODO: Any point in setting this? Perhaps wire it into a function + // that handles setting the flag and other useful spindown tasks? problemsEncountered = true - os.Exit(1) - // failMessage := fmt.Sprint("configuration validation failed: ", err) - // appConfig.FlagParser.Fail(failMessage) + // Provide user with error and valid usage details + fmt.Printf("Failed to process configuration:\n%s\n\n", err) + config.WriteDefaultHelpText(defaultAppName) + os.Exit(1) } - log := appConfig.Logger + log := appConfig.GetLogger() log.Debug("Config object created") // https://www.joeshaw.org/dont-defer-close-on-writable-files/ - if appConfig.LogFileHandle != nil { + if appConfig.GetLogFileHandle() != nil { log.Debug("Deferring closure of log file") - defer appConfig.LogFileHandle.Close() + defer appConfig.GetLogFileHandle().Close() } log.WithFields(logrus.Fields{ - "paths": appConfig.Paths, - "file_pattern": appConfig.FilePattern, - "extensions": appConfig.FileExtensions, - "file_age": appConfig.FileAge, + "paths": appConfig.GetPaths(), + "file_pattern": appConfig.GetFilePattern(), + "extensions": appConfig.GetFileExtensions(), + "file_age": appConfig.GetFileAge(), }).Info("Starting evaluation of paths list") // Used as a global counter/bucket for presentation/logging purposes var appResults paths.ProcessingResults var pass int - var totalPaths int = len(appConfig.Paths) - for _, path := range appConfig.Paths { + var totalPaths int = len(appConfig.GetPaths()) + for _, path := range appConfig.GetPaths() { pass++ log.WithFields(logrus.Fields{ "total_paths": totalPaths, "iteration": pass, - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), }).Infof("Beginning processing of path %q (%d of %d)", path, pass, totalPaths) @@ -105,17 +100,17 @@ func main() { problemsEncountered = true log.WithFields(logrus.Fields{ - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), }).Errorf("Requested path not found: %q", path) - if appConfig.IgnoreErrors { + if appConfig.GetIgnoreErrors() { log.WithFields(logrus.Fields{ - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), }).Warn("Error encountered, but continuing as requested.") continue } else { log.WithFields(logrus.Fields{ - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), }).Warn("Error encountered and option to ignore errors not set. Exiting") os.Exit(1) @@ -128,13 +123,13 @@ func main() { problemsEncountered = true log.WithFields(logrus.Fields{ - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), "iteration": pass, }).Error("error:", err) - if !appConfig.IgnoreErrors { + if !appConfig.GetIgnoreErrors() { log.WithFields(logrus.Fields{ - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), }).Warn("Error encountered and option to ignore errors not set. Exiting") } log.Warn("Error encountered, but continuing as requested.") @@ -147,16 +142,16 @@ func main() { log.WithFields(logrus.Fields{ "path": path, - "file_pattern": appConfig.FilePattern, - "extensions": appConfig.FileExtensions, - "file_age": appConfig.FileAge, + "file_pattern": appConfig.GetFilePattern(), + "extensions": appConfig.GetFileExtensions(), + "file_age": appConfig.GetFileAge(), "iteration": pass, }).Info("No matches found") log.WithFields(logrus.Fields{ "total_paths": totalPaths, "iteration": pass, - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), }).Infof("Ending processing of path %q (%d of %d)", path, pass, totalPaths) if pass < totalPaths { @@ -168,9 +163,9 @@ func main() { log.WithFields(logrus.Fields{ "path": path, - "file_pattern": appConfig.FilePattern, - "extensions": appConfig.FileExtensions, - "file_age": appConfig.FileAge, + "file_pattern": appConfig.GetFilePattern(), + "extensions": appConfig.GetFileExtensions(), + "file_age": appConfig.GetFileAge(), "total_file_size": fileMatches.TotalFileSize(), "iteration": pass, }).Infof("%d files eligible for removal (%s)", @@ -181,9 +176,9 @@ func main() { appResults.EligibleFileSize += fileMatches.TotalFileSize() log.WithFields(logrus.Fields{ - "keep_oldest": appConfig.KeepOldest, + "keep_oldest": appConfig.GetKeepOldest(), "iteration": pass, - }).Infof("%d files to keep as requested", appConfig.NumFilesToKeep) + }).Infof("%d files to keep as requested", appConfig.GetNumFilesToKeep()) filesToPrune := fileMatches.FilesToPrune(appConfig) @@ -192,7 +187,7 @@ func main() { log.WithFields(logrus.Fields{ "total_paths": totalPaths, "iteration": pass, - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), }).Infof("Ending processing of path %q (%d of %d)", path, pass, totalPaths) if pass < totalPaths { @@ -206,7 +201,7 @@ func main() { "total_file_size": filesToPrune.TotalFileSizeHR(), "iteration": pass, }).Debug("Calling cleanPath") - log.Infof("Ignoring file removal errors: %t", appConfig.IgnoreErrors) + log.Infof("Ignoring file removal errors: %t", appConfig.GetIgnoreErrors()) removalResults, err := paths.CleanPath(filesToPrune, appConfig) appResults.SuccessRemoved += len(removalResults.SuccessfulRemovals) @@ -242,11 +237,12 @@ func main() { log.Warnf("Error encountered while processing %s: %s", path, err) - if !appConfig.IgnoreErrors { + if !appConfig.GetIgnoreErrors() { log.WithFields(logrus.Fields{ - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), "iteration": pass, }).Warn("Error encountered and option to ignore errors not set. Exiting") + os.Exit(1) } log.Warn("Error encountered, but continuing as requested.") continue @@ -255,7 +251,7 @@ func main() { log.WithFields(logrus.Fields{ "total_paths": totalPaths, "iteration": pass, - "ignore_errors": appConfig.IgnoreErrors, + "ignore_errors": appConfig.GetIgnoreErrors(), }).Infof("Ending processing of path %q (%d of %d)", path, pass, totalPaths) @@ -277,9 +273,9 @@ func main() { }) if problemsEncountered { - summaryLogger.Warnf("%s completed, but issues were encountered.", appConfig.AppName) + summaryLogger.Warnf("%s completed, but issues were encountered.", appConfig.GetAppName()) } else { - summaryLogger.Infof("%s successfully completed.", appConfig.AppName) + summaryLogger.Infof("%s successfully completed.", appConfig.GetAppName()) } } diff --git a/main_test.go b/main_test.go index 15cb84d3..da45f346 100644 --- a/main_test.go +++ b/main_test.go @@ -17,6 +17,8 @@ package main import ( + "os" + "runtime" "testing" "github.com/atc0005/elbow/config" @@ -24,27 +26,40 @@ import ( func TestMain(t *testing.T) { - appName := "Elbow" - appDescription := "prunes content matching specific patterns, either in a single directory or recursively through a directory tree." - appURL := "https://github.com/atc0005/elbow" + // https://stackoverflow.com/questions/33723300/how-to-test-the-passing-of-arguments-in-golang - defaultConfig := config.NewConfig(appName, appDescription, appURL, version) + // Save old command-line arguments so that we can restore them later + oldArgs := os.Args - var emptySlice = []string{} - var nilSlice []string + // Defer restoring original command-line arguments + defer func() { os.Args = oldArgs }() - t.Logf("%v\n", emptySlice) - t.Log(len(emptySlice)) - t.Log("emptySlice is nil:", emptySlice == nil) - t.Log("-------------------------") + // TODO: A useful way to automate retrieving the app name? + appName := "elbow" + if runtime.GOOS == "windows" { + appName += ".exe" + } - t.Logf("%v\n", nilSlice) - t.Log(len(nilSlice)) - t.Log("nilSlice is nil:", nilSlice == nil) - t.Log("-------------------------") + // Note to self: Don't add/escape double-quotes here. The shell strips + // them away and the application never sees them. + os.Args = []string{ + appName, + "--paths", "/tmp/elbow/path1", + "--keep", "1", + "--recurse", + "--keep-old", + "--log-level", "info", + "--use-syslog", + "--log-format", "text", + "--console-output", "stdout", + } - t.Logf("%v\n", defaultConfig.FileExtensions) - t.Log(len(defaultConfig.FileExtensions)) - t.Log("defaultConfig.FileExtensions is nil:", defaultConfig.FileExtensions == nil) + // TODO: Flesh this out + _, err := config.NewConfig(version) + if err != nil { + t.Errorf("Error encountered when instantiating configuration: %s", err) + } else { + t.Log("No errors encountered when instantiating configuration") + } } diff --git a/matches/matches.go b/matches/matches.go index 857ce597..8924b6fe 100644 --- a/matches/matches.go +++ b/matches/matches.go @@ -71,26 +71,26 @@ func (fm FileMatch) SizeHR() string { // HasMatchingExtension validates whether a file has the desired extension func HasMatchingExtension(filename string, config *config.Config) bool { - log := config.Logger + log := config.GetLogger() // NOTE: We do NOT compare extensions insensitively. We can add that // functionality in the future if needed. ext := filepath.Ext(filename) - if len(config.FileExtensions) == 0 { + if len(config.GetFileExtensions()) == 0 { log.Debug("No extension limits have been set!") log.Debugf("Considering %s safe for removal\n", filename) return true } - if InList(ext, config.FileExtensions) { + if InList(ext, config.GetFileExtensions()) { log.Debugf("%s has a valid extension for removal\n", filename) return true } log.Debug("HasMatchingExtension: returning false for:", filename) log.Debugf("HasMatchingExtension: returning false (%q not in %q)", - ext, config.FileExtensions) + ext, config.GetFileExtensions()) return false } @@ -98,32 +98,32 @@ func HasMatchingExtension(filename string, config *config.Config) bool { // pattern func HasMatchingFilenamePattern(filename string, config *config.Config) bool { - log := config.Logger + log := config.GetLogger() - if strings.TrimSpace(config.FilePattern) == "" { + if strings.TrimSpace(config.GetFilePattern()) == "" { log.Debug("No FilePattern has been specified!") log.Debugf("Considering %s safe for removal\n", filename) return true } // Search for substring - if strings.Contains(filename, config.FilePattern) { + if strings.Contains(filename, config.GetFilePattern()) { log.Debug("HasMatchingFilenamePattern: returning true for:", filename) log.Debugf("HasMatchingFilenamePattern: returning true (%q contains %q)", - filename, config.FilePattern) + filename, config.GetFilePattern()) return true } log.Debug("HasMatchingFilenamePattern: returning false for:", filename) log.Debugf("HasMatchingFilenamePattern: returning false (%q does not contain %q)", - filename, config.FilePattern) + filename, config.GetFilePattern()) return false } // HasMatchingAge validates whether a file matches the desired age threshold func HasMatchingAge(file os.FileInfo, config *config.Config) bool { - log := config.Logger + log := config.GetLogger() // used by this function's context logger and for return code var ageCheckResults bool @@ -135,18 +135,18 @@ func HasMatchingAge(file os.FileInfo, config *config.Config) bool { contextLogger := log.WithFields(logrus.Fields{ "file_mod_time": fileModTime.Format(time.RFC3339), "current_time": now.Format(time.RFC3339), - "file_age_flag": config.FileAge, + "file_age_flag": config.GetFileAge(), "filename": file.Name(), }) // The default for this flag is 0, so only a positive, non-zero number // is considered for use with age matching. - if config.FileAge > 0 { + if config.GetFileAge() > 0 { // Flip user specified number of days negative so that we can wind // back that many days from the file modification time. This gives // us our threshold to compare file modification times against. - daysBack := -(config.FileAge) + daysBack := -(config.GetFileAge()) fileAgeThreshold := now.AddDate(0, 0, daysBack) // Bundle more fields now that we have access to the data @@ -222,32 +222,32 @@ func (fm FileMatches) SortByModTimeDesc() { // object settings. func (fm FileMatches) FilesToPrune(c *config.Config) FileMatches { - log := c.Logger + log := c.GetLogger() var pruneStartRange int var pruneEndRange int switch { - case c.NumFilesToKeep > len(fm): + case c.GetNumFilesToKeep() > len(fm): log.Debug("Specified number to keep is larger than total matches; will process all matches") pruneStartRange = 0 pruneEndRange = len(fm) - case c.KeepOldest: + case c.GetKeepOldest(): fm.SortByModTimeAsc() log.Debug("Keeping older files by sorting in ascending order") pruneStartRange = 0 - pruneEndRange = (len(fm) - c.NumFilesToKeep) - case !c.KeepOldest: + pruneEndRange = (len(fm) - c.GetNumFilesToKeep()) + case !c.GetKeepOldest(): fm.SortByModTimeDesc() log.Debug("Keeping newer files by sorting in descending order") pruneStartRange = 0 - pruneEndRange = (len(fm) - c.NumFilesToKeep) + pruneEndRange = (len(fm) - c.GetNumFilesToKeep()) } log.WithFields(logrus.Fields{ "start_range": pruneStartRange, "end_range": pruneEndRange, - "num_to_keep": c.NumFilesToKeep, + "num_to_keep": c.GetNumFilesToKeep(), }).Debug("Building list of files to prune by skipping forward specified number of files to keep") return fm[pruneStartRange:pruneEndRange] diff --git a/paths/paths.go b/paths/paths.go index 875a0b89..8209a6b2 100644 --- a/paths/paths.go +++ b/paths/paths.go @@ -74,7 +74,7 @@ type PathPruningResults struct { // code (nil if no errors were encountered). func CleanPath(files matches.FileMatches, config *config.Config) (PathPruningResults, error) { - log := config.Logger + log := config.GetLogger() for _, file := range files { log.WithFields(logrus.Fields{ @@ -82,13 +82,13 @@ func CleanPath(files matches.FileMatches, config *config.Config) (PathPruningRes "shortpath": file.Name(), "size": file.Size(), "modified": file.ModTime().Format("2006-01-02 15:04:05"), - "removal_enabled": config.Remove, + "removal_enabled": config.GetRemove(), }).Debug("Matching file") } var removalResults PathPruningResults - if !config.Remove { + if !config.GetRemove() { log.Info("File removal not enabled, not removing files") @@ -100,7 +100,7 @@ func CleanPath(files matches.FileMatches, config *config.Config) (PathPruningRes for _, file := range files { log.WithFields(logrus.Fields{ - "removal_enabled": config.Remove, + "removal_enabled": config.GetRemove(), // fully-qualified path to the file "file": file.Path, @@ -121,7 +121,7 @@ func CleanPath(files matches.FileMatches, config *config.Config) (PathPruningRes removalResults.FailedRemovals = append(removalResults.FailedRemovals, file) // Confirm that we should ignore errors (likely enabled) - if !config.IgnoreErrors { + if !config.GetIgnoreErrors() { remainingFiles := len(files) - len(removalResults.FailedRemovals) - len(removalResults.SuccessfulRemovals) log.Debugf("Abandoning removal of %d remaining files", remainingFiles) break @@ -140,9 +140,10 @@ func CleanPath(files matches.FileMatches, config *config.Config) (PathPruningRes } // PathExists confirms that the specified path exists +// FIXME: Update this to break reliance on config.Config; use bare args func PathExists(path string, config *config.Config) bool { - log := config.Logger + log := config.GetLogger() // Make sure path isn't empty if strings.TrimSpace(path) == "" { @@ -166,16 +167,16 @@ func PathExists(path string, config *config.Config) bool { // returns a slice of FileMatch objects func ProcessPath(config *config.Config, path string) (matches.FileMatches, error) { - log := config.Logger + log := config.GetLogger() var fileMatches matches.FileMatches var err error log.WithFields(logrus.Fields{ - "recursive_search": config.RecursiveSearch, - }).Debugf("Recursive search: %t", config.RecursiveSearch) + "recursive_search": config.GetRecursiveSearch(), + }).Debugf("Recursive search: %t", config.GetRecursiveSearch()) - if config.RecursiveSearch { + if config.GetRecursiveSearch() { // Walk walks the file tree rooted at root, calling the anonymous function // for each file or directory in the tree, including root. All errors that