From fc08e1c15a7b40741c8cc8a593051564473d272c Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Fri, 12 Nov 2021 10:29:54 -0300 Subject: [PATCH] customimages: avoid type casting and add tests Previously the implementation of CustomImages map was a bit redundant. Basically a map[string]string was created using the string representation of each languages.language as key, and when were we going to access this map we always receive a languages.Language and need to get the string representation of this language, which make an unnecessary type casting. This commit change the type of CustomImages to store a languages.Language as key and avoid these type casting. The function `NewCustomImages` was renamed to `Default` to make more clear and a new function `MustParseCustomImages` was created to parse the input taken from Viper. This commit also add some new tests to assert the default values and test the parsing. Updates #718 Signed-off-by: Matheus Alcantara --- config/config.go | 16 +-- config/config_test.go | 9 +- internal/controllers/analyzer/analyzer.go | 5 +- .../entities/custom_images/custom_images.go | 105 ++++++++++++++++-- .../custom_images/custom_images_test.go | 98 +++++++++++++++- internal/helpers/messages/error.go | 2 +- internal/services/formatters/service.go | 2 +- 7 files changed, 203 insertions(+), 34 deletions(-) diff --git a/config/config.go b/config/config.go index 89c5808fe..fa4300a97 100644 --- a/config/config.go +++ b/config/config.go @@ -169,7 +169,7 @@ func New() *Config { ContainerBindProjectPath: "", ToolsConfig: toolsconfig.Default(), ShowVulnerabilitiesTypes: []string{vulnerability.Vulnerability.ToString()}, - CustomImages: customimages.NewCustomImages(), + CustomImages: customimages.Default(), DisableDocker: dist.IsStandAlone(), CustomRulesPath: "", EnableInformationSeverity: false, @@ -230,7 +230,7 @@ func (c *Config) LoadStartFlags(cmd *cobra.Command) *Config { // config instance. Note the values loaded from config file will override // current config instance. // -//nolint:funlen,gocyclo +//nolint:funlen func (c *Config) LoadFromConfigFile() *Config { if !c.setViperConfigsAndReturnIfExistFile() { return c @@ -306,16 +306,8 @@ func (c *Config) LoadFromConfigFile() *Config { ) c.EnableInformationSeverity = viper.GetBool(c.toLowerCamel(EnvEnableInformationSeverity)) - if images := viper.Get(c.toLowerCamel(EnvCustomImages)); images != nil { - customImg := customimages.CustomImages{} - bytes, err := json.Marshal(images) - if err != nil { - logger.LogErrorWithLevel(messages.MsgErrorWhileParsingCustomImages, err) - } - if err := json.Unmarshal(bytes, &customImg); err != nil { - logger.LogErrorWithLevel(messages.MsgErrorWhileParsingCustomImages, err) - } - c.CustomImages = customImg + if images := viper.GetStringMap(c.toLowerCamel(EnvCustomImages)); images != nil { + c.CustomImages = customimages.MustParseCustomImages(images) } c.ShowVulnerabilitiesTypes = valueordefault.GetSliceStringValueOrDefault( diff --git a/config/config_test.go b/config/config_test.go index e28a76b01..40acf3769 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -23,6 +23,7 @@ import ( "strings" "testing" + "github.com/ZupIT/horusec-devkit/pkg/enums/languages" "github.com/ZupIT/horusec-devkit/pkg/enums/vulnerability" "github.com/ZupIT/horusec-devkit/pkg/utils/logger" @@ -120,7 +121,7 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, toolsconfig.Config{ IsToIgnore: true, }, configs.ToolsConfig[tools.GoSec]) - assert.Equal(t, "docker.io/company/go:latest", configs.CustomImages["go"]) + assert.Equal(t, "docker.io/company/go:latest", configs.CustomImages[languages.Go]) }) t.Run("Should return horusec config using config file and override by environment", func(t *testing.T) { viper.Reset() @@ -160,7 +161,7 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, toolsconfig.Config{ IsToIgnore: true, }, configs.ToolsConfig[tools.GoSec]) - assert.Equal(t, "docker.io/company/go:latest", configs.CustomImages["go"]) + assert.Equal(t, "docker.io/company/go:latest", configs.CustomImages[languages.Go]) assert.NoError(t, os.Setenv(config.EnvHorusecAPIUri, "http://horusec.com")) assert.NoError(t, os.Setenv(config.EnvTimeoutInSecondsRequest, "99")) @@ -263,7 +264,7 @@ func TestNewHorusecConfig(t *testing.T) { assert.Equal(t, toolsconfig.Config{ IsToIgnore: true, }, configs.ToolsConfig[tools.GoSec]) - assert.Equal(t, "docker.io/company/go:latest", configs.CustomImages["go"]) + assert.Equal(t, "docker.io/company/go:latest", configs.CustomImages[languages.Go]) assert.NoError(t, os.Setenv(config.EnvTimeoutInSecondsRequest, "99")) assert.NoError(t, os.Setenv(config.EnvTimeoutInSecondsAnalysis, "999")) @@ -636,7 +637,7 @@ func TestConfig_Bytes(t *testing.T) { "version": "" }`) cfg := config.Config{} - assert.Equal(t, expectedConfig, cfg.Bytes()) + assert.Equal(t, string(expectedConfig), string(cfg.Bytes())) }) } diff --git a/internal/controllers/analyzer/analyzer.go b/internal/controllers/analyzer/analyzer.go index 7b5445c65..1ce68fcfa 100644 --- a/internal/controllers/analyzer/analyzer.go +++ b/internal/controllers/analyzer/analyzer.go @@ -498,8 +498,11 @@ func (a *Analyzer) setAnalysisError(err error) { a.analysis.Errors += toAppend + err.Error() } } + func (a *Analyzer) getCustomOrDefaultImage(language languages.Language) string { - if customImage := a.config.CustomImages[language.GetCustomImagesKeyByLanguage()]; customImage != "" { + // Images can be set to empty on config file, so we need to use only if its not empty. + // If its empty we return the default value. + if customImage := a.config.CustomImages[language]; customImage != "" { return customImage } return fmt.Sprintf("%s/%s", images.DefaultRegistry, images.MapValues()[language]) diff --git a/internal/entities/custom_images/custom_images.go b/internal/entities/custom_images/custom_images.go index 999b8a874..15f22a61a 100644 --- a/internal/entities/custom_images/custom_images.go +++ b/internal/entities/custom_images/custom_images.go @@ -15,22 +15,103 @@ package customimages import ( + "encoding/json" + "fmt" + "github.com/ZupIT/horusec-devkit/pkg/enums/languages" + "github.com/ZupIT/horusec-devkit/pkg/utils/logger" "github.com/ZupIT/horusec/internal/enums/images" + "github.com/ZupIT/horusec/internal/helpers/messages" ) -type CustomImages map[string]string - -func NewCustomImages() map[string]string { - customMap := map[string]string{} - allLanguages := languages.Generic.MapLanguagesEnableInCLI() - imagesEnableToCustom := images.MapValues() - for langEnable := range imagesEnableToCustom { - for lang, key := range allLanguages { - if langEnable == lang { - customMap[key] = "" - } +// CustomImages is a map of language to a custom image. +// +// The custom image value can be empty. +type CustomImages map[languages.Language]string + +// MarshalJSON implements json.Marshaler interface. +// +// Note that we only implement this interface to get the same +// json representation of custom images that is used config file. +// On config file we use all keys in lower case and the const values +// from languages package use language names in CamelCase, so, when we +// print the custom images on debug logging we get a different result from +// config file, what can cause doubts. +// +// A better approach would be use always the language name as declared on +// languages package, but changing this now will introduce breaking changes. +// So we should centralize these castings only in this package and let the +// others not worry about it. +// +// nolint: funlen +func (c CustomImages) MarshalJSON() ([]byte, error) { + if len(c) == 0 { + return []byte("null"), nil + } + + // TODO(matheus): This method should be removed from Language type. + // A better approach would convert to a public function from languages + // package. + enabledLanguages := languages.Generic.MapLanguagesEnableInCLI() + + result := make(map[string]string, len(c)) + + for language, image := range c { + if lang, exists := enabledLanguages[language]; exists { + result[lang] = image + } + } + + return json.Marshal(result) +} + +// Default return a new CustomImages map with +// empty images for all languages. +func Default() CustomImages { + defaultImages := images.MapValues() + + customImages := make(CustomImages, len(defaultImages)) + + for lang := range defaultImages { + customImages[lang] = "" + } + + return customImages +} + +// MustParseCustomImages parse a input to CustomImages. +// +// If some error occur the default values will be returned and the error +// will be logged. +func MustParseCustomImages(input map[string]interface{}) CustomImages { + customImages, err := parseCustomImages(input) + if err != nil { + logger.LogErrorWithLevel(messages.MsgErrorWhileParsingCustomImages, err) + return Default() + } + return customImages +} + +// nolint: funlen +func parseCustomImages(input map[string]interface{}) (CustomImages, error) { + customImg := make(CustomImages, len(input)) + + for language, value := range input { + // TODO(matheus): We should rename CSharp const value. + if language == "csharp" { + language = string(languages.CSharp) } + + lang := languages.ParseStringToLanguage(language) + if lang == languages.Unknown { + return nil, fmt.Errorf("invalid language %s", language) + } + v, ok := value.(string) + if !ok { + return nil, fmt.Errorf("invalid value %v. Must be a string", value) + } + customImg[lang] = v } - return customMap + + return customImg, nil } diff --git a/internal/entities/custom_images/custom_images_test.go b/internal/entities/custom_images/custom_images_test.go index 40cd67930..4500d266a 100644 --- a/internal/entities/custom_images/custom_images_test.go +++ b/internal/entities/custom_images/custom_images_test.go @@ -12,16 +12,108 @@ // See the License for the specific language governing permissions and // limitations under the License. -package customimages +package customimages_test import ( + "io" "testing" + "github.com/ZupIT/horusec-devkit/pkg/enums/languages" + "github.com/ZupIT/horusec-devkit/pkg/utils/logger" + customimages "github.com/ZupIT/horusec/internal/entities/custom_images" "github.com/stretchr/testify/assert" ) func TestNewCustomImages(t *testing.T) { - t.Run("Should return 12 languages enable and in custom expected", func(t *testing.T) { - assert.Equal(t, 12, len(NewCustomImages())) + t.Run("Should return 12 custom images", func(t *testing.T) { + assert.Equal(t, 12, len(customimages.Default())) }) + + t.Run("Should return empty image for all languages as default", func(t *testing.T) { + images := customimages.Default() + for lang, image := range images { + assert.Empty(t, image, "Expected empty default image for %s", lang) + } + }) +} + +func TestMustParseCustomImages(t *testing.T) { + testcases := []struct { + name string + input map[string]interface{} + expected customimages.CustomImages + }{ + { + name: "Should parse valid custom images", + input: map[string]interface{}{ + "go": "custom/image", + "csharp": "custom/image", + "dart": "custom/image", + "ruby": "custom/image", + "python": "custom/image", + "java": "custom/image", + "kotlin": "custom/image", + "javascript": "custom/image", + "typescript": "custom/image", + "leaks": "custom/image", + "hcl": "custom/image", + "c": "custom/image", + "php": "custom/image", + "html": "custom/image", + "generic": "custom/image", + "yaml": "custom/image", + "elixir": "custom/image", + "shell": "custom/image", + "nginx": "custom/image", + "swift": "custom/image", + }, + expected: customimages.CustomImages{ + languages.Go: "custom/image", + languages.CSharp: "custom/image", + languages.Dart: "custom/image", + languages.Ruby: "custom/image", + languages.Python: "custom/image", + languages.Java: "custom/image", + languages.Kotlin: "custom/image", + languages.Javascript: "custom/image", + languages.Typescript: "custom/image", + languages.Leaks: "custom/image", + languages.HCL: "custom/image", + languages.C: "custom/image", + languages.PHP: "custom/image", + languages.HTML: "custom/image", + languages.Generic: "custom/image", + languages.Yaml: "custom/image", + languages.Elixir: "custom/image", + languages.Shell: "custom/image", + languages.Nginx: "custom/image", + languages.Swift: "custom/image", + }, + }, + { + name: "Should return default values using invalid schema", + input: map[string]interface{}{ + "go": map[string]interface{}{ + "invalid": "schema", + }, + }, + expected: customimages.Default(), + }, + { + name: "Should return default values using invalid language", + input: map[string]interface{}{ + "invalid": "custom/image", + }, + expected: customimages.Default(), + }, + } + + logger.LogSetOutput(io.Discard) + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + images := customimages.MustParseCustomImages(tt.input) + + assert.Equal(t, tt.expected, images) + }) + } } diff --git a/internal/helpers/messages/error.go b/internal/helpers/messages/error.go index f6f5c1bbd..d4e7d5d89 100644 --- a/internal/helpers/messages/error.go +++ b/internal/helpers/messages/error.go @@ -71,6 +71,6 @@ const ( MsgErrorErrorOnCreateConfigFile = "{HORUSEC-CLI} Error on create config file: " MsgErrorErrorOnReadConfigFile = "{HORUSEC-CLI} Error on read config file on path: " MsgErrorFailedToPullImage = "{HORUSEC_CLI} Failed to pull docker image" - MsgErrorWhileParsingCustomImages = "{HORUSEC_CLI} Error when parsing custom images config." + MsgErrorWhileParsingCustomImages = "{HORUSEC_CLI} Error when parsing custom images config. Using default values" MsgErrorSettingLogFile = "{HORUSEC_CLI} Error when setting log file" ) diff --git a/internal/services/formatters/service.go b/internal/services/formatters/service.go index 064b02f57..ff800c4a2 100644 --- a/internal/services/formatters/service.go +++ b/internal/services/formatters/service.go @@ -265,7 +265,7 @@ func (s *Service) GetCustomRulesByLanguage(lang languages.Language) []engine.Rul } func (s *Service) GetCustomImageByLanguage(language languages.Language) string { - return s.config.CustomImages[language.GetCustomImagesKeyByLanguage()] + return s.config.CustomImages[language] } func (s *Service) IsOwaspDependencyCheckDisable() bool {