From 239f91323b468831de5f621650c2e862e90a7cec Mon Sep 17 00:00:00 2001 From: Jesse Schmidt Date: Mon, 4 Dec 2023 13:16:39 -0800 Subject: [PATCH] ci: Fix linting in touched files --- .golangci.yml | 35 +++++++++++++ asconfig/asconfig_test.go | 4 -- asconfig/generate.go | 103 +++++++++++++++++++++++--------------- asconfig/generate_test.go | 41 +++++++++------ asconfig/schema.go | 7 ++- info/as_parser.go | 22 +++++--- 6 files changed, 147 insertions(+), 65 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 943156f..5a87f40 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -65,6 +65,41 @@ run: issues: exclude-rules: + - path: '(.+)test\.go' + linters: + - 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 + - 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 - linters: - lll source: "// " \ No newline at end of file diff --git a/asconfig/asconfig_test.go b/asconfig/asconfig_test.go index beef0e4..97de06e 100644 --- a/asconfig/asconfig_test.go +++ b/asconfig/asconfig_test.go @@ -77,8 +77,6 @@ func (suite *AsConfigTestSuite) TestAsConfigGetFlatMap() { }, } - Init(logr.Discard(), "/Users/jesseschmidt/Developer/aerospike-admin/lib/live_cluster/client/config-schemas") // TODO: replace with better location. Maybe a single test schema - for _, tc := range testCases { suite.Run(tc.name, func() { logger := logr.Discard() @@ -365,8 +363,6 @@ func (suite *AsConfigTestSuite) TestAsConfigGetExpandMap() { }, } - Init(logr.Discard(), "/Users/jesseschmidt/Developer/aerospike-admin/lib/live_cluster/client/config-schemas") // TODO: replace with better location. Maybe a single test schema - for _, tc := range testCases { suite.Run(tc.name, func() { logger := logr.Discard() diff --git a/asconfig/generate.go b/asconfig/generate.go index 711a79c..4bfba47 100644 --- a/asconfig/generate.go +++ b/asconfig/generate.go @@ -37,6 +37,7 @@ func isSupportedGenerateVersion(version string) (bool, error) { } cmp, err := lib.CompareVersions(version, "5.0.0") + return cmp >= 0, err } @@ -46,6 +47,7 @@ func isSupportedGenerateVersion(version string) (bool, error) { // default values are out of the acceptable range required by the server. func GenerateConf(log logr.Logger, confGetter ConfGetter, removeDefaults bool) (Conf, error) { log.V(1).Info("Generating config") + validConfig := Conf{} // Flatten the config returned from the server. Then convert it to a map @@ -111,15 +113,15 @@ func (p *pipeline) execute(conf Conf) error { // GetConfigStep is a pipeline step that retrieves the configs and metadata. type GetConfigStep struct { - log logr.Logger confGetter ConfGetter + log logr.Logger } // newGetConfigStep creates a new GetConfigStep with the provided log and ConfGetter. func newGetConfigStep(log logr.Logger, confGetter ConfGetter) *GetConfigStep { return &GetConfigStep{ - log: log, confGetter: confGetter, + log: log, } } @@ -140,26 +142,28 @@ func (s *GetConfigStep) execute(conf Conf) error { } conf["metadata"] = metadata["metadata"] + return nil } // ServerVersionCheckStep is a pipeline step that checks if the server version is supported. type ServerVersionCheckStep struct { - log logr.Logger checkFunc func(string) (bool, error) + log logr.Logger } // newServerVersionCheckStep creates a new ServerVersionCheckStep with the provided log and check function. func newServerVersionCheckStep(log logr.Logger, checkFunc func(string) (bool, error)) *ServerVersionCheckStep { return &ServerVersionCheckStep{ - log: log, checkFunc: checkFunc, + log: log, } } // execute checks if the server version is supported using the check function. func (s *ServerVersionCheckStep) execute(conf Conf) error { s.log.V(1).Info("Checking server version") + build := conf["metadata"].(Conf)["build"].(string) isSupported, err := s.checkFunc(build) @@ -208,23 +212,26 @@ func (s *copyEffectiveRackIDStep) execute(conf Conf) error { // For this ns find which rack this node belongs to for rack, nodesStr := range rackInfo { - if strings.Contains(nodesStr.(string), nodeID) { - rackIDStr := rackRegex.FindStringSubmatch(rack)[1] - if rackIDStr == "" { - return fmt.Errorf("unable to find rack id for rack %s", rack) - } + if !strings.Contains(nodesStr.(string), nodeID) { + continue + } - rackID, err := strconv.Atoi(rackIDStr) + rackIDStr := rackRegex.FindStringSubmatch(rack)[1] + if rackIDStr == "" { + return fmt.Errorf("unable to find rack id for rack %s", rack) + } - if err != nil { - return fmt.Errorf("unable to convert rack id %s to int", rackIDStr) - } + rackID, err := strconv.Atoi(rackIDStr) - // Copy effective rack-id over the ns config - key := fmt.Sprintf("namespaces.{%s}.rack-id", ns) - flatConfig[key] = rackID - break + if err != nil { + return fmt.Errorf("unable to convert rack id %s to int", rackIDStr) } + + // Copy effective rack-id over the ns config + key := fmt.Sprintf("namespaces.{%s}.rack-id", ns) + flatConfig[key] = rackID + + break } } @@ -246,6 +253,7 @@ func newRenameLoggingContextsStep(log logr.Logger) *renameKeysStep { // execute renames logging contexts in the config. func (s *renameKeysStep) execute(conf Conf) error { s.log.V(1).Info("Renaming keys") + config := conf["config"].(lib.Stats) logging, ok := config["logging"].(lib.Stats) @@ -260,12 +268,14 @@ func (s *renameKeysStep) execute(conf Conf) error { case lib.Stats: if key == "stderr" { newLoggingEntries["console"] = value + delete(logging, key) } else if !strings.HasSuffix(key, ".log") { - delete(logging, key) newLoggingEntries["syslog"] = v syslog := newLoggingEntries["syslog"].(lib.Stats) syslog["path"] = key + + delete(logging, key) } default: continue @@ -301,6 +311,7 @@ func sortKeys(config lib.Stats) []string { } sort.Strings(keys) + return keys } @@ -320,8 +331,8 @@ func convertDictToList(config lib.Stats) []lib.Stats { config2["name"] = key1 list[count] = config2 count++ - keys2 := sortKeys(config2) + keys2 := sortKeys(config2) for _, key2 := range keys2 { value := config2[key2] @@ -338,7 +349,7 @@ func convertDictToList(config lib.Stats) []lib.Stats { } // convertDictRespToConf converts a dictionary response to a Conf. -func convertDictRespToConf(log logr.Logger, config lib.Stats, sep string) { +func convertDictRespToConf(config lib.Stats) { if _, ok := config["logging"].(lib.Stats); ok { config["logging"] = convertDictToList(config["logging"].(lib.Stats)) } @@ -362,10 +373,13 @@ func convertDictRespToConf(log logr.Logger, config lib.Stats, sep string) { // execute flattens the config. func (s *flattenConfStep) execute(conf Conf) error { s.log.V(1).Info("Flattening config") + origConfig := conf["config"].(lib.Stats) - convertDictRespToConf(s.log, origConfig, sep) + + convertDictRespToConf(origConfig) conf["flat_config"] = flattenConf(s.log, conf["config"].(lib.Stats), sep) + return nil } @@ -381,9 +395,10 @@ func newTransformKeyValuesStep(log logr.Logger) *transformKeyValuesStep { } } -func splitContextBaseKey(key string) (string, string) { - bKey := baseKey(key) - contextKey := strings.TrimSuffix(key, sep+bKey) +func splitContextBaseKey(key string) (contextKey, bKey string) { + bKey = baseKey(key) + contextKey = strings.TrimSuffix(key, sep+bKey) + return contextKey, bKey } @@ -427,6 +442,7 @@ func sortedKeys(m map[string]interface{}) []string { } sort.Strings(keys) + return keys } @@ -436,39 +452,44 @@ func undefinedOrNull(val interface{}) bool { lower := strings.ToLower(str) return lower == "undefined" || lower == "null" } + return false } // convertIndexedToList converts an indexed key to a list key. It returns the // new key, the index, and the value as a string. If the key is not indexed or the value is // not a string, it returns empty strings. -func convertIndexedToList(key string, value interface{}) (string, string) { - if newKey, _, _, _ := parseIndexField(key); newKey != "" { - if strVal, ok := value.(string); ok { +func convertIndexedToList(key string, value interface{}) (newKey, strVal string) { + if newKey, _, _ = parseIndexField(key); newKey != "" { + if str, ok := value.(string); ok { + strVal = str return newKey, strVal } } - return "", "" + return newKey, strVal } -func parseIndexField(key string) (string, int, string, error) { +func parseIndexField(key string) (newKey string, index int, err error) { if match := indexedRe.FindStringSubmatch(key); match != nil { - index, err := strconv.Atoi(match[2]) + index, err = strconv.Atoi(match[2]) if err != nil { - return "", 0, "", err + return "", 0, err } - return match[1], index, match[3], nil + newKey = match[1] + + return newKey, index, nil } - return "", 0, "", nil + return newKey, index, nil } // execute transforms key values in the config. func (s *transformKeyValuesStep) execute(conf Conf) error { s.log.V(1).Info("Transforming key values") + origFlatConf := conf["flat_config"].(Conf) newFlatConf := make(Conf, len(origFlatConf)) // we will overwrite flat_config sortedKeys := sortedKeys(origFlatConf) @@ -498,7 +519,7 @@ func (s *transformKeyValuesStep) execute(conf Conf) error { newKey = getPluralKey(newKey) if strings.HasSuffix(key, "shadow") { - _, index, _, err := parseIndexField(key) + _, index, err := parseIndexField(key) if err != nil { return err } @@ -568,6 +589,7 @@ func newRemoveSecurityIfDisabledStep(log logr.Logger) *removeSecurityIfDisabledS // execute removes security configurations if security is disabled. func (s *removeSecurityIfDisabledStep) execute(conf Conf) error { s.log.V(1).Info("Removing security configs if security is disabled") + contexts := conf["flat_config"].(Conf) build := conf["metadata"].(Conf)["build"].(string) @@ -606,7 +628,7 @@ func newRemoveDefaultsStep(log logr.Logger) *removeDefaultsStep { } } -func compareDefaults(defVal interface{}, confVal interface{}) bool { +func compareDefaults(defVal, confVal interface{}) bool { switch val := defVal.(type) { case []interface{}: return reflect.DeepEqual(val, confVal) @@ -625,16 +647,16 @@ func compareDefaults(defVal interface{}, confVal interface{}) bool { if sliceVal, ok := confVal.([]string); ok && len(sliceVal) == 1 { return sliceVal[0] == val } + return defVal == confVal case uint64: // Schema deals with uint64 when positive but config deals with int - return uint64(val) == uint64(confVal.(int)) + return val == uint64(confVal.(int)) case int64: // Schema deals with int64 when negative but config deals with int - return uint64(val) == uint64(confVal.(int)) + return val == int64(confVal.(int)) default: return val == confVal - } } @@ -648,6 +670,7 @@ func defaultSlice(m map[string][]string, key string) []string { func (s *removeDefaultsStep) execute(conf Conf) error { s.log.V(1).Info("Removing default values") + flatConf := conf["flat_config"].(Conf) build := conf["metadata"].(Conf)["build"].(string) @@ -716,6 +739,7 @@ func (s *removeDefaultsStep) execute(conf Conf) error { for loggingContext, levels := range loggingMap { freq := "CRITICAL" count := 0 + for level, contexts := range levels { if len(contexts) > count { freq = level @@ -729,6 +753,7 @@ func (s *removeDefaultsStep) execute(conf Conf) error { flatConf[loggingContext+sep+"any"] = freq } + return nil } @@ -747,8 +772,8 @@ func newExpandConfStep(log logr.Logger) *expandConfStep { // execute expands the config. func (s *expandConfStep) execute(conf Conf) error { s.log.V(1).Info("Expanding config") - flatConf := conf["flat_config"].(Conf) + flatConf := conf["flat_config"].(Conf) expandedConf := expandConf(s.log, &flatConf, sep) conf["expanded_config"] = expandedConf diff --git a/asconfig/generate_test.go b/asconfig/generate_test.go index 1e70f0d..d83f44f 100644 --- a/asconfig/generate_test.go +++ b/asconfig/generate_test.go @@ -22,9 +22,9 @@ func (suite *GenerateTestSuite) SetupTest() { type GenerateTC struct { name string + removeDefaults bool allConfigs Conf metadata Conf - removeDefaults bool expected Conf } @@ -67,6 +67,7 @@ func TestGenerateTestSuiteSuite(t *testing.T) { var logging = GenerateTC{ "logging", + false, Conf{ "config": Conf{ "logging": Conf{ @@ -260,7 +261,7 @@ var logging = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - false, + Conf{ "logging": []Conf{ { @@ -459,6 +460,7 @@ var logging = GenerateTC{ var namespaceTC = GenerateTC{ "namespaces", + false, Conf{ "config": Conf{ "racks": []Conf{ @@ -654,7 +656,7 @@ var namespaceTC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - false, + Conf{ "namespaces": []Conf{ { @@ -847,6 +849,7 @@ var namespaceTC = GenerateTC{ var networkTC = GenerateTC{ "network", + false, Conf{ "config": Conf{ "network": Conf{ @@ -894,7 +897,7 @@ var networkTC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - false, + Conf{ "network": Conf{ "fabric": Conf{ @@ -951,6 +954,7 @@ var networkTC = GenerateTC{ var security57TC = GenerateTC{ "security post 5.7", + false, Conf{ "config": Conf{ "security": Conf{ @@ -967,7 +971,7 @@ var security57TC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - false, + Conf{ "security": Conf{ "enable-quotas": true, @@ -986,6 +990,7 @@ var security57TC = GenerateTC{ var security56TC = GenerateTC{ "security pre 5.7", + false, Conf{ "config": Conf{ "security": Conf{ @@ -1002,7 +1007,7 @@ var security56TC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "5.6.0.0", "node_id": "BB9030011AC4202"}}, - false, + Conf{ "security": Conf{ "enable-quotas": true, @@ -1022,6 +1027,7 @@ var security56TC = GenerateTC{ var serviceTC = GenerateTC{ "service", + false, Conf{ "config": Conf{ "service": Conf{ @@ -1079,7 +1085,7 @@ var serviceTC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - false, + Conf{ "service": Conf{ "advertise-ipv6": false, @@ -1138,6 +1144,7 @@ var serviceTC = GenerateTC{ var xdr5TC = GenerateTC{ "xdr5", + false, Conf{ "config": Conf{ "xdr": Conf{ @@ -1221,7 +1228,7 @@ var xdr5TC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - false, + Conf{ "xdr": Conf{ "dcs": []Conf{ @@ -1311,6 +1318,7 @@ var xdr5TC = GenerateTC{ // Same as above but remove defaults var loggingDefaultsTC = GenerateTC{ "logging with remove default", + true, Conf{ "config": Conf{ "logging": Conf{ @@ -1505,7 +1513,7 @@ var loggingDefaultsTC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - true, + Conf{ "logging": []Conf{ { @@ -1532,6 +1540,7 @@ var loggingDefaultsTC = GenerateTC{ var namespacesDefaultsTC = GenerateTC{ "namespaces with remove defaults", + true, Conf{ "config": Conf{ "racks": []Conf{ @@ -1727,7 +1736,7 @@ var namespacesDefaultsTC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - true, + Conf{ "namespaces": []Conf{ { @@ -1787,6 +1796,7 @@ var namespacesDefaultsTC = GenerateTC{ var networkDefaultsTC = GenerateTC{ "network with remove defaults", + true, Conf{ "config": Conf{ "network": Conf{ @@ -1834,7 +1844,7 @@ var networkDefaultsTC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - true, + Conf{ "network": Conf{ "fabric": Conf{ @@ -1861,6 +1871,7 @@ var networkDefaultsTC = GenerateTC{ var security57DefaultsTC = GenerateTC{ "security post 5.7 with remove defaults", + true, Conf{ "config": Conf{ "security": Conf{ @@ -1877,7 +1888,7 @@ var security57DefaultsTC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - true, + Conf{ "security": Conf{ "enable-quotas": true, @@ -1887,6 +1898,7 @@ var security57DefaultsTC = GenerateTC{ var serviceDefaultsTC = GenerateTC{ "service with remove defaults", + true, Conf{ "config": Conf{ "service": Conf{ @@ -1944,7 +1956,7 @@ var serviceDefaultsTC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - true, + Conf{ "service": Conf{ "cluster-name": "6.x-cluster-security", @@ -1961,6 +1973,7 @@ var serviceDefaultsTC = GenerateTC{ var xdr5DefaultsTC = GenerateTC{ "xdr5 with remove defaults", + true, Conf{ "config": Conf{ "xdr": Conf{ @@ -2044,7 +2057,7 @@ var xdr5DefaultsTC = GenerateTC{ }, }, Conf{"metadata": Conf{"build": "6.4.0.0", "node_id": "BB9030011AC4202"}}, - true, + Conf{ "xdr": Conf{ "dcs": []Conf{ diff --git a/asconfig/schema.go b/asconfig/schema.go index d7bf7dc..e204a3a 100644 --- a/asconfig/schema.go +++ b/asconfig/schema.go @@ -199,15 +199,17 @@ func getDefaultSchema(ver string) (map[string]interface{}, error) { if _, removed := removedKeys[key]; !removed { if val, ok := defMap[key]; ok { - switch val.(type) { + switch val := val.(type) { case []string: - if !reflect.DeepEqual(val.([]string), eval(v).([]string)) { + if !reflect.DeepEqual(val, eval(v).([]string)) { removedKeys[key] = true + delete(defMap, key) } default: if eval(v) != val { removedKeys[key] = true + delete(defMap, key) } } @@ -235,6 +237,7 @@ func getRequiredSchema(ver string) (map[string][][]string, error) { for _, k := range keys { v := flatSchema[k] + if reqRegex.MatchString(k) { key := removeJSONSpecKeywords(k) key = reqRegex.ReplaceAllString(key, "${1}") diff --git a/info/as_parser.go b/info/as_parser.go index 12070c4..2eb437a 100644 --- a/info/as_parser.go +++ b/info/as_parser.go @@ -145,7 +145,9 @@ func NewAsInfo(log logr.Logger, h *aero.Host, cp *aero.ClientPolicy) *AsInfo { return NewAsInfoWithConnFactory(log, h, cp, aeroConnFactory) } -func NewAsInfoWithConnFactory(log logr.Logger, h *aero.Host, cp *aero.ClientPolicy, connFact connectionFactory) *AsInfo { +func NewAsInfoWithConnFactory( + log logr.Logger, h *aero.Host, cp *aero.ClientPolicy, connFact connectionFactory, +) *AsInfo { logger := log.WithValues("node", h) return &AsInfo{ @@ -328,6 +330,7 @@ func (info *AsInfo) GetAsConfig(contextList ...string) (lib.Stats, error) { key := constConfigs configs, err := info.execute(info.log, rawCmdList, m, key) + if err != nil { return nil, fmt.Errorf( "failed to get config info from aerospike server: %v", err, @@ -607,8 +610,9 @@ func (info *AsInfo) createXDRConfigCmdList(build string, m map[string]string) ([ return nil, err } - m = mergeDicts(m, resp) var dcNames []string + + m = mergeDicts(m, resp) rawXDRConfig := resp[cmdConfigXDR] xdrConfig := parseIntoMap(rawXDRConfig, ";", "=") rawNames, ok := xdrConfig[constStatDCNames].(string) @@ -620,12 +624,14 @@ func (info *AsInfo) createXDRConfigCmdList(build string, m map[string]string) ([ } results := make(chan error, len(dcNames)) - var wg sync.WaitGroup + var wg sync.WaitGroup for _, dc := range dcNames { wg.Add(1) + go func(dc string) { defer wg.Done() + resp, err := info.doInfo(cmdConfigDC + dc) if err != nil { @@ -633,8 +639,9 @@ func (info *AsInfo) createXDRConfigCmdList(build string, m map[string]string) ([ return } - m = mergeDicts(m, resp) var nsNames []string + + m = mergeDicts(m, resp) rawDCConfig := resp[cmdConfigDC+dc] xdrConfig := parseIntoMap(rawDCConfig, ";", "=") rawNames, ok := xdrConfig[constStatNSNames].(string) @@ -984,6 +991,7 @@ func parseConfigInfo(rawMap map[string]string) lib.Stats { } else { xc = parseAllXDRConfig(rawMap, cmdConfigXDR) } + if len(xc) > 0 { configMap[ConfigXDRContext] = xc } @@ -1089,6 +1097,7 @@ func parseAllXDRConfig(rawMap map[string]string, cmd string) lib.Stats { dcNamesRaw := xdrConfigMap.TryString(constStatDCNames, "") dcNames := strings.Split(dcNamesRaw, ",") + delete(xdrConfigMap, constStatDCNames) xdrConfigMap[ConfigDCContext] = make(lib.Stats, len(dcNames)) @@ -1102,6 +1111,7 @@ func parseAllXDRConfig(rawMap map[string]string, cmd string) lib.Stats { xdrConfigMap[ConfigDCContext].(lib.Stats)[dc] = dcMap nsNamesRaw := dcMap.TryString(constStatNSNames, "") nsNames := strings.Split(nsNamesRaw, ",") + delete(dcMap, constStatNSNames) dcMap[ConfigNamespaceContext] = make(lib.Stats, len(nsNames)) @@ -1551,9 +1561,9 @@ func getParsedValue(val interface{}) interface{} { return value } else if value, err := strconv.ParseBool(valStr); err == nil { return value - } else { - return valStr } + + return valStr } func parseIntoListOfMap(str, del1, del2, sep string) []lib.Stats {