Merge pull request #26 from sugarraysam/sblaisdo-disable-validators

Add a --disable/--enabled flag to validate cmd.

Makefile

@@ -20,12 +20,12 @@ all: build
 ##@ Development
 
 test: ## Run tests.
-	@go test -count=1 $(PKGS)
+	@go test -count=1 -race $(PKGS)
 
 
 test-e2e: ## Run e2e integration tests
 	@CGO_ENABLED=1 go build -mod=vendor -a -o $(E2E_MTCLI_PATH) cmd/mtcli/main.go
-	@E2E_MTCLI_PATH=$(E2E_MTCLI_PATH) go test $(INTEGRATION_TESTS)
+	@E2E_MTCLI_PATH=$(E2E_MTCLI_PATH) go test -count=1 -race $(INTEGRATION_TESTS)
 
 check: golangci-lint goimports ## Runs all checks.
 	@go fmt $(PKGS) $(INTEGRATION_TESTS)

internal/cmd/validate.go

@@ -18,25 +18,30 @@ import (
 func init() {
 	validateCmd.Flags().StringVar(&validateEnv, "env", validateEnv, "integration, stage or production")
 	validateCmd.Flags().StringVar(&validateVersion, "version", validateVersion, "addon imageset version")
+	validateCmd.Flags().StringVar(&validateDisabled, "disabled", validateDisabled, "Disable specific validators, separated by ','. Can't be combined with --enabled.")
+	validateCmd.Flags().StringVar(&validateEnabled, "enabled", validateEnabled, "Enable specific validators, separated by ','. Can't be combined with --disabled.")
 	mtcli.AddCommand(validateCmd)
 }
 
 var (
 	validateEnv      = "stage"
 	validateVersion  = ""
+	validateDisabled = ""
+	validateEnabled  = ""
 	validateExamples = []string{
 		"  # Validate an addon in staging. Uses the latest version if it supports imageset.",
 		"  mtcli validate --env stage --version latest internal/testdata/addons-imageset/reference-addon",
 		"  # Validate a version 1.0.0 of a production addon using imageset.",
 		"  mtcli validate --env production --version 1.0.0 <path/to/addon_dir>",
 		"  # Validate a staging addon that is not using imageset, but a static indexImage.",
 		"  mtcli validate --env stage <path/to/addon_dir>",
+		"  # Validate an integration addon using imageset, disabling validators 001_foo and 002_bar.",
+		"  mtcli validate --env integration --disabled 001_foo,002_bar <path/to/addon_dir>",
+		"  # Validate an integration addon using imageset, enabled only 001_foo.",
+		"  mtcli validate --env integration --enabled 001_foo <path/to/addon_dir>",
 	}
-	validateLong = `
-Validate an addon metadata against custom validators and the managed-tenants-cli JSON schema:
-    https://github.com/mt-sre/managed-tenants-cli/blob/main/docs/tenants/zz_schema_generated.md.
-	`
-	validateCmd = &cobra.Command{
+	validateLong = "Validate an addon metadata and it's bundles against custom validators."
+	validateCmd  = &cobra.Command{
 		Use:     "validate",
 		Short:   "Validate addon metadata, bundles and imagesets.",
 		Long:    validateLong,
@@ -70,7 +75,11 @@ func validateMain(cmd *cobra.Command, args []string) {
 	}
 
 	metaBundle := utils.NewMetaBundle(meta, bundles)
-	success, errs := validate.Validate(*metaBundle)
+	filter, err := validate.NewFilter(validateDisabled, validateEnabled)
+	if err != nil {
+		log.Fatal(err)
+	}
+	success, errs := validate.Validate(*metaBundle, filter)
 	if len(errs) > 0 {
 		utils.PrintValidationErrors(errs)
 		os.Exit(1)

pkg/validate/validate.go

@@ -1,21 +1,22 @@
 package validate
 
 import (
+	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/mt-sre/addon-metadata-operator/pkg/validators"
 
 	"github.com/mt-sre/addon-metadata-operator/pkg/utils"
 )
 
-func Validate(mb utils.MetaBundle) (bool, []error) {
+func Validate(mb utils.MetaBundle, filter *validatorsFilter) (bool, []error) {
 	errs := []error{}
 	allSuccess := true
-	validators := GetAllValidators()
 
 	printMetaHeading()
 
-	for _, validator := range validators {
+	for _, validator := range filter.GetValidators() {
 		fmt.Printf("\r%s\t\t", validator.Description)
 		success, err := validator.Runner(mb)
 		if err != nil {
@@ -32,19 +33,104 @@ func Validate(mb utils.MetaBundle) (bool, []error) {
 	return allSuccess, errs
 }
 
-func GetAllValidators() []utils.Validator {
-	return []utils.Validator{
-		{
-			Description: "Ensure defaultChannel is present in list of channels",
-			Runner:      validators.ValidateDefaultChannel,
-		},
-		{
-			Description: "Ensure `label` to follow the format api.openshift.com/addon-<operator-id>",
-			Runner:      validators.ValidateAddonLabel,
-		},
-		{
-			Description: "Ensure current csv is present in the index image",
-			Runner:      validators.ValidateCSVPresent,
-		},
+// name formatting rule: [0-9]{3}_([a-z]*_?)*
+var AllValidators = map[string]utils.Validator{
+	"001_default_channel": {
+		Description: "Ensure defaultChannel is present in list of channels",
+		Runner:      validators.ValidateDefaultChannel,
+	},
+	"002_label_format": {
+		Description: "Ensure `label` to follow the format api.openshift.com/addon-<operator-id>",
+		Runner:      validators.ValidateAddonLabel,
+	},
+	"003_csv_present": {
+		Description: "Ensure current csv is present in the index image",
+		Runner:      validators.ValidateCSVPresent,
+	},
+}
+
+func getExistingValidatorNames() []string {
+	var res []string
+	for name := range AllValidators {
+		res = append(res, name)
+	}
+	return res
+}
+
+type validatorsFilter struct {
+	AllEnabled     bool
+	ValidatorNames []string
+}
+
+func NewFilter(disabled, enabled string) (*validatorsFilter, error) {
+	// empty filter - all validators are enabled
+	if disabled == "" && enabled == "" {
+		return &validatorsFilter{AllEnabled: true}, nil
+	}
+
+	if err := verifyDisabledEnabled(disabled, enabled); err != nil {
+		return nil, err
+	}
+
+	var validatorNames []string
+	if enabled != "" {
+		validatorNames = strings.Split(enabled, ",")
+	} else {
+		validatorNames = getEnabledValidatorNamesFromDisabled(disabled)
+	}
+
+	return &validatorsFilter{AllEnabled: false, ValidatorNames: validatorNames}, nil
+}
+
+func verifyDisabledEnabled(disabled, enabled string) error {
+	// error: mutually exclusive
+	if disabled != "" && enabled != "" {
+		return errors.New("Can't set both --disabled and --enabled. They are mutually exclusive.")
+	}
+	var rawNames string
+	if enabled != "" {
+		rawNames = enabled
+	} else {
+		rawNames = disabled
+	}
+
+	validNames := strings.Join(getExistingValidatorNames(), ",")
+	for _, name := range strings.Split(rawNames, ",") {
+		if _, ok := AllValidators[name]; !ok {
+			return fmt.Errorf("Could not find validator with name %v. Existing validators are %v.", name, validNames)
+		}
+	}
+	return nil
+}
+
+func getEnabledValidatorNamesFromDisabled(disabled string) []string {
+	var res []string
+
+	allDisabled := make(map[string]bool)
+	for _, disabledName := range strings.Split(disabled, ",") {
+		allDisabled[disabledName] = true
+	}
+
+	for _, name := range getExistingValidatorNames() {
+		if _, ok := allDisabled[name]; !ok {
+			res = append(res, name)
+		}
+	}
+	return res
+}
+
+func (f *validatorsFilter) GetValidators() []utils.Validator {
+	var res []utils.Validator
+	if f.AllEnabled {
+		for _, validator := range AllValidators {
+			res = append(res, validator)
+		}
+	} else {
+		for _, name := range f.ValidatorNames {
+			// no need to track 'ok' as it was already validated by NewFilter func
+			validator := AllValidators[name]
+			res = append(res, validator)
+		}
 	}
+	return res
 }

pkg/validate/validate_test.go

@@ -1,6 +1,7 @@
 package validate
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/mt-sre/addon-metadata-operator/pkg/utils"
@@ -22,7 +23,7 @@ var validatorsToTest []Validator = []Validator{
 	validators.ValidatorDefaultChannelTestBundle{},
 }
 
-func Test_AllValidators(t *testing.T) {
+func TestAllValidators(t *testing.T) {
 	for _, validator := range validatorsToTest {
 		validator := validator
 		t.Run(validator.Name(), func(t *testing.T) {
@@ -44,3 +45,104 @@ func Test_AllValidators(t *testing.T) {
 		})
 	}
 }
+
+func TestFilterDisabledValidators(t *testing.T) {
+	n_validators := len(AllValidators)
+
+	cases := []struct {
+		name     string
+		disabled []string
+	}{
+		{
+			name:     "all_enabled",
+			disabled: []string{},
+		},
+		{
+			name:     "disable_default_channel",
+			disabled: []string{"001_default_channel"},
+		},
+		{
+			name:     "disable_all",
+			disabled: []string{"001_default_channel", "002_label_format", "003_csv_present"},
+		},
+	}
+	for _, tc := range cases {
+		tc := tc // pin
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			filter, err := NewFilter(strings.Join(tc.disabled, ","), "")
+			require.NoError(t, err)
+
+			n_enabled := len(filter.GetValidators())
+			n_disabled := len(tc.disabled)
+			require.Equal(t, n_enabled+n_disabled, n_validators)
+		})
+	}
+}
+
+func TestFilterEnabledValidators(t *testing.T) {
+	cases := []struct {
+		name    string
+		enabled []string
+	}{
+		{
+			name:    "enable_default_channel",
+			enabled: []string{"001_default_channel"},
+		},
+		{
+			name:    "enable_two",
+			enabled: []string{"001_default_channel", "002_label_format"},
+		},
+	}
+	for _, tc := range cases {
+		tc := tc // pin
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			filter, err := NewFilter("", strings.Join(tc.enabled, ","))
+			require.NoError(t, err)
+			require.Equal(t, len(filter.GetValidators()), len(tc.enabled))
+		})
+	}
+}
+
+func TestEmptyFilterAllEnabled(t *testing.T) {
+	t.Parallel()
+	filter, err := NewFilter("", "")
+	require.NoError(t, err)
+	require.Equal(t, len(filter.GetValidators()), len(AllValidators))
+}
+
+func TestFilterError(t *testing.T) {
+	cases := []struct {
+		name     string
+		enabled  []string
+		disabled []string
+	}{
+		{
+			name:     "mutually_exclusive",
+			enabled:  []string{"001_default_channel"},
+			disabled: []string{"001_default_channel"},
+		},
+		{
+			name:     "enabled_dont_exist",
+			enabled:  []string{"invalid"},
+			disabled: []string{},
+		},
+		{
+			name:     "disabled_dont_exist",
+			enabled:  []string{},
+			disabled: []string{"invalid"},
+		},
+	}
+	for _, tc := range cases {
+		tc := tc // pin
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			disabled := strings.Join(tc.disabled, ",")
+			enabled := strings.Join(tc.enabled, ",")
+			filter, err := NewFilter(disabled, enabled)
+			require.Error(t, err)
+			require.Nil(t, filter)
+		})
+	}
+}