refactor(flag): improve flag system architecture and extensibility (#8718)

Co-authored-by: DmitriyLewen <[email protected]>

Author: knqyf263

pkg/commands/app.go

@@ -315,9 +315,9 @@ func TestFlags(t *testing.T) {
 			rootCmd.SetOut(io.Discard)
 
 			flags := &flag.Flags{
-				GlobalFlagGroup: globalFlags,
-				ReportFlagGroup: flag.NewReportFlagGroup(),
-				ScanFlagGroup:   flag.NewScanFlagGroup(),
+				globalFlags,
+				flag.NewReportFlagGroup(),
+				flag.NewScanFlagGroup(),
 			}
 			cmd := &cobra.Command{
 				Use: "test",

pkg/commands/app_test.go

@@ -77,16 +77,14 @@ func (f *AWSFlagGroup) Flags() []Flagger {
 	}
 }
 
-func (f *AWSFlagGroup) ToOptions() (AWSOptions, error) {
-	if err := parseFlags(f); err != nil {
-		return AWSOptions{}, err
-	}
-	return AWSOptions{
+func (f *AWSFlagGroup) ToOptions(opts *Options) error {
+	opts.AWSOptions = AWSOptions{
 		Region:       f.Region.Value(),
 		Endpoint:     f.Endpoint.Value(),
 		Services:     f.Services.Value(),
 		SkipServices: f.SkipServices.Value(),
 		Account:      f.Account.Value(),
 		ARN:          f.ARN.Value(),
-	}, nil
+	}
+	return nil
 }

pkg/flag/aws_flags.go

@@ -106,17 +106,15 @@ func (fg *CacheFlagGroup) Flags() []Flagger {
 	}
 }
 
-func (fg *CacheFlagGroup) ToOptions() (CacheOptions, error) {
-	if err := parseFlags(fg); err != nil {
-		return CacheOptions{}, err
-	}
-
-	return CacheOptions{
+func (fg *CacheFlagGroup) ToOptions(opts *Options) error {
+	opts.CacheOptions = CacheOptions{
+		ClearCache:   fg.ClearCache.Value(),
 		CacheBackend: fg.CacheBackend.Value(),
 		CacheTTL:     fg.CacheTTL.Value(),
 		RedisTLS:     fg.RedisTLS.Value(),
 		RedisCACert:  fg.RedisCACert.Value(),
 		RedisCert:    fg.RedisCert.Value(),
 		RedisKey:     fg.RedisKey.Value(),
-	}, nil
+	}
+	return nil
 }

pkg/flag/cache_flags.go

@@ -78,17 +78,14 @@ func (fg *CleanFlagGroup) Flags() []Flagger {
 	}
 }
 
-func (fg *CleanFlagGroup) ToOptions() (CleanOptions, error) {
-	if err := parseFlags(fg); err != nil {
-		return CleanOptions{}, err
-	}
-
-	return CleanOptions{
+func (fg *CleanFlagGroup) ToOptions(opts *Options) error {
+	opts.CleanOptions = CleanOptions{
 		CleanAll:             fg.CleanAll.Value(),
 		CleanVulnerabilityDB: fg.CleanVulnerabilityDB.Value(),
 		CleanJavaDB:          fg.CleanJavaDB.Value(),
 		CleanChecksBundle:    fg.CleanChecksBundle.Value(),
 		CleanScanCache:       fg.CleanScanCache.Value(),
 		CleanVEXRepositories: fg.CleanVEXRepositories.Value(),
-	}, nil
+	}
+	return nil
 }

pkg/flag/clean_flags.go

@@ -127,44 +127,40 @@ func (f *DBFlagGroup) Flags() []Flagger {
 	}
 }
 
-func (f *DBFlagGroup) ToOptions() (DBOptions, error) {
-	if err := parseFlags(f); err != nil {
-		return DBOptions{}, err
-	}
-
+func (f *DBFlagGroup) ToOptions(opts *Options) error {
 	skipDBUpdate := f.SkipDBUpdate.Value()
 	skipJavaDBUpdate := f.SkipJavaDBUpdate.Value()
 	downloadDBOnly := f.DownloadDBOnly.Value()
 	downloadJavaDBOnly := f.DownloadJavaDBOnly.Value()
 
 	if downloadDBOnly && downloadJavaDBOnly {
-		return DBOptions{}, xerrors.New("--download-db-only and --download-java-db-only options can not be specified both")
+		return xerrors.New("--download-db-only and --download-java-db-only options can not be specified both")
 	}
 	if downloadDBOnly && skipDBUpdate {
-		return DBOptions{}, xerrors.New("--skip-db-update and --download-db-only options can not be specified both")
+		return xerrors.New("--skip-db-update and --download-db-only options can not be specified both")
 	}
 	if downloadJavaDBOnly && skipJavaDBUpdate {
-		return DBOptions{}, xerrors.New("--skip-java-db-update and --download-java-db-only options can not be specified both")
+		return xerrors.New("--skip-java-db-update and --download-java-db-only options can not be specified both")
 	}
 
 	var dbRepositories, javaDBRepositories []name.Reference
 	for _, repo := range f.DBRepositories.Value() {
 		ref, err := parseRepository(repo, db.SchemaVersion)
 		if err != nil {
-			return DBOptions{}, xerrors.Errorf("invalid DB repository: %w", err)
+			return xerrors.Errorf("invalid DB repository: %w", err)
 		}
 		dbRepositories = append(dbRepositories, ref)
 	}
 
 	for _, repo := range f.JavaDBRepositories.Value() {
 		ref, err := parseRepository(repo, javadb.SchemaVersion)
 		if err != nil {
-			return DBOptions{}, xerrors.Errorf("invalid javadb repository: %w", err)
+			return xerrors.Errorf("invalid javadb repository: %w", err)
 		}
 		javaDBRepositories = append(javaDBRepositories, ref)
 	}
 
-	return DBOptions{
+	opts.DBOptions = DBOptions{
 		Reset:              f.Reset.Value(),
 		DownloadDBOnly:     downloadDBOnly,
 		SkipDBUpdate:       skipDBUpdate,
@@ -173,7 +169,8 @@ func (f *DBFlagGroup) ToOptions() (DBOptions, error) {
 		NoProgress:         f.NoProgress.Value(),
 		DBRepositories:     dbRepositories,
 		JavaDBRepositories: javaDBRepositories,
-	}, nil
+	}
+	return nil
 }
 
 func parseRepository(repo string, dbSchemaVersion int) (name.Reference, error) {

pkg/flag/db_flags.go

@@ -101,15 +101,15 @@ func TestDBFlagGroup_ToOptions(t *testing.T) {
 				DBRepositories:     flag.DBRepositoryFlag.Clone(),
 				JavaDBRepositories: flag.JavaDBRepositoryFlag.Clone(),
 			}
-			got, err := f.ToOptions()
+			flags := flag.Flags{f}
+			got, err := flags.ToOptions(nil)
 			if tt.wantErr != "" {
-				require.Error(t, err)
 				assert.ErrorContains(t, err, tt.wantErr)
 				return
 			}
 			require.NoError(t, err)
 
-			assert.EqualExportedValues(t, tt.want, got)
+			assert.EqualExportedValues(t, tt.want, got.DBOptions)
 
 			// Assert log messages
 			assert.Equal(t, tt.wantLogs, out.Messages(), tt.name)

pkg/flag/db_flags_test.go

@@ -137,17 +137,13 @@ func (f *GlobalFlagGroup) Bind(cmd *cobra.Command) error {
 	return nil
 }
 
-func (f *GlobalFlagGroup) ToOptions() (GlobalOptions, error) {
-	if err := parseFlags(f); err != nil {
-		return GlobalOptions{}, err
-	}
-
+func (f *GlobalFlagGroup) ToOptions(opts *Options) error {
 	// Keep TRIVY_NON_SSL for backward compatibility
 	insecure := f.Insecure.Value() || os.Getenv("TRIVY_NON_SSL") != ""
 
 	log.Debug("Cache dir", log.String("dir", f.CacheDir.Value()))
 
-	return GlobalOptions{
+	opts.GlobalOptions = GlobalOptions{
 		ConfigFile:            f.ConfigFile.Value(),
 		ShowVersion:           f.ShowVersion.Value(),
 		Quiet:                 f.Quiet.Value(),
@@ -156,5 +152,6 @@ func (f *GlobalFlagGroup) ToOptions() (GlobalOptions, error) {
 		Timeout:               f.Timeout.Value(),
 		CacheDir:              f.CacheDir.Value(),
 		GenerateDefaultConfig: f.GenerateDefaultConfig.Value(),
-	}, nil
+	}
+	return nil
 }

pkg/flag/global_flags.go

@@ -119,16 +119,12 @@ func (f *ImageFlagGroup) Flags() []Flagger {
 	}
 }
 
-func (f *ImageFlagGroup) ToOptions() (ImageOptions, error) {
-	if err := parseFlags(f); err != nil {
-		return ImageOptions{}, err
-	}
-
+func (f *ImageFlagGroup) ToOptions(opts *Options) error {
 	var platform ftypes.Platform
 	if p := f.Platform.Value(); p != "" {
 		pl, err := v1.ParsePlatform(p)
 		if err != nil {
-			return ImageOptions{}, xerrors.Errorf("unable to parse platform: %w", err)
+			return xerrors.Errorf("unable to parse platform: %w", err)
 		}
 		if pl.OS == "*" {
 			pl.OS = "" // Empty OS means any OS
@@ -139,12 +135,12 @@ func (f *ImageFlagGroup) ToOptions() (ImageOptions, error) {
 	if value := f.MaxImageSize.Value(); value != "" {
 		parsedSize, err := units.FromHumanSize(value)
 		if err != nil {
-			return ImageOptions{}, xerrors.Errorf("invalid max image size %q: %w", value, err)
+			return xerrors.Errorf("invalid max image size %q: %w", value, err)
 		}
 		maxSize = parsedSize
 	}
 
-	return ImageOptions{
+	opts.ImageOptions = ImageOptions{
 		Input:               f.Input.Value(),
 		ImageConfigScanners: xstrings.ToTSlice[types.Scanner](f.ImageConfigScanners.Value()),
 		ScanRemovedPkgs:     f.ScanRemovedPkgs.Value(),
@@ -153,5 +149,6 @@ func (f *ImageFlagGroup) ToOptions() (ImageOptions, error) {
 		PodmanHost:          f.PodmanHost.Value(),
 		ImageSources:        xstrings.ToTSlice[ftypes.ImageSource](f.ImageSources.Value()),
 		MaxImageSize:        maxSize,
-	}, nil
+	}
+	return nil
 }

pkg/flag/image_flags.go

@@ -79,13 +79,14 @@ func TestImageFlagGroup_ToOptions(t *testing.T) {
 				Platform:     flag.PlatformFlag.Clone(),
 			}
 
-			got, err := f.ToOptions()
+			flags := flag.Flags{f}
+			got, err := flags.ToOptions(nil)
 			if tt.wantErr != "" {
 				assert.ErrorContains(t, err, tt.wantErr)
 				return
 			}
 			require.NoError(t, err)
-			assert.EqualExportedValues(t, tt.want, got)
+			assert.EqualExportedValues(t, tt.want, got.ImageOptions)
 		})
 	}
 }

pkg/flag/image_flags_test.go

@@ -2,11 +2,11 @@ package flag
 
 import (
 	"errors"
-	"fmt"
 	"strconv"
 	"strings"
 
 	"github.com/samber/lo"
+	"golang.org/x/xerrors"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -173,33 +173,29 @@ func (f *K8sFlagGroup) Flags() []Flagger {
 	}
 }
 
-func (f *K8sFlagGroup) ToOptions() (K8sOptions, error) {
-	if err := parseFlags(f); err != nil {
-		return K8sOptions{}, err
-	}
-
+func (f *K8sFlagGroup) ToOptions(opts *Options) error {
 	tolerations, err := optionToTolerations(f.Tolerations.Value())
 	if err != nil {
-		return K8sOptions{}, err
+		return err
 	}
 
 	exludeNodeLabels := make(map[string]string)
 	exludeNodes := f.ExcludeNodes.Value()
 	for _, exludeNodeValue := range exludeNodes {
 		excludeNodeParts := strings.Split(exludeNodeValue, ":")
 		if len(excludeNodeParts) != 2 {
-			return K8sOptions{}, fmt.Errorf("exclude node %s must be a key:value", exludeNodeValue)
+			return xerrors.Errorf("exclude node %s must be a key:value", exludeNodeValue)
 		}
 		exludeNodeLabels[excludeNodeParts[0]] = excludeNodeParts[1]
 	}
 	if len(f.ExcludeNamespaces.Value()) > 0 && len(f.IncludeNamespaces.Value()) > 0 {
-		return K8sOptions{}, errors.New("include-namespaces and exclude-namespaces flags cannot be used together")
+		return xerrors.New("include-namespaces and exclude-namespaces flags cannot be used together")
 	}
 	if len(f.ExcludeKinds.Value()) > 0 && len(f.IncludeKinds.Value()) > 0 {
-		return K8sOptions{}, errors.New("include-kinds and exclude-kinds flags cannot be used together")
+		return xerrors.New("include-kinds and exclude-kinds flags cannot be used together")
 	}
 
-	return K8sOptions{
+	opts.K8sOptions = K8sOptions{
 		KubeConfig:             f.KubeConfig.Value(),
 		K8sVersion:             f.K8sVersion.Value(),
 		Tolerations:            tolerations,
@@ -215,7 +211,8 @@ func (f *K8sFlagGroup) ToOptions() (K8sOptions, error) {
 		ExcludeNamespaces:      f.ExcludeNamespaces.Value(),
 		IncludeNamespaces:      f.IncludeNamespaces.Value(),
 		Burst:                  f.Burst.Value(),
-	}, nil
+	}
+	return nil
 }
 
 func optionToTolerations(tolerationsOptions []string) ([]corev1.Toleration, error) {

pkg/flag/kubernetes_flags.go

@@ -115,11 +115,7 @@ func (f *LicenseFlagGroup) Flags() []Flagger {
 	}
 }
 
-func (f *LicenseFlagGroup) ToOptions() (LicenseOptions, error) {
-	if err := parseFlags(f); err != nil {
-		return LicenseOptions{}, err
-	}
-
+func (f *LicenseFlagGroup) ToOptions(opts *Options) error {
 	licenseCategories := make(map[types.LicenseCategory][]string)
 	licenseCategories[types.CategoryForbidden] = f.LicenseForbidden.Value()
 	licenseCategories[types.CategoryRestricted] = f.LicenseRestricted.Value()
@@ -128,10 +124,11 @@ func (f *LicenseFlagGroup) ToOptions() (LicenseOptions, error) {
 	licenseCategories[types.CategoryPermissive] = f.LicensePermissive.Value()
 	licenseCategories[types.CategoryUnencumbered] = f.LicenseUnencumbered.Value()
 
-	return LicenseOptions{
+	opts.LicenseOptions = LicenseOptions{
 		LicenseFull:            f.LicenseFull.Value(),
 		IgnoredLicenses:        f.IgnoredLicenses.Value(),
 		LicenseConfidenceLevel: f.LicenseConfidenceLevel.Value(),
 		LicenseCategories:      licenseCategories,
-	}, nil
+	}
+	return nil
 }

pkg/flag/license_flags.go

@@ -204,12 +204,8 @@ func (f *MisconfFlagGroup) Flags() []Flagger {
 	}
 }
 
-func (f *MisconfFlagGroup) ToOptions() (MisconfOptions, error) {
-	if err := parseFlags(f); err != nil {
-		return MisconfOptions{}, err
-	}
-
-	return MisconfOptions{
+func (f *MisconfFlagGroup) ToOptions(opts *Options) error {
+	opts.MisconfOptions = MisconfOptions{
 		IncludeNonFailures:      f.IncludeNonFailures.Value(),
 		ResetChecksBundle:       f.ResetChecksBundle.Value(),
 		ChecksBundleRepository:  f.ChecksBundleRepository.Value(),
@@ -225,5 +221,6 @@ func (f *MisconfFlagGroup) ToOptions() (MisconfOptions, error) {
 		MisconfigScanners:       xstrings.ToTSlice[analyzer.Type](f.MisconfigScanners.Value()),
 		ConfigFileSchemas:       f.ConfigFileSchemas.Value(),
 		RenderCause:             xstrings.ToTSlice[types.ConfigType](f.RenderCause.Value()),
-	}, nil
+	}
+	return nil
 }

pkg/flag/misconf_flags.go

@@ -58,13 +58,10 @@ func (f *ModuleFlagGroup) Flags() []Flagger {
 	}
 }
 
-func (f *ModuleFlagGroup) ToOptions() (ModuleOptions, error) {
-	if err := parseFlags(f); err != nil {
-		return ModuleOptions{}, err
-	}
-
-	return ModuleOptions{
+func (f *ModuleFlagGroup) ToOptions(opts *Options) error {
+	opts.ModuleOptions = ModuleOptions{
 		ModuleDir:      f.Dir.Value(),
 		EnabledModules: f.EnabledModules.Value(),
-	}, nil
+	}
+	return nil
 }