From 3c6c52614d49090abb88d7d5970707b8487c4c56 Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Mon, 30 Sep 2024 12:24:31 +0530 Subject: [PATCH 01/15] feat: Enable golangci --- .github/workflows/golangci.lint.yml | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .github/workflows/golangci.lint.yml diff --git a/.github/workflows/golangci.lint.yml b/.github/workflows/golangci.lint.yml new file mode 100644 index 0000000..5932589 --- /dev/null +++ b/.github/workflows/golangci.lint.yml @@ -0,0 +1,31 @@ +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 + - 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: ${{ env.GO_VERSION }} + - name: Run golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + version: v1.58 + args: --timeout=5m + From 1a2a66bb7bf8c9fd9583de877fe979e2458cc6ff Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Mon, 30 Sep 2024 12:30:15 +0530 Subject: [PATCH 02/15] feat: Added golang lint rules --- .golangci.yml | 62 +++++++++++++++++++++++++++++++++++++++ cmd/convert.go | 3 +- cmd/root_test.go | 1 - cmd/utils.go | 2 -- conf/config_marshaller.go | 1 - conf/config_validator.go | 5 +--- conf/loader.go | 7 ----- testutils/utils.go | 1 - 8 files changed, 64 insertions(+), 18 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..e5626fc --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,62 @@ + +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 + - exportloopref + - 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' + + diff --git a/cmd/convert.go b/cmd/convert.go index 353dbd2..172dfb4 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -183,9 +183,8 @@ func newConvertCmd() *cobra.Command { } logger.Debugf("Writing converted data to: %s", outputPath) - _, err = outFile.Write([]byte(out)) + _, err = outFile.WriteString(out) return err - }, PreRunE: func(cmd *cobra.Command, args []string) error { if len(args) > convertArgMax { diff --git a/cmd/root_test.go b/cmd/root_test.go index 7982ab0..0f1d424 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -160,7 +160,6 @@ func (suite *RootTest) TestPersistentPreRunRootInitConfig() { bool2, err := flagSet2.GetBool("bool2") suite.NoError(err) suite.Equal(false, bool2) - }) } } diff --git a/cmd/utils.go b/cmd/utils.go index fb0b2e8..186b230 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -26,7 +26,6 @@ type metaDataArgs struct { } func genMetaDataText(src []byte, msg []byte, mdata map[string]string) ([]byte, error) { - metaHeader := "# *** Aerospike Metadata Generated by Asconfig ***" err := metadata.Unmarshal(src, mdata) @@ -117,7 +116,6 @@ func getConfFileFormat(path string, cmd *cobra.Command) (conf.Format, error) { 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/conf/config_marshaller.go b/conf/config_marshaller.go index 5cbcb82..0900f44 100644 --- a/conf/config_marshaller.go +++ b/conf/config_marshaller.go @@ -19,7 +19,6 @@ func NewConfigMarshaller(conf ConfHandler, format Format) ConfigMarshaller { } func (cm ConfigMarshaller) MarshalText() (text []byte, err error) { - switch cm.Format { case AsConfig: text = []byte(cm.ToConfFile()) diff --git a/conf/config_validator.go b/conf/config_validator.go index ccb279e..436b9dd 100644 --- a/conf/config_validator.go +++ b/conf/config_validator.go @@ -33,7 +33,6 @@ func NewConfigValidator(confHandler ConfHandler, mgmtLogger logr.Logger, 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{} @@ -45,7 +44,6 @@ func (cv *ConfigValidator) Validate() (*ValidationErrors, error) { } if !valid || err != nil || len(verrs.Errors) > 0 { - config := cv.ToMap() jsonConfigStr, err := json.Marshal(config) @@ -84,7 +82,7 @@ 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. @@ -120,7 +118,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 diff --git a/conf/loader.go b/conf/loader.go index 9f83fcd..1c02693 100644 --- a/conf/loader.go +++ b/conf/loader.go @@ -47,7 +47,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 +67,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 +109,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) @@ -150,7 +146,6 @@ Ex configMap } */ func typedContextsToObject(k string, v any, m configMap) { - if isTypedContext(k) { v := m[k] // if a typed context does not have a map value. @@ -166,7 +161,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 +189,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 { diff --git a/testutils/utils.go b/testutils/utils.go index 6df23a4..54db293 100644 --- a/testutils/utils.go +++ b/testutils/utils.go @@ -104,7 +104,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 From 67111de47b5d52f83c44f2be7a5dcc1c31fc51e1 Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Tue, 8 Oct 2024 22:36:28 +0530 Subject: [PATCH 03/15] feat: added lint to makefile --- .pre-commit-config.yaml | 15 +++++++++++++++ Makefile | 18 ++++++++++++++++++ go.mod | 4 +--- 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 .pre-commit-config.yaml 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..2177ad3 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,17 @@ 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.59.1 + +.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 --cache + # Clean up .PHONY: clean clean: diff --git a/go.mod b/go.mod index b49fdb4..00b9e98 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,6 @@ module github.com/aerospike/asconfig -go 1.21 - -toolchain go1.21.7 +go 1.22 require ( github.com/aerospike/aerospike-client-go/v7 v7.2.1 From a9946a17cbd5160ba65f3a350c24e0780a47e67e Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 12:48:09 +0530 Subject: [PATCH 04/15] feat: lint all the code --- .golangci.yml | 6 +- cmd/convert.go | 403 ++++++++++++++++++--------------- cmd/diff.go | 10 +- cmd/generate.go | 16 +- cmd/root.go | 14 +- cmd/root_test.go | 44 ++-- cmd/utils.go | 14 +- cmd/validate.go | 5 +- conf/config_handler.go | 2 +- conf/config_marshaller.go | 10 +- conf/config_validator.go | 22 +- conf/loader.go | 6 +- conf/metadata/metadata_test.go | 5 + schema/schemamap.go | 6 +- testutils/main/testgen.go | 78 ++++--- testutils/utils.go | 13 +- 16 files changed, 378 insertions(+), 276 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index e5626fc..71ee1e0 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -27,7 +27,7 @@ linters: - dogsled - dupl - errcheck - - exportloopref + - copyloopvar - exhaustive - goconst - gocritic @@ -59,4 +59,8 @@ run: issues-exit-code: 1 go: '1.22' +issues: + exclude-dirs: + - testdata + - .git diff --git a/cmd/convert.go b/cmd/convert.go index 172dfb4..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,226 +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 + } + + out = append(mtext, out...) - logger.Debugf("Writing converted data to: %s", outputPath) - _, err = outFile.WriteString(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 - } + } - if len(args) > 0 { - source := args[0] - if _, err := os.Stat(source); errors.Is(err, os.ErrNotExist) { - return errors.Join(errFileNotExist, err) - } - } + defer outFile.Close() + } - // validate flags - _, err := cmd.Flags().GetString("output") - if err != nil { - return err - } + logger.Debugf("Writing converted data to: %s", outputPath) - force, err := cmd.Flags().GetBool("force") - if err != nil { - return err - } + _, err = outFile.WriteString(string(out)) - // read stdin by default - var srcPath string - if len(args) == 0 { - srcPath = os.Stdin.Name() - } else { - srcPath = args[0] - } + return err +} - cfgData, err = os.ReadFile(srcPath) - if err != nil { - return err - } +func preRunConvertCmd(cmd *cobra.Command, args []string) error { + var cfgData []byte - 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) > convertArgMax { + return errTooManyArguments + } - av, err := cmd.Flags().GetString("aerospike-version") - if err != nil { - return err - } + if len(args) > 0 { + source := args[0] + if _, err := os.Stat(source); errors.Is(err, os.ErrNotExist) { + return errors.Join(errFileNotExist, 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 - } - } + // validate flags + _, err := cmd.Flags().GetString("output") + if err != nil { + return err + } + + 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 0f1d424..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,32 +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 186b230..d97e830 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -25,7 +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) @@ -42,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)) @@ -53,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 } @@ -78,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 } @@ -105,12 +109,12 @@ 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") 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 0900f44..fb6e3a7 100644 --- a/conf/config_marshaller.go +++ b/conf/config_marshaller.go @@ -8,13 +8,13 @@ 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, } } @@ -25,6 +25,8 @@ func (cm ConfigMarshaller) MarshalText() (text []byte, err error) { 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 436b9dd..7c2242f 100644 --- a/conf/config_validator.go +++ b/conf/config_validator.go @@ -17,16 +17,16 @@ 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, } } @@ -36,6 +36,7 @@ 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, @@ -59,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 @@ -86,7 +88,7 @@ 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) } @@ -178,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 1c02693..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) } @@ -145,7 +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. @@ -304,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 54db293..abc6cb7 100644 --- a/testutils/utils.go +++ b/testutils/utils.go @@ -9,8 +9,8 @@ import ( "os" "os/exec" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/image" "github.com/docker/docker/client" v1 "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -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 types.ImagePullOptions, 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, From 5f7ccb428e2e615aa92a346a83a8b6a45afaac4b Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 12:50:01 +0530 Subject: [PATCH 05/15] fix: no lint for schema folder --- .golangci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yml b/.golangci.yml index 71ee1e0..bc28dc6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -63,4 +63,5 @@ issues: exclude-dirs: - testdata - .git + - schema From 5acd212de7828021b679df9b8a8d2cd41573f5f2 Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 13:00:00 +0530 Subject: [PATCH 06/15] fix: run golang action with the same version as make file --- .github/workflows/golangci.lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci.lint.yml b/.github/workflows/golangci.lint.yml index 5932589..cca9e88 100644 --- a/.github/workflows/golangci.lint.yml +++ b/.github/workflows/golangci.lint.yml @@ -26,6 +26,6 @@ jobs: - name: Run golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.58 + version: v1.59.1 args: --timeout=5m From 6043d497ac44768553d6f25eb0b5f9eeb68e72a8 Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 13:05:17 +0530 Subject: [PATCH 07/15] fix: go version for linting --- .github/workflows/golangci.lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci.lint.yml b/.github/workflows/golangci.lint.yml index cca9e88..f969904 100644 --- a/.github/workflows/golangci.lint.yml +++ b/.github/workflows/golangci.lint.yml @@ -22,7 +22,7 @@ jobs: - name: Setup-go uses: actions/setup-go@v3 with: - go-version: ${{ env.GO_VERSION }} + go-version: 1.22 - name: Run golangci-lint uses: golangci/golangci-lint-action@v3 with: From 1d82976c1ff91771836c4018f46e74b27765aa81 Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 13:10:08 +0530 Subject: [PATCH 08/15] fix: enable verbose on golang-lint ghaction --- .github/workflows/golangci.lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci.lint.yml b/.github/workflows/golangci.lint.yml index f969904..eee98e6 100644 --- a/.github/workflows/golangci.lint.yml +++ b/.github/workflows/golangci.lint.yml @@ -27,5 +27,5 @@ jobs: uses: golangci/golangci-lint-action@v3 with: version: v1.59.1 - args: --timeout=5m + args: --timeout=5m -v From 0ace4086268a29116bd48fab74ae1d6f77bf4a0d Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 16:07:37 +0530 Subject: [PATCH 09/15] fix: using golangci-lint 1.60 --- .github/workflows/golangci.lint.yml | 8 +++----- Makefile | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/workflows/golangci.lint.yml b/.github/workflows/golangci.lint.yml index eee98e6..93f312c 100644 --- a/.github/workflows/golangci.lint.yml +++ b/.github/workflows/golangci.lint.yml @@ -23,9 +23,7 @@ jobs: uses: actions/setup-go@v3 with: go-version: 1.22 - - name: Run golangci-lint - uses: golangci/golangci-lint-action@v3 - with: - version: v1.59.1 - args: --timeout=5m -v + - name: Run Lint + run: | + make golanci-lint | make go-lint diff --git a/Makefile b/Makefile index 2177ad3..04d0b43 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,7 @@ $(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.59.1 +GOLANGCI_LINT_VERSION ?= v1.60.0 .PHONY: golanci-lint golanci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. From 4fc11a3ef4c8e76cae0222330519d672fd426d19 Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 16:09:38 +0530 Subject: [PATCH 10/15] fix: update to v1.61.0 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 04d0b43..2cda466 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,7 @@ $(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.60.0 +GOLANGCI_LINT_VERSION ?= v1.61.0 .PHONY: golanci-lint golanci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. From d46a6e242d958acb1003c2e893a79f0797a88c52 Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 16:10:41 +0530 Subject: [PATCH 11/15] fix: fixing the lint command in make file --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2cda466..b04d1b6 100644 --- a/Makefile +++ b/Makefile @@ -41,7 +41,7 @@ $(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 --cache + $(GOLANGCI_LINT) run # Clean up .PHONY: clean From 7858d3987015d15861e94ab29f4f8a2af50cae1b Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 16:12:27 +0530 Subject: [PATCH 12/15] fix: run golangci-lint with verbose --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b04d1b6..6122739 100644 --- a/Makefile +++ b/Makefile @@ -41,7 +41,7 @@ $(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 + $(GOLANGCI_LINT) run -c .golangci.yml -v # Clean up .PHONY: clean From fc9f0e1f60bc6890d32791b3069fe08bd1ebf1ea Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 16:38:06 +0530 Subject: [PATCH 13/15] fix: Ensure fresh linting --- .github/workflows/golangci.lint.yml | 6 +++++- .golangci.yml | 2 +- Makefile | 8 ++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/golangci.lint.yml b/.github/workflows/golangci.lint.yml index 93f312c..1a1f857 100644 --- a/.github/workflows/golangci.lint.yml +++ b/.github/workflows/golangci.lint.yml @@ -16,6 +16,8 @@ jobs: 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 @@ -25,5 +27,7 @@ jobs: go-version: 1.22 - name: Run Lint run: | - make golanci-lint | make go-lint + make remove-lint + make golanci-lint + make go-lint diff --git a/.golangci.yml b/.golangci.yml index bc28dc6..1d7f58f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -63,5 +63,5 @@ issues: exclude-dirs: - testdata - .git - - schema + - schema/schemas diff --git a/Makefile b/Makefile index 6122739..272e242 100644 --- a/Makefile +++ b/Makefile @@ -35,14 +35,22 @@ $(ACONFIG_BIN): $(SOURCES) GOLANGCI_LINT ?= $(GOBIN)/golangci-lint GOLANGCI_LINT_VERSION ?= v1.61.0 +# install golangci-lint .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) +# Run linting in verbose +.PHONY: go-lint go-lint: golanci-lint ## Run golangci-lint against code. $(GOLANGCI_LINT) run -c .golangci.yml -v +# Clean up golangci-lint +.PHONY: remove-lint +remove-lint: + $(RM) ${GOBIN}/golangci-lint + # Clean up .PHONY: clean clean: From 50ba55b7b34dfcbfa1efc4a7621cd4faa74f2abe Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 16:44:19 +0530 Subject: [PATCH 14/15] fix: Fix Test Cases --- conf/config_marshaller_test.go | 2 +- conf/config_validator_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conf/config_marshaller_test.go b/conf/config_marshaller_test.go index 8f4d0eb..9b10221 100644 --- a/conf/config_marshaller_test.go +++ b/conf/config_marshaller_test.go @@ -11,7 +11,7 @@ import ( func Test_asconf_MarshalText(t *testing.T) { type fields struct { - cfg ConfHandler + cfg Handler outFmt Format } tests := []struct { diff --git a/conf/config_validator_test.go b/conf/config_validator_test.go index 874e768..a073c82 100644 --- a/conf/config_validator_test.go +++ b/conf/config_validator_test.go @@ -13,7 +13,7 @@ import ( func Test_asconf_Validate(t *testing.T) { type fields struct { - cfg ConfHandler + cfg Handler logger *logrus.Logger managementLibLogger logr.Logger aerospikeVersion string From 714b49137892a0d2278a709d66b7345c859ce781 Mon Sep 17 00:00:00 2001 From: Akash Chandra Date: Wed, 9 Oct 2024 17:19:54 +0530 Subject: [PATCH 15/15] feat: Basic Refactoring --- cmd/convert.go | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/cmd/convert.go b/cmd/convert.go index e5c47b6..41961c5 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -35,23 +35,7 @@ func init() { var convertCmd = newConvertCmd() func newConvertCmd() *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{ + res := &cobra.Command{ Use: "convert [flags] ", Short: "Convert between yaml and Aerospike config format.", Long: `Convert is used to convert between yaml and aerospike configuration @@ -75,14 +59,27 @@ func createConvertCmd() *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: runCmdE, + PreRunE: runPreCmdE, } -} -func runConvertCmd(cmd *cobra.Command, args []string) error { - var cfgData []byte + // 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.") + + res.Version = VERSION + + return res +} +func runCmdE(cmd *cobra.Command, args []string) error { logger.Debug("Running convert command") + var cfgData []byte // read stdin by default var srcPath string if len(args) == 0 { @@ -120,7 +117,7 @@ func runConvertCmd(cmd *cobra.Command, args []string) error { case conf.YAML: outFmt = conf.AsConfig case conf.Invalid: - return fmt.Errorf("%w: %s", errInvalidFormat, srcFormat) + return fmt.Errorf("%w: %s", errInvalidFormat, srcPath) default: return fmt.Errorf("%w: %s", errInvalidFormat, srcFormat) } @@ -183,6 +180,7 @@ func runConvertCmd(cmd *cobra.Command, args []string) error { } outFileName = strings.TrimSuffix(outFileName, filepath.Ext(outFileName)) + outputPath = filepath.Join(outputPath, outFileName) switch outFmt { @@ -201,7 +199,7 @@ func runConvertCmd(cmd *cobra.Command, args []string) error { if outputPath == os.Stdout.Name() { outFile = os.Stdout } else { - outFile, err = os.OpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) + outFile, err = os.OpenFile(outputPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) if err != nil { return err } @@ -216,7 +214,7 @@ func runConvertCmd(cmd *cobra.Command, args []string) error { return err } -func preRunConvertCmd(cmd *cobra.Command, args []string) error { +func runPreCmdE(cmd *cobra.Command, args []string) error { var cfgData []byte if len(args) > convertArgMax {