Skip to content

Commit

Permalink
chore: address linter report - swallowed errors and minor test cleanu…
Browse files Browse the repository at this point in the history
…p of nil checks (#740)
  • Loading branch information
mildwonkey authored Oct 17, 2024
1 parent 7dc3dc9 commit 05a7f6e
Show file tree
Hide file tree
Showing 22 changed files with 126 additions and 107 deletions.
5 changes: 1 addition & 4 deletions src/cmd/common/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import (
"github.com/defenseunicorns/lula/src/pkg/message"
)

func SetupClI(logLevel string) error {

func SetupClI(logLevel string) {
match := map[string]message.LogLevel{
"warn": message.WarnLevel,
"info": message.InfoLevel,
Expand Down Expand Up @@ -37,6 +36,4 @@ func SetupClI(logLevel string) error {
}

printViperConfigUsed()

return nil
}
5 changes: 4 additions & 1 deletion src/cmd/console/console.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ var consoleCmd = &cobra.Command{

func ConsoleCommand() *cobra.Command {
consoleCmd.Flags().StringVarP(&opts.InputFile, "input-file", "f", "", "the path to the target OSCAL model")
consoleCmd.MarkFlagRequired("input-file")
err := consoleCmd.MarkFlagRequired("input-file")
if err != nil {
message.Fatal(err, "error initializing console command flags")
}
return consoleCmd
}
1 change: 1 addition & 0 deletions src/cmd/dev/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func ReadValidation(cmd *cobra.Command, spinner *message.Spinner, path string, t
go func() {
if timeout != NO_TIMEOUT {
time.Sleep(time.Duration(timeout) * time.Second)
//nolint:errcheck
cmd.Help()
message.Fatalf(fmt.Errorf("timed out waiting for stdin"), "timed out waiting for stdin")
}
Expand Down
8 changes: 6 additions & 2 deletions src/cmd/dev/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,19 @@ var lintCmd = &cobra.Command{
validationResults := DevLintCommand(lintOpts.InputFiles)

// If result file is specified, write the validation results to the file
var err error
if lintOpts.ResultFile != "" {
// If there is only one validation result, write it to the file
if len(validationResults) == 1 {
oscalValidation.WriteValidationResult(validationResults[0], lintOpts.ResultFile)
err = oscalValidation.WriteValidationResult(validationResults[0], lintOpts.ResultFile)
} else {
// If there are multiple validation results, write them to the file
oscalValidation.WriteValidationResults(validationResults, lintOpts.ResultFile)
err = oscalValidation.WriteValidationResults(validationResults, lintOpts.ResultFile)
}
}
if err != nil {
message.Fatal(err, "Error writing validation results")
}

// If there is at least one validation result that is not valid, exit with a fatal error
failedFiles := []string{}
Expand Down
27 changes: 21 additions & 6 deletions src/cmd/evaluate/evaluate.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,18 @@ func init() {
v := common.InitViper()

evaluateCmd.Flags().StringSliceVarP(&opts.InputFile, "input-file", "f", []string{}, "Path to the file to be evaluated")
evaluateCmd.MarkFlagRequired("input-file")

err := evaluateCmd.MarkFlagRequired("input-file")
if err != nil {
message.Fatal(err, "error initializing evaluate command flags")
}
evaluateCmd.Flags().StringVarP(&opts.Target, "target", "t", v.GetString(common.VTarget), "the specific control implementations or framework to validate against")
evaluateCmd.Flags().BoolVarP(&opts.summary, "summary", "s", v.GetBool(common.VSummary), "Print a summary of the evaluation")
evaluateCmd.Flags().BoolVar(&opts.machine, "machine", false, "Print a machine-readable output")
evaluateCmd.Flags().MarkHidden("machine") // Hidden for now as internal use only

err = evaluateCmd.Flags().MarkHidden("machine") // Hidden for now as internal use only
if err != nil {
message.Fatal(err, "error initializing hidden evaluate command flags")
}
}

func EvaluateCommand() *cobra.Command {
Expand Down Expand Up @@ -101,7 +107,10 @@ func EvaluateAssessments(assessmentMap map[string]*oscalTypes_1_1_2.AssessmentRe
AssessmentResults: assessment,
}

oscal.WriteOscalModel(filePath, &model)
err := oscal.WriteOscalModel(filePath, &model)
if err != nil {
return err
}
}
return nil
}
Expand Down Expand Up @@ -139,7 +148,10 @@ func evaluateTarget(target oscal.EvalResult, source string, summary, machine boo
// Print summary
if summary {
message.Info("Summary of All Observations:")
findingsWithoutObservations = result.Collapse(resultComparison).PrintObservationComparisonTable(false, true, false)
findingsWithoutObservations, err = result.Collapse(resultComparison).PrintObservationComparisonTable(false, true, false)
if err != nil {
return fmt.Errorf("error printing results: %w", err)
}
if len(findingsWithoutObservations) > 0 {
message.Warnf("%d Finding(s) Without Observations", len(findingsWithoutObservations))
message.Info(strings.Join(findingsWithoutObservations, ", "))
Expand Down Expand Up @@ -205,7 +217,10 @@ func evaluateTarget(target oscal.EvalResult, source string, summary, machine boo
"removed-satisfied": resultComparison["removed-satisfied"],
"removed-not-satisfied": resultComparison["removed-not-satisfied"],
}
findingsWithoutObservations = result.Collapse(failedFindings).PrintObservationComparisonTable(true, false, true)
findingsWithoutObservations, err = result.Collapse(failedFindings).PrintObservationComparisonTable(true, false, true)
if err != nil {
return fmt.Errorf("error printing results: %w", err)
}
// handle controls that failed but didn't have observations
if len(findingsWithoutObservations) > 0 {
message.Warnf("%d Failed Finding(s) Without Observations", len(findingsWithoutObservations))
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ var generateComponentCmd = &cobra.Command{
// Create a component definition from the catalog given required context
comp, err := oscal.ComponentFromCatalog(command, source, catalog, title, componentOpts.Requirements, remarks, componentOpts.Framework)
if err != nil {
message.Fatalf(err, fmt.Sprintf("error creating component - %s\n", err.Error()))
message.Fatalf(err, "error creating component - %s\n", err.Error())
}

var model = oscalTypes_1_1_2.OscalModels{
Expand Down
5 changes: 4 additions & 1 deletion src/cmd/tools/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ func ComposeCommand() *cobra.Command {
},
}
composeCmd.Flags().StringVarP(&inputFile, "input-file", "f", "", "the path to the target OSCAL component definition")
composeCmd.MarkFlagRequired("input-file")
err := composeCmd.MarkFlagRequired("input-file")
if err != nil {
message.Fatal(err, "error initializing evaluate flags")
}
composeCmd.Flags().StringVarP(&outputFile, "output-file", "o", "", "the path to the output file. If not specified, the output file will be the original filename with `-composed` appended")
composeCmd.Flags().StringVarP(&renderTypeString, "render", "r", "", "values to render the template with, options are: masked, constants, non-sensitive, all")
composeCmd.Flags().StringSliceVarP(&setOpts, "set", "s", []string{}, "set value overrides for templated data")
Expand Down
8 changes: 6 additions & 2 deletions src/cmd/tools/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,16 @@ var lintCmd = &cobra.Command{

// If result file is specified, write the validation results to the file
if opts.ResultFile != "" {
var err error
// If there is only one validation result, write it to the file
if len(validationResults) == 1 {
oscalValidation.WriteValidationResult(validationResults[0], opts.ResultFile)
err = oscalValidation.WriteValidationResult(validationResults[0], opts.ResultFile)
} else {
// If there are multiple validation results, write them to the file
oscalValidation.WriteValidationResults(validationResults, opts.ResultFile)
err = oscalValidation.WriteValidationResults(validationResults, opts.ResultFile)
}
if err != nil {
message.Fatal(err, "Error writing validation results")
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/cmd/tools/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ func TemplateCommand() *cobra.Command {
}

cmd.Flags().StringVarP(&inputFile, "input-file", "f", "", "the path to the target artifact")
cmd.MarkFlagRequired("input-file")
err := cmd.MarkFlagRequired("input-file")
if err != nil {
message.Fatal(err, "error initializing template command flags")
}
cmd.Flags().StringVarP(&outputFile, "output-file", "o", "", "the path to the output file. If not specified, the output file will be directed to stdout")
cmd.Flags().StringSliceVarP(&setOpts, "set", "s", []string{}, "set a value in the template data")
cmd.Flags().StringVarP(&renderTypeString, "render", "r", "masked", "values to render the template with, options are: masked, constants, non-sensitive, all")
Expand Down
5 changes: 4 additions & 1 deletion src/cmd/tools/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ func init() {
toolsCmd.AddCommand(upgradeCmd)

upgradeCmd.Flags().StringVarP(&upgradeOpts.InputFile, "input-file", "f", "", "the path to a oscal json schema file")
upgradeCmd.MarkFlagRequired("input-file")
err := upgradeCmd.MarkFlagRequired("input-file")
if err != nil {
message.Fatal(err, "error initializing upgrade command flags")
}
upgradeCmd.Flags().StringVarP(&upgradeOpts.OutputFile, "output-file", "o", "", "the path to write the linted oscal json schema file (default is the input file)")
upgradeCmd.Flags().StringVarP(&upgradeOpts.Version, "version", "v", versioning.GetLatestSupportedVersion(), "the version of the oscal schema to validate against (default is the latest supported version)")
upgradeCmd.Flags().StringVarP(&upgradeOpts.ValidationResult, "validation-result", "r", "", "the path to write the validation result file")
Expand Down
6 changes: 5 additions & 1 deletion src/cmd/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/defenseunicorns/lula/src/pkg/common/composition"
"github.com/defenseunicorns/lula/src/pkg/common/oscal"
"github.com/defenseunicorns/lula/src/pkg/common/validation"
"github.com/defenseunicorns/lula/src/pkg/message"
"github.com/defenseunicorns/lula/src/types"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -113,7 +114,10 @@ func ValidateCommand() *cobra.Command {

cmd.Flags().StringVarP(&outputFile, "output-file", "o", "", "the path to write assessment results. Creates a new file or appends to existing files")
cmd.Flags().StringVarP(&inputFile, "input-file", "f", "", "the path to the target OSCAL component definition")
cmd.MarkFlagRequired("input-file")
err := cmd.MarkFlagRequired("input-file")
if err != nil {
message.Fatal(err, "error initializing upgrade command flags")
}
cmd.Flags().StringVarP(&target, "target", "t", v.GetString(common.VTarget), "the specific control implementations or framework to validate against")
cmd.Flags().BoolVar(&confirmExecution, "confirm-execution", false, "confirm execution scripts run as part of the validation")
cmd.Flags().BoolVar(&runNonInteractively, "non-interactive", false, "run the command non-interactively")
Expand Down
13 changes: 10 additions & 3 deletions src/internal/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,10 @@ func CollectTemplatingData(constants map[string]interface{}, variables []Variabl
templateData.SensitiveVariables = mergeStringMaps(templateData.SensitiveVariables, envMap)

// Apply overrides
overrideTemplateValues(templateData, overrides)
err = overrideTemplateValues(templateData, overrides)
if err != nil {
return templateData, err
}

// Validate that all env vars have values - currently debug prints missing env vars (do we want to return an error?)
var variablesMissing strings.Builder
Expand Down Expand Up @@ -380,7 +383,7 @@ func checkForInvalidKeys(constants map[string]interface{}, variables []VariableC
}

// overrideTemplateValues overrides values in the templateData object with values from the overrides map
func overrideTemplateValues(templateData *TemplateData, overrides map[string]string) {
func overrideTemplateValues(templateData *TemplateData, overrides map[string]string) error {
for path, value := range overrides {
// for each key, check if .var or .const
// if .var, set the value in the templateData.Variables or templateData.SensitiveVariables
Expand All @@ -396,9 +399,13 @@ func overrideTemplateValues(templateData *TemplateData, overrides map[string]str
} else if strings.HasPrefix(path, "."+CONST+".") {
// Set the value in the templateData.Constants
key := strings.TrimPrefix(path, "."+CONST+".")
setNestedValue(templateData.Constants, key, value)
err := setNestedValue(templateData.Constants, key, value)
if err != nil {
return err
}
}
}
return nil
}

// Helper function to set a value in a map based on a JSON-like key path
Expand Down
83 changes: 20 additions & 63 deletions src/pkg/common/composition/composition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
oscalTypes_1_1_2 "github.com/defenseunicorns/go-oscal/src/types/oscal-1-1-2"
"github.com/defenseunicorns/lula/src/internal/template"
"github.com/defenseunicorns/lula/src/pkg/common/composition"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v3"
)

Expand Down Expand Up @@ -122,25 +123,11 @@ func TestComposeFromPath(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}

if compDefComposed.Components == nil {
t.Error("expected the component definition to have components")
}

if compDefComposed.BackMatter == nil {
t.Error("expected the component definition to have back matter")
}

if compDefComposed.BackMatter.Resources == nil {
t.Error("expected the component definition to have back matter resources")
}

if len(*compDefComposed.BackMatter.Resources) != 0 {
t.Error("expected the back matter to contain 0 resources (validation)")
}
require.NotNil(t, compDefComposed)
require.NotNil(t, compDefComposed.Components)
require.NotNil(t, compDefComposed.BackMatter)
require.NotNil(t, compDefComposed.BackMatter.Resources)
require.Equal(t, len(*compDefComposed.BackMatter.Resources), 0)
})

// Test the templating of the component definition with nested templated imports
Expand Down Expand Up @@ -168,25 +155,11 @@ func TestComposeFromPath(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}

if compDefComposed.Components == nil {
t.Error("expected the component definition to have components")
}

if compDefComposed.BackMatter == nil {
t.Error("expected the component definition to have back matter")
}

if compDefComposed.BackMatter.Resources == nil {
t.Fatalf("expected the component definition to have back matter resources")
}

if len(*compDefComposed.BackMatter.Resources) != 1 {
t.Error("expected the back matter to contain 1 resource (validation)")
}
require.NotNil(t, compDefComposed)
require.NotNil(t, compDefComposed.Components)
require.NotNil(t, compDefComposed.BackMatter)
require.NotNil(t, compDefComposed.BackMatter.Resources)
require.Len(t, *compDefComposed.BackMatter.Resources, 1)
})

t.Run("Errors when file does not exist", func(t *testing.T) {
Expand Down Expand Up @@ -240,9 +213,7 @@ func TestComposeComponentDefinitions(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}
require.NotNil(t, compDefComposed)

// Only the last-modified timestamp should be different
if !reflect.DeepEqual(*og.BackMatter, *compDefComposed.BackMatter) {
Expand All @@ -260,12 +231,10 @@ func TestComposeComponentDefinitions(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}
require.NotNil(t, compDefComposed)

if reflect.DeepEqual(*og, *compDefComposed) {
t.Errorf("expected component definition to have changed.")
t.Error("expected component definition to have changed.")
}
})

Expand All @@ -279,9 +248,7 @@ func TestComposeComponentDefinitions(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}
require.NotNil(t, compDefComposed)

if compDefComposed.Components == og.Components {
t.Error("expected there to be components")
Expand All @@ -302,9 +269,7 @@ func TestComposeComponentDefinitions(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}
require.NotNil(t, compDefComposed)

if compDefComposed.Components == og.Components {
t.Error("expected there to be components")
Expand Down Expand Up @@ -332,9 +297,7 @@ func TestComposeComponentDefinitions(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}
require.NotNil(t, compDefComposed)

if compDefComposed.Components == og.Components {
t.Error("expected there to be new components")
Expand Down Expand Up @@ -393,9 +356,7 @@ func TestComposeComponentValidations(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}
require.NotNil(t, compDefComposed)

// Only the last-modified timestamp should be different
if !reflect.DeepEqual(*og.BackMatter, *compDefComposed.BackMatter) {
Expand All @@ -413,9 +374,7 @@ func TestComposeComponentValidations(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}
require.NotNil(t, compDefComposed)

if reflect.DeepEqual(*og, *compDefComposed) {
t.Error("expected the component definition to be changed")
Expand All @@ -440,9 +399,7 @@ func TestComposeComponentValidations(t *testing.T) {
}

compDefComposed := model.ComponentDefinition
if compDefComposed == nil {
t.Error("expected the component definition to be non-nil")
}
require.NotNil(t, compDefComposed)

if reflect.DeepEqual(*og, *compDefComposed) {
t.Error("expected the component definition to be changed")
Expand Down
Loading

0 comments on commit 05a7f6e

Please sign in to comment.