From ea86eae5fe07d1dc3c129c76c551fca74c5ed9c7 Mon Sep 17 00:00:00 2001 From: jmeridth Date: Wed, 15 Jan 2025 00:38:53 -0600 Subject: [PATCH] feat: designate output type - [x] add json tags to all structs so serialization is "automated" - [x] "simplify" output logic - [x] use hclog formatting correctly: message + key/value pairs (no string concatenation) - [x] add tests on output logic - [x] fix config trace output (write-directory and invasive were reversed) Signed-off-by: jmeridth --- config/config.go | 50 ++++++++++++++++++------- config/config_test.go | 27 ++++++++++++++ pluginkit/armory.go | 14 +++---- pluginkit/change.go | 10 ++--- pluginkit/test.go | 12 +++--- pluginkit/test_set.go | 14 +++---- pluginkit/test_suite.go | 81 +++++++++++++++++------------------------ pluginkit/vessel.go | 17 +++++---- 8 files changed, 131 insertions(+), 94 deletions(-) diff --git a/config/config.go b/config/config.go index 0009e81..82a0758 100644 --- a/config/config.go +++ b/config/config.go @@ -15,11 +15,14 @@ import ( "github.com/spf13/viper" ) +var allowedOutputTypes = map[string]string{"json": "", "yaml": ""} + type Config struct { ServiceName string // Must be unique in the config file or logs will be overwritten LogLevel string Logger hclog.Logger Write bool + Output string WriteDirectory string Invasive bool TestSuites []string @@ -33,6 +36,7 @@ func NewConfig(requiredVars []string) Config { topInvasive := viper.GetBool("invasive") writeDir := viper.GetString("write-directory") write := viper.GetBool("write") + output := strings.ToLower(strings.TrimSpace(viper.GetString("output"))) loglevel := viper.GetString(fmt.Sprintf("services.%s.loglevel", serviceName)) invasive := viper.GetBool(fmt.Sprintf("services.%s.invasive", serviceName)) @@ -69,6 +73,12 @@ func NewConfig(requiredVars []string) Config { errString = fmt.Sprintf("missing required variables: %v", missingVars) } + if output == "" { + output = "yaml" + } else if _, ok := allowedOutputTypes[output]; !ok { + errString = "bad output type, allowed output types are json or yaml" + } + var err error if errString != "" { err = errors.New(errString) @@ -79,6 +89,7 @@ func NewConfig(requiredVars []string) Config { LogLevel: loglevel, WriteDirectory: writeDir, Write: write, + Output: output, Invasive: invasive, TestSuites: testSuites, Vars: vars, @@ -87,14 +98,16 @@ func NewConfig(requiredVars []string) Config { if serviceName == "" { serviceName = "overview" } - config.SetConfig(serviceName, false) + + config.SetConfig(serviceName, output == "json") config.Logger.Trace(fmt.Sprintf("Creating a new config instance for service '%s'%v", serviceName, config)) config.Logger.Trace(fmt.Sprintf("loglevel: %s", loglevel)) - config.Logger.Trace(fmt.Sprintf("write-directory: %v", invasive)) - config.Logger.Trace(fmt.Sprintf("invasive: %v", writeDir)) + config.Logger.Trace(fmt.Sprintf("write-directory: %v", writeDir)) + config.Logger.Trace(fmt.Sprintf("write: %v", write)) + config.Logger.Trace(fmt.Sprintf("invasive: %v", invasive)) config.Logger.Trace(fmt.Sprintf("test-suites: %v", testSuites)) config.Logger.Trace(fmt.Sprintf("vars: %v", vars)) - config.Logger.Trace(fmt.Sprintf("writing output to file: %v", write)) + config.Logger.Trace(fmt.Sprintf("output type: %v", output)) return config } @@ -119,11 +132,26 @@ func (c *Config) SetConfig(name string, jsonFormat bool) { logFilePath = path.Join(c.WriteDirectory, name, logFile) } + writer := io.Writer(os.Stdout) + if c.Write { + writer = c.setupLogginFilesAndDirectories(logFilePath) + } + + logger := hclog.New(&hclog.LoggerOptions{ + Level: hclog.LevelFromString(c.LogLevel), + JSONFormat: jsonFormat, + Output: writer, + }) + log.SetOutput(logger.StandardWriter(&hclog.StandardLoggerOptions{InferLevels: false, InferLevelsWithTimestamp: false})) + c.Logger = logger +} + +func (c *Config) setupLogginFilesAndDirectories(logFilePath string) io.Writer { // Create log file and directory if it doesn't exist if _, err := os.Stat(logFilePath); os.IsNotExist(err) { // mkdir all directories from filepath - os.MkdirAll(path.Dir(logFilePath), os.ModePerm) - os.Create(logFilePath) + _ = os.MkdirAll(path.Dir(logFilePath), os.ModePerm) + _, _ = os.Create(logFilePath) } logFileObj, err := os.OpenFile(logFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0640) @@ -131,13 +159,7 @@ func (c *Config) SetConfig(name string, jsonFormat bool) { if err != nil { log.Panic(err) // TODO: handle this error better } - writer := io.MultiWriter(logFileObj, os.Stdout) - logger := hclog.New(&hclog.LoggerOptions{ - Level: hclog.LevelFromString(c.LogLevel), - JSONFormat: jsonFormat, - Output: writer, - }) - log.SetOutput(logger.StandardWriter(&hclog.StandardLoggerOptions{InferLevels: false, InferLevelsWithTimestamp: false})) - c.Logger = logger + writer := io.MultiWriter(logFileObj, os.Stdout) + return writer } diff --git a/config/config_test.go b/config/config_test.go index b5d5830..9594423 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -16,6 +16,8 @@ var testConfigs = []struct { missingVars []string config string logLevelExpected string + output string + outputExpected string invasiveSet bool writeDirSet bool expectedError string @@ -210,6 +212,27 @@ services: my-service-1: vars: key: value +`}, { + testName: "Good - Default YAML output when missing", + runningService: "my-service-1", + requiredVars: []string{}, + outputExpected: "yaml", + config: ` +services: + my-service-1: + test-suites: + - tlp_green +`}, { + testName: "Bad - Bad output type", + runningService: "my-service-1", + requiredVars: []string{}, + expectedError: "bad output type, allowed output types are json or yaml", + config: ` +output: bad +services: + my-service-1: + test-suites: + - tlp_green `}, } @@ -261,6 +284,10 @@ func TestNewConfig(t *testing.T) { if len(config.TestSuites) == 0 { t.Errorf("expected testSuites to be set") } + + if tt.outputExpected != "" && config.Output != tt.outputExpected { + t.Errorf("expected output to be '%s', but got '%s'", tt.outputExpected, config.Output) + } }) } } diff --git a/pluginkit/armory.go b/pluginkit/armory.go index fbdeb69..1ece7c8 100644 --- a/pluginkit/armory.go +++ b/pluginkit/armory.go @@ -6,11 +6,11 @@ import ( ) type Armory struct { - PluginName string - ServiceTarget string - Config *config.Config - Logger hclog.Logger - TestSuites map[string][]TestSet - StartupFunc func() error - CleanupFunc func() error + PluginName string `json:"pluginName"` // PluginName is the name of the plugin + ServiceTarget string `json:"serviceTarget"` // ServiceTarget is the name of the service the plugin is running on + Config *config.Config `json:"config"` // Config is the global configuration for the plugin + Logger hclog.Logger `json:"logger"` // Logger is the global logger for the plugin + TestSuites map[string][]TestSet `json:"testSuites"` // TestSuites is a map of testSuite names to their testSets + StartupFunc func() error `json:"startupFunc"` // StartupFunc is a function to run before the testSets + CleanupFunc func() error `json:"cleanupFunc"` // CleanupFunc is a function to run after the testSets } diff --git a/pluginkit/change.go b/pluginkit/change.go index 6fb1e97..ce510e7 100644 --- a/pluginkit/change.go +++ b/pluginkit/change.go @@ -7,13 +7,13 @@ import ( // Change is a struct that contains the data and functions associated with a single change type Change struct { - TargetName string // TargetName is the name of the resource or configuration that was changed - TargetObject interface{} // TargetObject is the object that was changed + TargetName string `json:"targetName"` // TargetName is the name of the resource or configuration that was changed + TargetObject interface{} `json:"targetObject"` // TargetObject is the object that was changed + Applied bool `json:"applied"` // Applied is true if the change was successfully applied at least once + Reverted bool `json:"reverted"` // Reverted is true if the change was successfully reverted and not applied again + Error error `json:"error"` // Error is used if an error occurred during the change applyFunc func() (interface{}, error) // Apply is the function that can be executed to make the change revertFunc func() error // Revert is the function that can be executed to revert the change - Applied bool // Applied is true if the change was successfully applied at least once - Reverted bool // Reverted is true if the change was successfully reverted and not applied again - Error error // Error is used if an error occurred during the change } // NewChange creates a new Change struct with the provided data diff --git a/pluginkit/test.go b/pluginkit/test.go index 889f906..b9dd039 100644 --- a/pluginkit/test.go +++ b/pluginkit/test.go @@ -4,12 +4,12 @@ type Test func() *TestResult // TestResult is a struct that contains the results of a single step within a testSet type TestResult struct { - Passed bool // Passed is true if the test passed - Description string // Description is a human-readable description of the test - Message string // Message is a human-readable description of the test result - Function string // Function is the name of the code that was executed - Value interface{} // Value is the object that was returned during the test - Changes map[string]*Change // Changes is a slice of changes that were made during the test + Passed bool `json:"passed"` // Passed is true if the test passed + Description string `json:"description"` // Description is a human-readable description of the test + Message string `json:"message"` // Message is a human-readable description of the test result + Function string `json:"functin"` // Function is the name of the code that was executed + Value interface{} `json:"value"` // Value is the object that was returned during the test + Changes map[string]*Change `json:"changes"` // Changes is a slice of changes that were made during the test } func (t *TestResult) Pass(message string, value interface{}) { diff --git a/pluginkit/test_set.go b/pluginkit/test_set.go index 314bd64..bf43001 100644 --- a/pluginkit/test_set.go +++ b/pluginkit/test_set.go @@ -13,13 +13,13 @@ type TestSet func() (testSetName string, result TestSetResult) // TestSetResult is a struct that contains the results of a check for a single control type TestSetResult struct { - Passed bool // Passed is true if the test passed - Description string // Description is a human-readable description of the test - Message string // Message is a human-readable description of the test result - DocsURL string // DocsURL is a link to the documentation for the test - ControlID string // ControlID is the ID of the control that the test is validating - Tests map[string]TestResult // Tests is a list of functions that were executed during the test - BadStateAlert bool // BadStateAlert is true if any change failed to revert at the end of the testSet + Passed bool `json:"passed"` // Passed is true if the test passed + Description string `json:"description"` // Description is a human-readable description of the test + Message string `json:"message"` // Message is a human-readable description of the test result + DocsURL string `json:"docsURL"` // DocsURL is a link to the documentation for the test + ControlID string `json:"controlID"` // ControlID is the ID of the control that the test is validating + Tests map[string]TestResult `json:"tests"` // Tests is a list of functions that were executed during the test + BadStateAlert bool `json:"badStateAlert"` // BadStateAlert is true if any change failed to revert at the end of the testSet invasivePlugin bool // invasivePlugin is true if the testSuite is allowed to make changes to the target service } diff --git a/pluginkit/test_suite.go b/pluginkit/test_suite.go index 615e432..98c8ea6 100644 --- a/pluginkit/test_suite.go +++ b/pluginkit/test_suite.go @@ -19,12 +19,12 @@ import ( // TestSuite is a struct that contains the results of all testSets, orgainzed by name type TestSuite struct { - TestSuiteName string // TestSuiteName is the name of the TestSuite - StartTime string // StartTime is the time the plugin started - EndTime string // EndTime is the time the plugin ended - TestSetResults map[string]TestSetResult // TestSetResults is a map of testSet names to their results - Passed bool // Passed is true if all testSets in the testSuite passed - BadStateAlert bool // BadState is true if any testSet failed to revert at the end of the testSuite + TestSuiteName string `json:"testSuiteName"` // TestSuiteName is the name of the TestSuite + StartTime string `json:"startTime"` // StartTime is the time the plugin started + EndTime string `json:"endTime"` // EndTime is the time the plugin ended + TestSetResults map[string]TestSetResult `json:"testSetResults"` // TestSetResults is a map of testSet names to their results + Passed bool `json:"passed"` // Passed is true if all testSets in the testSuite passed + BadStateAlert bool `json:"badStateAlert"` // BadState is true if any testSet failed to revert at the end of the testSuite config *config.Config // config is the global configuration for the plugin testSets []TestSet // testSets is a list of testSet functions for the current testSuite @@ -95,35 +95,26 @@ func (t *TestSuite) AddTestSetResult(name string, result TestSetResult) { t.TestSetResults[name] = result } -// WriteTestSetResultsJSON unmarhals the TestSuite into a JSON file in the user's WriteDirectory -func (t *TestSuite) WriteTestSetResultsJSON() error { - // Log an error if PluginName was not provided - if t.TestSuiteName == "" { - return errors.New("TestSuite name was not provided before attempting to write results") - } - filepath := path.Join(t.config.WriteDirectory, t.TestSuiteName, "results.json") - - // Create log file and directory if it doesn't exist - if _, err := os.Stat(filepath); os.IsNotExist(err) { - os.MkdirAll(t.config.WriteDirectory, os.ModePerm) - os.Create(filepath) +func (t *TestSuite) WriteTestSetResults(serviceName string, output string) error { + if t.TestSuiteName == "" || serviceName == "" { + return fmt.Errorf("testSuite name and service name must be provided before attempting to write results: testSuite='%s' service='%s'", t.TestSuiteName, serviceName) } - // Write results to file - file, err := os.OpenFile(filepath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0640) - if err != nil { - return err + var err error + var result []byte + switch output { + case "json": + result, err = json.Marshal(t) + case "yaml": + result, err = yaml.Marshal(t) + default: + err = fmt.Errorf("output type '%s' is not supported. Supported types are 'json' and 'yaml'", output) } - defer file.Close() - - // Marshal results to JSON - json, err := json.Marshal(t) if err != nil { return err } - // Write JSON to file - _, err = file.Write(json) + err = t.writeTestSetResultsToFile(serviceName, result, output) if err != nil { return err } @@ -131,40 +122,36 @@ func (t *TestSuite) WriteTestSetResultsJSON() error { return nil } -// WriteTestSetResultsYAML unmarhals the TestSuite into a YAML file in the user's WriteDirectory -func (t *TestSuite) WriteTestSetResultsYAML(serviceName string) error { - // Log an error if PluginName was not provided - if t.TestSuiteName == "" || serviceName == "" { - return fmt.Errorf("testSuite name and service name must be provided before attempting to write results: testSuite='%s' service='%s'", t.TestSuiteName, serviceName) - } +func (t *TestSuite) writeTestSetResultsToFile(serviceName string, result []byte, extension string) error { dir := path.Join(t.config.WriteDirectory, serviceName) - filepath := path.Join(dir, t.TestSuiteName+".yml") + filepath := path.Join(dir, t.TestSuiteName+extension) - t.config.Logger.Trace(fmt.Sprintf("Writing results to %s", filepath)) + t.config.Logger.Trace("Writing results", "filepath", filepath) // Create log file and directory if it doesn't exist if _, err := os.Stat(dir); os.IsNotExist(err) { - os.MkdirAll(dir, os.ModePerm) - t.config.Logger.Error("write directory for this plugin created for results, but should have been created when initializing logs:" + dir) + err = os.MkdirAll(dir, os.ModePerm) + if err != nil { + t.config.Logger.Error("Error creating directory", "directory", dir) + return err + } + t.config.Logger.Info("write directory for this plugin created for results, but should have been created when initializing logs", "directory", dir) } - os.Create(filepath) - - // Write results to file - file, err := os.OpenFile(filepath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0640) + _, err := os.Create(filepath) if err != nil { + t.config.Logger.Error("Error creating file", "filepath", filepath) return err } - defer file.Close() - // Marshal results to YAML - yaml, err := yaml.Marshal(t) + file, err := os.OpenFile(filepath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0640) if err != nil { + t.config.Logger.Error("Error opening file", "filepath", filepath) return err } + defer file.Close() - // Write YAML to file - _, err = file.Write(yaml) + _, err = file.Write(result) if err != nil { return err } diff --git a/pluginkit/vessel.go b/pluginkit/vessel.go index f1aae09..a9af466 100644 --- a/pluginkit/vessel.go +++ b/pluginkit/vessel.go @@ -10,12 +10,12 @@ import ( // The vessel gets the armory in position to execute the testSets specified in the testSuites type Vessel struct { - ServiceName string - PluginName string - RequiredVars []string - Armory *Armory - TestSuites []TestSuite - Initializer func(*config.Config) error + ServiceName string `json:"serviceName"` + PluginName string `json:"pluginName"` + RequiredVars []string `json:"requiredVars"` + Armory *Armory `json:"armory"` + TestSuites []TestSuite `json:"testSuites"` + Initializer func(*config.Config) error `json:"initializer"` config *config.Config logger hclog.Logger executedTestSets *[]string @@ -93,12 +93,13 @@ func (v *Vessel) Mobilize() (err error) { } v.config.Logger.Trace("Mobilization complete") - // loop through the testSuites and write the results if !v.config.Write { return } + + // loop through the testSuites and write the results for _, testSuite := range v.TestSuites { - err := testSuite.WriteTestSetResultsYAML(v.ServiceName) + err := testSuite.WriteTestSetResults(v.ServiceName, v.config.Output) if err != nil { v.config.Logger.Error(fmt.Sprintf("Failed to write results for testSuite '%s': %v", testSuite.TestSuiteName, err)) }