diff --git a/.github/workflows/golangci.lint.yml b/.github/workflows/golangci.lint.yml new file mode 100644 index 0000000..3f298c9 --- /dev/null +++ b/.github/workflows/golangci.lint.yml @@ -0,0 +1,34 @@ +name: golangci-lint +on: + push: + tags: + - v* + branches: + - main + pull_request: + branches: + - main + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - name: Checkout sources + uses: actions/checkout@v3 + with: + submodules: true + - name: Get go version from go.mod + run: | + echo "GO_VERSION=$(grep '^go ' go.mod | cut -d " " -f 2)" >> $GITHUB_ENV + - name: Setup-go + uses: actions/setup-go@v3 + with: + go-version: 1.22 + - name: Run Lint + run: | + make remove-lint + make golanci-lint + make go-lint + + diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..1d7f58f --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,67 @@ + +linters-settings: + goconst: + min-len: 2 + min-occurrences: 3 + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + govet: + enable: + - shadow + - fieldalignment + nolintlint: + require-explanation: true + require-specific: true + +linters: + disable-all: true + enable: + - bodyclose +# - unused # intentionally commented to avoid unused func warning as this repo is library +# - depguard + - dogsled + - dupl + - errcheck + - copyloopvar + - exhaustive + - goconst + - gocritic + - gofmt + - goimports + - gocyclo + - gosec + - gosimple +# - govet # intentionally disabled because we choose readability over performance + - ineffassign + - misspell + - nolintlint + - nakedret + - prealloc # pre-allocate slices with define size if the slice size is known in advance + - predeclared + - revive + - staticcheck + - stylecheck + - thelper + - tparallel + - typecheck + - unconvert + - unparam + - whitespace + - lll + - wsl # While space linter + +run: + issues-exit-code: 1 + go: '1.22' + +issues: + exclude-dirs: + - testdata + - .git + - schema/schemas + diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..e10e781 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,15 @@ +repos: + - repo: https://github.com/tekwizely/pre-commit-golang + rev: v1.0.0-rc.1 # change this to the latest version + hooks: + - id: go-mod-tidy + - id: golangci-lint + args: + - --config=.golangci.yml + + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.4.0 + hooks: + - id: check-merge-conflict + - id: end-of-file-fixer + - id: trailing-whitespace \ No newline at end of file diff --git a/Makefile b/Makefile index fb356c3..d384e30 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,10 @@ +# Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set) +ifeq (,$(shell go env GOBIN)) +GOBIN=$(shell go env GOPATH)/bin +else +GOBIN=$(shell go env GOBIN) +endif + # Variables required for this Makefile ROOT_DIR = $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST)))) VERSION = $(shell git describe --tags --always) @@ -25,6 +32,22 @@ SOURCES := $(shell find . -name "*.go") $(ACONFIG_BIN): $(SOURCES) $(GO_ENV_VARS) go build -ldflags="-X 'github.com/aerospike/asconfig/cmd.VERSION=$(VERSION)'" -o $(ACONFIG_BIN) . +GOLANGCI_LINT ?= $(GOBIN)/golangci-lint +GOLANGCI_LINT_VERSION ?= v1.61.0 + +.PHONY: golanci-lint +golanci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. +$(GOLANGCI_LINT): $(GOBIN) + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOBIN) $(GOLANGCI_LINT_VERSION) + +go-lint: golanci-lint ## Run golangci-lint against code. + $(GOLANGCI_LINT) run -c .golangci.yml -v + + +.PHONY: remove-lint +remove-lint: + $(RM) ${GOBIN}/golangci-lint + # Clean up .PHONY: clean clean: diff --git a/cmd/convert.go b/cmd/convert.go index 353dbd2..e5c47b6 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -35,8 +35,23 @@ func init() { var convertCmd = newConvertCmd() func newConvertCmd() *cobra.Command { - var cfgData []byte - res := &cobra.Command{ + cmd := createConvertCmd() + initConvertCmdFlags(cmd) + cmd.RunE = runConvertCmd + cmd.PreRunE = preRunConvertCmd + + return cmd +} +func initConvertCmdFlags(cmd *cobra.Command) { + asCommonFlags := getCommonFlags() + cmd.Flags().AddFlagSet(asCommonFlags) + cmd.Flags().BoolP("force", "f", false, "Override checks for supported server version and config validation") + cmd.Flags().StringP("output", "o", os.Stdout.Name(), "File path to write output to") + cmd.Flags().StringP("format", "F", "conf", "The format of the source file(s). Valid options are: yaml, yml, and conf.") +} + +func createConvertCmd() *cobra.Command { + return &cobra.Command{ Use: "convert [flags] ", Short: "Convert between yaml and Aerospike config format.", Long: `Convert is used to convert between yaml and aerospike configuration @@ -60,227 +75,238 @@ func newConvertCmd() *cobra.Command { Ex: asconfig convert -a "6.4.0" If the file has been converted by asconfig before, the --aerospike-version option is not needed. Ex: asconfig convert -a "6.4.0" aerospike.yaml | asconfig convert --format conf`, - RunE: func(cmd *cobra.Command, args []string) error { - logger.Debug("Running convert command") - - // read stdin by default - var srcPath string - if len(args) == 0 { - srcPath = os.Stdin.Name() - } else { - srcPath = args[0] - } + } +} - asVersion, err := cmd.Flags().GetString("aerospike-version") - if err != nil { - return err - } +func runConvertCmd(cmd *cobra.Command, args []string) error { + var cfgData []byte - logger.Debugf("Processing flag aerospike-version value=%s", asVersion) + logger.Debug("Running convert command") - force, err := cmd.Flags().GetBool("force") - if err != nil { - return err - } + // read stdin by default + var srcPath string + if len(args) == 0 { + srcPath = os.Stdin.Name() + } else { + srcPath = args[0] + } - logger.Debugf("Processing flag force value=%t", force) + asVersion, err := cmd.Flags().GetString("aerospike-version") + if err != nil { + return err + } - srcFormat, err := getConfFileFormat(srcPath, cmd) - if err != nil { - return err - } + logger.Debugf("Processing flag aerospike-version value=%s", asVersion) - logger.Debugf("Processing flag format value=%v", srcFormat) + force, err := cmd.Flags().GetBool("force") + if err != nil { + return err + } - var outFmt conf.Format - switch srcFormat { - case conf.AsConfig: - outFmt = conf.YAML - case conf.YAML: - outFmt = conf.AsConfig - default: - return fmt.Errorf("%w: %s", errInvalidFormat, srcFormat) - } + logger.Debugf("Processing flag force value=%t", force) - // if the version option is empty, - // try populating from the metadata - if asVersion == "" { - asVersion, err = getMetaDataItem(cfgData, metaKeyAerospikeVersion) - if err != nil && !force { - return errors.Join(errMissingAerospikeVersion, err) - } - } + srcFormat, err := getConfFileFormat(srcPath, cmd) + if err != nil { + return err + } - // load - asconfig, err := conf.NewASConfigFromBytes(mgmtLibLogger, cfgData, srcFormat) + logger.Debugf("Processing flag format value=%v", srcFormat) - if err != nil { - return err - } + var outFmt conf.Format - // validate - if !force { - verrs, err := conf.NewConfigValidator(asconfig, mgmtLibLogger, asVersion).Validate() - if err != nil || verrs != nil { - return errors.Join(err, verrs) - } - } + switch srcFormat { + case conf.AsConfig: + outFmt = conf.YAML + case conf.YAML: + outFmt = conf.AsConfig + case conf.Invalid: + return fmt.Errorf("%w: %s", errInvalidFormat, srcFormat) + default: + return fmt.Errorf("%w: %s", errInvalidFormat, srcFormat) + } - // convert - out, err := conf.NewConfigMarshaller(asconfig, outFmt).MarshalText() - if err != nil { - return err - } + // if the version option is empty, + // try populating from the metadata + if asVersion == "" { + asVersion, err = getMetaDataItem(cfgData, metaKeyAerospikeVersion) + if err != nil && !force { + return errors.Join(errMissingAerospikeVersion, err) + } + } - // prepend metadata to the config output - mtext, err := genMetaDataText( - cfgData, - nil, - map[string]string{ - metaKeyAerospikeVersion: asVersion, - metaKeyAsconfigVersion: VERSION, - }, - ) - if err != nil { - return err - } - out = append(mtext, out...) + // load + asConfig, err := conf.NewASConfigFromBytes(mgmtLibLogger, cfgData, srcFormat) - outputPath, err := cmd.Flags().GetString("output") - if err != nil { - return err - } + if err != nil { + return err + } - if stat, err := os.Stat(outputPath); !errors.Is(err, os.ErrNotExist) && stat.IsDir() { - // output path is a directory so write a new file to it - outFileName := filepath.Base(srcPath) - if srcPath == os.Stdin.Name() { - outFileName = defaultOutputFileName - } - - outFileName = strings.TrimSuffix(outFileName, filepath.Ext(outFileName)) - - outputPath = filepath.Join(outputPath, outFileName) - if outFmt == conf.YAML { - outputPath += ".yaml" - } else if outFmt == conf.AsConfig { - outputPath += ".conf" - } else { - return fmt.Errorf("output format unrecognized %w", errInvalidFormat) - } - } + // validate + if !force { + verrs, err := conf.NewConfigValidator(asConfig, mgmtLibLogger, asVersion).Validate() + if err != nil || verrs != nil { + return errors.Join(err, verrs) + } + } - var outFile *os.File - if outputPath == os.Stdout.Name() { - outFile = os.Stdout - } else { - outFile, err = os.OpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) - if err != nil { - return err - } + // convert + out, err := conf.NewConfigMarshaller(asConfig, outFmt).MarshalText() + if err != nil { + return err + } - defer outFile.Close() - } + // prepend metadata to the config output + mtext, err := genMetaDataText( + cfgData, + nil, + map[string]string{ + metaKeyAerospikeVersion: asVersion, + metaKeyAsconfigVersion: VERSION, + }, + ) + if err != nil { + return err + } - logger.Debugf("Writing converted data to: %s", outputPath) - _, err = outFile.Write([]byte(out)) + out = append(mtext, out...) + + outputPath, err := cmd.Flags().GetString("output") + if err != nil { + return err + } + + if stat, err := os.Stat(outputPath); !errors.Is(err, os.ErrNotExist) && stat.IsDir() { + // output path is a directory so write a new file to it + outFileName := filepath.Base(srcPath) + if srcPath == os.Stdin.Name() { + outFileName = defaultOutputFileName + } + + outFileName = strings.TrimSuffix(outFileName, filepath.Ext(outFileName)) + outputPath = filepath.Join(outputPath, outFileName) + + switch outFmt { + case conf.YAML: + outputPath += ".yaml" + case conf.AsConfig: + outputPath += ".conf" + case conf.Invalid: + return fmt.Errorf("output format unrecognized %w", errInvalidFormat) + default: + return fmt.Errorf("output format unrecognized %w", errInvalidFormat) + } + } + + var outFile *os.File + if outputPath == os.Stdout.Name() { + outFile = os.Stdout + } else { + outFile, err = os.OpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) + if err != nil { return err + } - }, - PreRunE: func(cmd *cobra.Command, args []string) error { - if len(args) > convertArgMax { - return errTooManyArguments - } + defer outFile.Close() + } - if len(args) > 0 { - source := args[0] - if _, err := os.Stat(source); errors.Is(err, os.ErrNotExist) { - return errors.Join(errFileNotExist, err) - } - } + logger.Debugf("Writing converted data to: %s", outputPath) - // validate flags - _, err := cmd.Flags().GetString("output") - if err != nil { - return err - } + _, err = outFile.WriteString(string(out)) - force, err := cmd.Flags().GetBool("force") - if err != nil { - return err - } + return err +} - // read stdin by default - var srcPath string - if len(args) == 0 { - srcPath = os.Stdin.Name() - } else { - srcPath = args[0] - } +func preRunConvertCmd(cmd *cobra.Command, args []string) error { + var cfgData []byte - cfgData, err = os.ReadFile(srcPath) - if err != nil { - return err - } + if len(args) > convertArgMax { + return errTooManyArguments + } - metaData := map[string]string{} - metadata.Unmarshal(cfgData, metaData) - - // if the aerospike server version is in the cfg file's - // metadata, don't mark --aerospike-version as required - var aeroVersionRequired bool - if _, ok := metaData[metaKeyAerospikeVersion]; !ok { - if !force { - cmd.MarkFlagRequired("aerospike-version") - aeroVersionRequired = true - } - } + if len(args) > 0 { + source := args[0] + if _, err := os.Stat(source); errors.Is(err, os.ErrNotExist) { + return errors.Join(errFileNotExist, err) + } + } - av, err := cmd.Flags().GetString("aerospike-version") - if err != nil { - return err - } + // validate flags + _, err := cmd.Flags().GetString("output") + if err != nil { + return err + } - if aeroVersionRequired { - if av == "" { - return errors.Join(errMissingAerospikeVersion, err) - } - - supported, err := asconfig.IsSupportedVersion(av) - if err != nil { - return errors.Join(errInvalidAerospikeVersion, err) - } - - // TODO include valid versions in the error message - // asconfig lib needs a getSupportedVersions func - if !supported { - return errUnsupportedAerospikeVersion - } - } + force, err := cmd.Flags().GetBool("force") + if err != nil { + return err + } - formatString, err := cmd.Flags().GetString("format") + // read stdin by default + var srcPath string + if len(args) == 0 { + srcPath = os.Stdin.Name() + } else { + srcPath = args[0] + } + + cfgData, err = os.ReadFile(srcPath) + if err != nil { + return err + } + + metaData := map[string]string{} + err = metadata.Unmarshal(cfgData, metaData) + + if err != nil { + return err + } + + // if the aerospike server version is in the cfg file's + // metadata, don't mark --aerospike-version as required + var aeroVersionRequired bool + + if _, ok := metaData[metaKeyAerospikeVersion]; !ok { + if !force { + err = cmd.MarkFlagRequired("aerospike-version") if err != nil { - return errors.Join(errMissingFormat, err) + return err } - _, err = ParseFmtString(formatString) - if err != nil && formatString != "" { - return errors.Join(errInvalidFormat, err) - } + aeroVersionRequired = true + } + } - return nil - }, + av, err := cmd.Flags().GetString("aerospike-version") + if err != nil { + return err } - // flags and configuration settings - // aerospike-version is marked required in this cmd's PreRun if the --force flag is not provided - asCommonFlags := getCommonFlags() - res.Flags().AddFlagSet(asCommonFlags) - res.Flags().BoolP("force", "f", false, "Override checks for supported server version and config validation") - res.Flags().StringP("output", "o", os.Stdout.Name(), "File path to write output to") - res.Flags().StringP("format", "F", "conf", "The format of the source file(s). Valid options are: yaml, yml, and conf.") + if aeroVersionRequired { + if av == "" { + return errors.Join(errMissingAerospikeVersion, err) + } + + supported, err := asconfig.IsSupportedVersion(av) + if err != nil { + return errors.Join(errInvalidAerospikeVersion, err) + } + + // TODO include valid versions in the error message + // asconfig lib needs a getSupportedVersions func + if !supported { + return errUnsupportedAerospikeVersion + } + } - res.Version = VERSION + formatString, err := cmd.Flags().GetString("format") + if err != nil { + return errors.Join(errMissingFormat, err) + } + + _, err = ParseFmtString(formatString) + if err != nil && formatString != "" { + return errors.Join(errInvalidFormat, err) + } - return res + return nil } diff --git a/cmd/diff.go b/cmd/diff.go index e52eb80..44f8b86 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -124,7 +124,7 @@ func newDiffCmd() *cobra.Command { // diffFlatMaps reports differences between flattened config maps // this only works for maps 1 layer deep as produced by the management // lib's flattenConf function -func diffFlatMaps(m1 map[string]any, m2 map[string]any) []string { +func diffFlatMaps(m1, m2 map[string]any) []string { var res []string allKeys := map[string]struct{}{} @@ -136,10 +136,14 @@ func diffFlatMaps(m1 map[string]any, m2 map[string]any) []string { allKeys[k] = struct{}{} } - var keysList []string + keysList := make([]string, len(allKeys)) + i := 0 + for k := range allKeys { - keysList = append(keysList, k) + keysList[i] = k + i++ } + sort.Strings(keysList) for _, k := range keysList { diff --git a/cmd/generate.go b/cmd/generate.go index 58b3afd..7d166b1 100644 --- a/cmd/generate.go +++ b/cmd/generate.go @@ -34,8 +34,10 @@ func newGenerateCmd() *cobra.Command { res := &cobra.Command{ Use: "generate [flags]", Short: "BETA: Generate a configuration file from a running Aerospike node.", - Long: `BETA: Generate a configuration file from a running Aerospike node. This can be useful if you have changed the configuration of a node dynamically (e.g. xdr) and would like to persist the changes.`, - RunE: func(cmd *cobra.Command, args []string) error { + Long: `BETA: Generate a configuration file from a running Aerospike node. \ + This can be useful if you have changed the configuration of a node dynamically (e.g. xdr) \ + and would like to persist the changes.`, + RunE: func(cmd *cobra.Command, _ []string) error { logger.Debug("Running generate command") outputPath, err := cmd.Flags().GetString("output") @@ -95,7 +97,7 @@ func newGenerateCmd() *cobra.Command { if outputPath == os.Stdout.Name() { outFile = os.Stdout } else { - outFile, err = os.OpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + outFile, err = os.OpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) if err != nil { return err } @@ -107,7 +109,8 @@ func newGenerateCmd() *cobra.Command { _, err = outFile.Write(fdata) logger.Warning( - "Community Edition is not supported. Generated static configuration does not save logging.syslog, mod-lua, service.user and service.group", + `Community Edition is not supported. \ + Generated static configuration does not save logging.syslog, mod-lua, service.user and service.group`, ) logger.Warning( "This feature is currently in beta. Use at your own risk and please report any issue to support.", @@ -146,7 +149,10 @@ func newGenerateCmd() *cobra.Command { config.BindPFlags(asFlagSet, "cluster") res.Flags().StringP("output", "o", os.Stdout.Name(), flags.DefaultWrapHelpString("File path to write output to")) - res.Flags().StringP("format", "F", "conf", flags.DefaultWrapHelpString("The format of the destination file(s). Valid options are: yaml, yml, and conf.")) + res.Flags().StringP( + "format", "F", "conf", + flags.DefaultWrapHelpString("The format of the destination file(s). Valid options are: yaml, yml, and conf."), + ) return res } diff --git a/cmd/root.go b/cmd/root.go index aa5f984..a82642e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -24,7 +24,7 @@ var rootCmd = newRootCmd() var ( VERSION = "development" // Replaced at compile time - errInvalidLogLevel = fmt.Errorf("Invalid log-level flag") + errInvalidLogLevel = fmt.Errorf("invalid log-level flag") aerospikeFlags = flags.NewDefaultAerospikeFlags() cfFileFlags = flags.NewConfFileFlags() ) @@ -35,7 +35,7 @@ func newRootCmd() *cobra.Command { Use: "asconfig", Short: "Manage Aerospike configuration", Long: "Asconfig is used to manage Aerospike configuration.", - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { var multiErr error cfgFile, err := config.InitConfig(cfFileFlags.File, cfFileFlags.Instance, cmd.Flags()) @@ -75,6 +75,7 @@ func newRootCmd() *cobra.Command { rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { logger.Error(err) cmd.Println(cmd.UsageString()) + return errors.Join(err, ErrSilent) }) @@ -94,6 +95,7 @@ func Execute() { logger.Error(err) } } + os.Exit(1) } } @@ -104,12 +106,12 @@ var mgmtLibLogger logr.Logger func init() { logger = logrus.New() - fmt := logrus.TextFormatter{} - fmt.FullTimestamp = true + logFmt := logrus.TextFormatter{} + logFmt.FullTimestamp = true - logger.SetFormatter(&fmt) + logger.SetFormatter(&logFmt) - schemaMap, err := schema.NewSchemaMap() + schemaMap, err := schema.NewMap() if err != nil { panic(err) } diff --git a/cmd/root_test.go b/cmd/root_test.go index 7982ab0..10d9805 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -35,12 +35,18 @@ func TestPersistentPreRunRootFlags(t *testing.T) { }, }, } + cmd := newRootCmd() for _, tc := range testCases { t.Run(tc.flags[0], func(t *testing.T) { - cmd.ParseFlags(tc.flags) - err := cmd.PersistentPreRunE(cmd, tc.arguments) + err := cmd.ParseFlags(tc.flags) + if err != nil { + t.Fatalf("unable to parse flags: %v", err) + } + + err = cmd.PersistentPreRunE(cmd, tc.arguments) + for _, expectedErr := range tc.expectedErrors { if !errors.Is(err, expectedErr) { t.Errorf("%v\n actual err: %v\n is not expected err: %v", tc.flags, err, expectedErr) @@ -78,7 +84,7 @@ type RootTest struct { suite.Suite } -func (suite *RootTest) TestPersistentPreRunRootInitConfig() { +func (tsuite *RootTest) TestPersistentPreRunRootInitConfig() { testCases := []struct { configFile string configFileTxt string @@ -97,7 +103,7 @@ func (suite *RootTest) TestPersistentPreRunRootInitConfig() { rootCmd := newRootCmd() subCmd := &cobra.Command{ Use: "sub", - Run: func(cmd *cobra.Command, args []string) {}, + Run: func(_ *cobra.Command, _ []string) {}, } flagSet1 := &pflag.FlagSet{} flagSet2 := &pflag.FlagSet{} @@ -118,12 +124,12 @@ func (suite *RootTest) TestPersistentPreRunRootInitConfig() { } for _, tc := range testCases { - suite.T().Run(tc.configFile, func(t *testing.T) { + tsuite.T().Run(tc.configFile, func(t *testing.T) { config.Reset() rootCmd, flagSet1, flagSet2 := createCmd() - err := os.WriteFile(tc.configFile, []byte(tc.configFileTxt), 0600) + err := os.WriteFile(tc.configFile, []byte(tc.configFileTxt), 0o600) if err != nil { t.Fatalf("unable to write %s: %v", tc.configFile, err) } @@ -134,33 +140,32 @@ func (suite *RootTest) TestPersistentPreRunRootInitConfig() { err = rootCmd.Execute() if err != nil { - suite.FailNow("unexpected error", err) + tsuite.FailNow("unexpected error", err) } str1, err := flagSet1.GetString("str1") - suite.NoError(err) - suite.Equal("localhost:3000", str1) + tsuite.NoError(err) + tsuite.Equal("localhost:3000", str1) int1, err := flagSet1.GetInt("int1") - suite.NoError(err) - suite.Equal(3000, int1) + tsuite.NoError(err) + tsuite.Equal(3000, int1) bool1, err := flagSet1.GetBool("bool1") - suite.NoError(err) - suite.Equal(true, bool1) + tsuite.NoError(err) + tsuite.Equal(true, bool1) str2, err := flagSet2.GetString("str2") - suite.NoError(err) - suite.Equal("localhost:4000", str2) + tsuite.NoError(err) + tsuite.Equal("localhost:4000", str2) int2, err := flagSet2.GetInt("int2") - suite.NoError(err) - suite.Equal(4000, int2) + tsuite.NoError(err) + tsuite.Equal(4000, int2) bool2, err := flagSet2.GetBool("bool2") - suite.NoError(err) - suite.Equal(false, bool2) - + tsuite.NoError(err) + tsuite.Equal(false, bool2) }) } } diff --git a/cmd/utils.go b/cmd/utils.go index fb0b2e8..d97e830 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -25,8 +25,7 @@ type metaDataArgs struct { asconfigVersion string } -func genMetaDataText(src []byte, msg []byte, mdata map[string]string) ([]byte, error) { - +func genMetaDataText(src, msg []byte, mdata map[string]string) ([]byte, error) { metaHeader := "# *** Aerospike Metadata Generated by Asconfig ***" err := metadata.Unmarshal(src, mdata) @@ -43,7 +42,7 @@ func genMetaDataText(src []byte, msg []byte, mdata map[string]string) ([]byte, e strMsg := string(msg) if len(msg) > 0 { - strMsg = strMsg + "\n#\n" + strMsg += "\n#\n" } mtext = []byte(fmt.Sprintf("%s\n%s%s%s\n\n", metaHeader, strMsg, mtext, metaFooter)) @@ -54,6 +53,7 @@ func genMetaDataText(src []byte, msg []byte, mdata map[string]string) ([]byte, e func getMetaDataItemOptional(src []byte, key string) (string, error) { mdata := map[string]string{} err := metadata.Unmarshal(src, mdata) + if err != nil { return "", err } @@ -79,7 +79,10 @@ func getMetaDataItem(src []byte, key string) (string, error) { // common flags func getCommonFlags() *pflag.FlagSet { res := &pflag.FlagSet{} - res.StringP("aerospike-version", "a", "", "Aerospike server version to validate the configuration file for. Ex: 6.2.0.\nThe first 3 digits of the Aerospike version number are required.\nThis option is required unless --force is used.") + res.StringP("aerospike-version", "a", "", + "Aerospike server version to validate the configuration file for. Ex: 6.2.0.\n"+ + "The first 3 digits of the Aerospike version number are required.\n"+ + "This option is required unless --force is used.") return res } @@ -106,18 +109,17 @@ func getConfFileFormat(path string, cmd *cobra.Command) (conf.Format, error) { fmtStr = ext } - fmt, err := ParseFmtString(fmtStr) + confFmt, err := ParseFmtString(fmtStr) if err != nil { return conf.Invalid, err } - return fmt, nil + return confFmt, nil } var ErrSilent = errors.New("SILENT") func ParseFmtString(in string) (f conf.Format, err error) { - switch strings.ToLower(in) { case "yaml", "yml": f = conf.YAML diff --git a/cmd/validate.go b/cmd/validate.go index 5d1cdf3..1e04fc6 100644 --- a/cmd/validate.go +++ b/cmd/validate.go @@ -68,7 +68,10 @@ func newValidateCmd() *cobra.Command { // if the Aerospike server version was not in the file // metadata, require that it is passed as an argument if version == "" { - cmd.MarkFlagRequired("aerospike-version") + err = cmd.MarkFlagRequired("aerospike-version") + if err != nil { + return err + } } versionArg, err := cmd.Flags().GetString("aerospike-version") diff --git a/conf/config_handler.go b/conf/config_handler.go index 6f55f7b..086e3f2 100644 --- a/conf/config_handler.go +++ b/conf/config_handler.go @@ -5,7 +5,7 @@ import ( "github.com/go-logr/logr" ) -type ConfHandler interface { +type Handler interface { IsValid(log logr.Logger, version string) (bool, []*asconfig.ValidationErr, error) ToMap() *asconfig.Conf ToConfFile() asconfig.DotConf diff --git a/conf/config_marshaller.go b/conf/config_marshaller.go index 5cbcb82..fb6e3a7 100644 --- a/conf/config_marshaller.go +++ b/conf/config_marshaller.go @@ -8,24 +8,25 @@ import ( type ConfigMarshaller struct { Format Format - ConfHandler + Handler } -func NewConfigMarshaller(conf ConfHandler, format Format) ConfigMarshaller { +func NewConfigMarshaller(conf Handler, format Format) ConfigMarshaller { return ConfigMarshaller{ - Format: format, - ConfHandler: conf, + Format: format, + Handler: conf, } } func (cm ConfigMarshaller) MarshalText() (text []byte, err error) { - switch cm.Format { case AsConfig: text = []byte(cm.ToConfFile()) case YAML: m := cm.ToMap() text, err = yaml.Marshal(m) + case Invalid: + err = fmt.Errorf("%w %s", ErrInvalidFormat, cm.Format) default: err = fmt.Errorf("%w %s", ErrInvalidFormat, cm.Format) } diff --git a/conf/config_validator.go b/conf/config_validator.go index ccb279e..7c2242f 100644 --- a/conf/config_validator.go +++ b/conf/config_validator.go @@ -17,26 +17,26 @@ var ( ) type ConfigValidator struct { - ConfHandler + Handler mgmtLogger logr.Logger version string } -func NewConfigValidator(confHandler ConfHandler, mgmtLogger logr.Logger, version string) *ConfigValidator { +func NewConfigValidator(confHandler Handler, mgmtLogger logr.Logger, version string) *ConfigValidator { return &ConfigValidator{ - ConfHandler: confHandler, - mgmtLogger: mgmtLogger, - version: version, + Handler: confHandler, + mgmtLogger: mgmtLogger, + version: version, } } // Validate validates the parsed configuration against the schema for the given versions. // ValidationErrors is not nil if any errors occur during validation. func (cv *ConfigValidator) Validate() (*ValidationErrors, error) { - valid, tempVerrs, err := cv.IsValid(cv.mgmtLogger, cv.version) verrs := ValidationErrors{} + for _, v := range tempVerrs { verr := ValidationErr{ ValidationErr: *v, @@ -45,7 +45,6 @@ func (cv *ConfigValidator) Validate() (*ValidationErrors, error) { } if !valid || err != nil || len(verrs.Errors) > 0 { - config := cv.ToMap() jsonConfigStr, err := json.Marshal(config) @@ -61,6 +60,7 @@ func (cv *ConfigValidator) Validate() (*ValidationErrors, error) { for i, verr := range verrs.Errors { context, _ := strings.CutPrefix(verr.Context, "(root).") context, err := jsonToConfigContext(jsonConfig, context) + if err != nil { // if we can't associate the error with its // corresponding field, just use the current context @@ -84,11 +84,11 @@ type VErrSlice []ValidationErr func (a VErrSlice) Len() int { return len(a) } func (a VErrSlice) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a VErrSlice) Less(i, j int) bool { return strings.Compare(a[i].Error(), a[j].Error()) == -1 } +func (a VErrSlice) Less(i, j int) bool { return a[i].Error() < a[j].Error() } // Outputs a human readable string of validation error details. // error is not nil if validation, or any other type of error occurs. -func (o ValidationErr) Error() string { +func (o *ValidationErr) Error() string { verrTemplate := "description: %s, error-type: %s" return fmt.Sprintf(verrTemplate, o.Description, o.ErrType) } @@ -120,7 +120,6 @@ func (o ValidationErrors) Error() string { errList := errorsByContext[ctx] for _, err := range errList { - // filter "Must validate one and only one schema " errors // I have never seen a useful one and they seem to always be // accompanied by another more useful error that will be displayed @@ -181,12 +180,12 @@ func jsonToConfigContext(jsonConfig any, context string) (string, error) { } // name should be a string - if nameStr, ok := name.(string); !ok { + nameStr, ok := name.(string) + if !ok { return "", fmt.Errorf("name is not a string in json config at: %v", context) - } else { - // set res to the name instead of the index - res = nameStr } + // set res to the name instead of the index + res = nameStr // set jsonConfig to the indexed object jsonConfig = indexedMap diff --git a/conf/loader.go b/conf/loader.go index 9f83fcd..f682a6f 100644 --- a/conf/loader.go +++ b/conf/loader.go @@ -14,6 +14,7 @@ import ( func NewASConfigFromBytes(log logr.Logger, src []byte, srcFmt Format) (*asconfig.AsConfig, error) { var err error + var cfg *asconfig.AsConfig switch srcFmt { @@ -21,6 +22,8 @@ func NewASConfigFromBytes(log logr.Logger, src []byte, srcFmt Format) (*asconfig cfg, err = loadYAML(log, src) case AsConfig: cfg, err = loadAsConf(log, src) + case Invalid: + return nil, fmt.Errorf("%w %s", ErrInvalidFormat, srcFmt) default: return nil, fmt.Errorf("%w %s", ErrInvalidFormat, srcFmt) } @@ -47,7 +50,6 @@ func NewASConfigFromBytes(log logr.Logger, src []byte, srcFmt Format) (*asconfig } func loadYAML(log logr.Logger, src []byte) (*asconfig.AsConfig, error) { - var data map[string]any err := yaml.Unmarshal(src, &data) @@ -68,7 +70,6 @@ func loadYAML(log logr.Logger, src []byte) (*asconfig.AsConfig, error) { } func loadAsConf(log logr.Logger, src []byte) (*asconfig.AsConfig, error) { - reader := bytes.NewReader(src) // TODO: Why doesn't the management lib do the map mutation? FromConfFile @@ -111,9 +112,7 @@ type mapping func(k string, v any, m configMap) // mutateMap maps functions to each key value pair in the management lib's Stats map // the functions are applied sequentially to each k,v pair. func mutateMap(in configMap, funcs []mapping) { - for k, v := range in { - switch v := v.(type) { case configMap: mutateMap(v, funcs) @@ -149,8 +148,7 @@ Ex configMap } } */ -func typedContextsToObject(k string, v any, m configMap) { - +func typedContextsToObject(k string, _ any, m configMap) { if isTypedContext(k) { v := m[k] // if a typed context does not have a map value. @@ -166,7 +164,6 @@ func typedContextsToObject(k string, v any, m configMap) { // isTypedContext returns true for asconfig contexts // that can map to strings instead of contexts func isTypedContext(in string) bool { - switch in { case "storage-engine", "index-type", "sindex-type": return true @@ -195,7 +192,6 @@ Ex configMap } */ func toPlural(k string, v any, m configMap) { - // convert asconfig fields/contexts that need to be plural // in order to create valid asconfig yaml. if plural := asconfig.PluralOf(k); plural != k { @@ -311,6 +307,7 @@ func sortLists(k string, v any, m configMap) { panic("unexpected gt value") } }) + m[k] = v } } diff --git a/conf/metadata/metadata_test.go b/conf/metadata/metadata_test.go index 9df2aab..adddff8 100644 --- a/conf/metadata/metadata_test.go +++ b/conf/metadata/metadata_test.go @@ -61,6 +61,7 @@ func TestUnmarshal(t *testing.T) { src []byte dst map[string]string } + tests := []struct { name string args args @@ -118,6 +119,7 @@ func TestUnmarshal(t *testing.T) { if err := metadata.Unmarshal(tt.args.src, tt.args.dst); (err != nil) != tt.wantErr { t.Errorf("Unmarshal() error = %v, wantErr %v", err, tt.wantErr) } + if !reflect.DeepEqual(tt.args.dst, tt.want) { t.Errorf("Unmarshal() = %v, want %v", tt.args.dst, tt.want) } @@ -139,6 +141,7 @@ func TestMarshalText(t *testing.T) { type args struct { src map[string]string } + tests := []struct { name string args args @@ -175,6 +178,7 @@ func TestMarshalText(t *testing.T) { want: []byte(testMarshalMetaPartial), }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := metadata.Marshal(tt.args.src) @@ -182,6 +186,7 @@ func TestMarshalText(t *testing.T) { t.Errorf("Marshal() error = %v, wantErr %v", err, tt.wantErr) return } + if !reflect.DeepEqual(got, tt.want) { t.Errorf("Marshal() = %v, want %v", got, tt.want) } diff --git a/schema/schemamap.go b/schema/schemamap.go index 08cb239..55b369e 100644 --- a/schema/schemamap.go +++ b/schema/schemamap.go @@ -12,10 +12,10 @@ import ( //go:embed schemas/json/aerospike var schemas embed.FS -type SchemaMap map[string]string +type Map map[string]string -func NewSchemaMap() (SchemaMap, error) { - schema := make(SchemaMap) +func NewMap() (Map, error) { + schema := make(Map) if err := fs.WalkDir( schemas, ".", func(path string, d fs.DirEntry, err error) error { diff --git a/testutils/main/testgen.go b/testutils/main/testgen.go index ec13b41..85b2ce3 100644 --- a/testutils/main/testgen.go +++ b/testutils/main/testgen.go @@ -52,7 +52,7 @@ func (o *obfuscateEntry) obfuscateLine(line []byte) ([]byte, error) { func obfuscateNamespaceCallback(o *obfuscateEntry, line []byte) ([]byte, error) { chop := len(strconv.Itoa(namespacesSeen)) - namespacesSeen = namespacesSeen + 1 + namespacesSeen++ o.value = o.value[:len(o.value)-chop] + strconv.Itoa(namespacesSeen) return line, nil @@ -60,7 +60,7 @@ func obfuscateNamespaceCallback(o *obfuscateEntry, line []byte) ([]byte, error) func obfuscateMountCallback(o *obfuscateEntry, line []byte) ([]byte, error) { chop := len(strconv.Itoa(mountSeen)) - mountSeen = mountSeen + 1 + mountSeen++ o.value = o.value[:len(o.value)-chop] + strconv.Itoa(mountSeen) return line, nil @@ -68,7 +68,7 @@ func obfuscateMountCallback(o *obfuscateEntry, line []byte) ([]byte, error) { func obfuscateSetCallback(o *obfuscateEntry, line []byte) ([]byte, error) { chop := len(strconv.Itoa(setSeen)) - setSeen = setSeen + 1 + setSeen++ o.value = o.value[:len(o.value)-chop] + strconv.Itoa(setSeen) return line, nil @@ -76,7 +76,7 @@ func obfuscateSetCallback(o *obfuscateEntry, line []byte) ([]byte, error) { func obfuscateDcCallback(o *obfuscateEntry, line []byte) ([]byte, error) { chop := len(strconv.Itoa(dcSeen)) - dcSeen = dcSeen + 1 + dcSeen++ o.value = o.value[:len(o.value)-chop] + strconv.Itoa(dcSeen) return line, nil @@ -84,7 +84,7 @@ func obfuscateDcCallback(o *obfuscateEntry, line []byte) ([]byte, error) { func obfuscateDeviceCallback(o *obfuscateEntry, line []byte) ([]byte, error) { chop := len(strconv.Itoa(deviceSeen)) - deviceSeen = deviceSeen + 1 + deviceSeen++ o.value = o.value[:len(o.value)-chop] + strconv.Itoa(deviceSeen) return line, nil @@ -92,7 +92,7 @@ func obfuscateDeviceCallback(o *obfuscateEntry, line []byte) ([]byte, error) { func obfuscateFilePathCallback(o *obfuscateEntry, line []byte) ([]byte, error) { chop := len(strconv.Itoa(fileSeen)) - fileSeen = fileSeen + 1 + fileSeen++ o.value = o.value[:len(o.value)-chop] + strconv.Itoa(fileSeen) return line, nil @@ -100,7 +100,7 @@ func obfuscateFilePathCallback(o *obfuscateEntry, line []byte) ([]byte, error) { func obfuscateTLSClusterName(o *obfuscateEntry, line []byte) ([]byte, error) { chop := len(strconv.Itoa(tlsSeen)) - tlsSeen = tlsSeen + 1 + tlsSeen++ o.value = o.value[:len(o.value)-chop] + strconv.Itoa(tlsSeen) return line, nil @@ -153,7 +153,7 @@ var obfuscateThese = []*obfuscateEntry{ cb: obfuscateDeviceCallback, }, { - pattern: regexp.MustCompile(`(file\s+)([^{][\S]+)`), + pattern: regexp.MustCompile(`(file\s+)([^{]\S+)`), value: "/dummy/file/path1", cb: obfuscateFilePathCallback, }, @@ -281,17 +281,18 @@ var obfuscateThese = []*obfuscateEntry{ }, } -var namespacesSeen int = 1 -var mountSeen int = 1 -var setSeen int = 1 -var dcSeen int = 1 -var deviceSeen int = 1 -var fileSeen int = 1 -var tlsSeen int = 1 +var namespacesSeen = 1 +var mountSeen = 1 +var setSeen = 1 +var dcSeen = 1 +var deviceSeen = 1 +var fileSeen = 1 +var tlsSeen = 1 func processFileData(in io.Reader) (io.Reader, error) { processedData := bytes.Buffer{} scanner := bufio.NewScanner(in) + for scanner.Scan() { tmp := scanner.Bytes() // inefficient but this is just a test gen tool @@ -310,6 +311,7 @@ func processFileData(in io.Reader) (io.Reader, error) { for _, obfs := range obfuscateThese { var err error line, err = obfs.obfuscateLine(line) + if err != nil { return nil, err } @@ -329,11 +331,27 @@ func main() { output := flag.String("output", "./testdata/cases", "path to output directory") overwrite := flag.Bool("overwrite", false, "if a testcase directory already exists for this input, overwrite it") obfuscate = flag.Bool("obfuscate", false, "obfuscate sensitive fields in the copied source config file") - aerospikeVersion := flag.String("aerospike-version", "6.2.0.2", "the aerospike version to pass to asconfig e.g: 6.2.0.2") - originalVersion := flag.String("original-version", "6.2.0.2", "the aerospike version that was originally used with this config e.g: 6.2.0.2") - serverImage := flag.String("server-image", "", "url to an Aerospike image to use. Overrides aerospike-version") - dockerUser := flag.String("docker-user", "", "Environment variable that holds a username used to authenticate with docker repositories during image pulls") - dockerPass := flag.String("docker-pass", "", "Environment variable that holds a base64 password or ID token used to authenticate with docker repositories during image pulls") + aerospikeVersion := flag.String( + "aerospike-version", + "6.2.0.2", + "the aerospike version to pass to asconfig e.g: 6.2.0.2") + originalVersion := flag.String( + "original-version", + "6.2.0.2", + "the aerospike version that was originally used with this config e.g: 6.2.0.2") + serverImage := flag.String( + "server-image", + "", + "url to an Aerospike image to use. Overrides aerospike-version") + dockerUser := flag.String( + "docker-user", + "", + "Environment variable that holds a username used to authenticate with docker repositories during image pulls") + dockerPass := flag.String( + "docker-pass", + "", + `Environment variable that holds a base64 password or ID token used to authenticate \ + with docker repositories during image pulls`) flag.Parse() @@ -362,7 +380,7 @@ func main() { } } - err := os.Mkdir(testCasePath, 0755) + err := os.Mkdir(testCasePath, 0o755) if err != nil { log.Fatalf("failed to create directory %s", testCasePath) } @@ -372,7 +390,6 @@ func main() { if err != nil { log.Fatalf("failed to open %s", inputPath) } - defer r.Close() processedFile, err := processFileData(r) if err != nil { @@ -381,10 +398,10 @@ func main() { copiedSrcPath := filepath.Join(testCasePath, filepath.Base(inputPath)) w, err := os.Create(copiedSrcPath) + if err != nil { log.Fatalf("failed to create %s", copiedSrcPath) } - defer w.Close() _, err = w.ReadFrom(processedFile) if err != nil { @@ -413,6 +430,7 @@ func main() { cmd := exec.Command("asconfig", args...) out, err := cmd.CombinedOutput() + if err != nil { log.Fatalf("command failed to run %v, %+v, out: %s", cmd, err, string(out)) } @@ -452,7 +470,8 @@ func main() { } yamlTestPath := filepath.Join(testCasePath, "yaml-tests.json") - err = os.WriteFile(yamlTestPath, data, 0655) + err = os.WriteFile(yamlTestPath, data, 0o600) + if err != nil { log.Fatalf("failed to write to %s", yamlTestPath) } @@ -485,7 +504,8 @@ func main() { } confTestPath := filepath.Join(testCasePath, "conf-tests.json") - err = os.WriteFile(confTestPath, data, 0655) + err = os.WriteFile(confTestPath, data, 0o600) + if err != nil { log.Fatalf("failed to write to %s", confTestPath) } @@ -498,11 +518,17 @@ func main() { } data, err = json.Marshal(vs) + if err != nil { + log.Fatalf("failed to marshal versions to json, %v", err) + } - err = os.WriteFile(versionsPath, data, 0655) + err = os.WriteFile(versionsPath, data, 0o600) if err != nil { log.Fatalf("failed to write to %s", versionsPath) } + r.Close() + w.Close() + fmt.Println("Done") } diff --git a/testutils/utils.go b/testutils/utils.go index 2693e3f..abc6cb7 100644 --- a/testutils/utils.go +++ b/testutils/utils.go @@ -68,16 +68,23 @@ func RemoveAerospikeContainer(id string, cli *client.Client) error { return nil } -func CreateAerospikeContainer(name string, c *container.Config, ch *container.HostConfig, imagePullOpts image.PullOptions, cli *client.Client) (string, error) { +func CreateAerospikeContainer(name string, c *container.Config, ch *container.HostConfig, + imagePullOpts image.PullOptions, cli *client.Client) (string, error) { ctx := context.Background() reader, err := cli.ImagePull(ctx, name, imagePullOpts) + if err != nil { log.Printf("Unable to pull image %s: %s", name, err) return "", err } defer reader.Close() - io.Copy(os.Stdout, reader) + _, err = io.Copy(os.Stdout, reader) + + if err != nil { + log.Printf("Unable to copy image pull output: %s", err) + return "", err + } platform := &v1.Platform{ Architecture: imagePullOpts.Platform, @@ -104,7 +111,6 @@ func StartAerospikeContainer(id string, cli *client.Client) error { } func IndexOf(l []string, s string) int { - for i, e := range l { if e == s { return i