diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..99b816d6 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,18 @@ +name: lint + +on: + pull_request: + branches: [main] + +jobs: + lint: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + + - name: make lint + run: | + go install honnef.co/go/tools/cmd/staticcheck@latest + make lint diff --git a/.github/workflows/lint-unit.yml b/.github/workflows/unit.yml similarity index 94% rename from .github/workflows/lint-unit.yml rename to .github/workflows/unit.yml index 25b377ed..72b03319 100644 --- a/.github/workflows/lint-unit.yml +++ b/.github/workflows/unit.yml @@ -1,4 +1,4 @@ -name: lint + unit tests +name: unit tests # About security when running the tests and NOT exposing # the secrets to externals. Currently Github Actions does @@ -35,9 +35,6 @@ jobs: [ -d $SOURCE ] && rm -r $SOURCE cp -r $GITHUB_WORKSPACE $SOURCE - - name: run golang linting - run: make check-format - - name: run unit tests run: make test diff --git a/Makefile b/Makefile index 43473830..28e4cfeb 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ MOUNT = -v $(PWD):$(WORKDIR) .PHONY: dist build clean ci-env build-rpm feature-tests format vendor -all: clean build test +all: clean build test lint dist: clean internal/connect/version.txt vendor @mkdir -p $(DIST)/build/packaging @@ -83,6 +83,11 @@ feature-tests: test-yast: build-so docker build -t go-connect-test-yast -f third_party/Dockerfile.yast . && docker run -t go-connect-test-yast +lint: internal/connect/version.txt + @gofmt -l internal/* cmd/* + @go vet ./... + @staticcheck ./... + clean: go clean @rm -f internal/connect/version.txt diff --git a/README.md b/README.md index f1a06577..e86700de 100644 --- a/README.md +++ b/README.md @@ -51,3 +51,6 @@ These values can be picked up from Glue's production environment. Once that is done, you can then simply run `make feature-tests`. This will run a all feature tests inside of a container by using the registration codes as provided by the `.env` file. + +Last but not least, you can run linters with `make lint` (requires `go vet` and +`staticcheck`) diff --git a/cmd/suseconnect/suseconnect.go b/cmd/suseconnect/suseconnect.go index cdbcfcdd..f2c05b52 100644 --- a/cmd/suseconnect/suseconnect.go +++ b/cmd/suseconnect/suseconnect.go @@ -36,7 +36,7 @@ func (p *singleStringFlag) String() string { func (p *singleStringFlag) Set(value string) error { if p.isSet { - return fmt.Errorf("this flag can only be specified once\n") + return fmt.Errorf("this flag can only be specified once") } p.value = value p.isSet = true @@ -127,7 +127,7 @@ func main() { connect.CFG.Load() if baseURL != "" { if err := validateURL(baseURL); err != nil { - fmt.Printf("URL \"%s\" not valid: %s\n", baseURL, err) + fmt.Printf("URL \"%s\" not valid: %s.\n", baseURL, err) os.Exit(1) } connect.CFG.BaseURL = baseURL @@ -293,8 +293,8 @@ func main() { func maybeBrokenSMTError() error { if !connect.URLDefault() && !connect.UpToDate() { - return fmt.Errorf("Your Registration Proxy server doesn't support this function. " + - "Please update it and try again.") + return fmt.Errorf("your Registration Proxy server doesn't support this function. " + + "Please update it and try again") } return nil } @@ -313,7 +313,7 @@ func exitOnError(err error) { } if je, ok := err.(connect.JSONError); ok { if err := maybeBrokenSMTError(); err != nil { - fmt.Println(err) + fmt.Printf("%v.", err.Error()) } else { fmt.Print("Error: Cannot parse response from server\n") fmt.Println(je) @@ -363,7 +363,7 @@ func validateURL(s string) error { return err } if u.Scheme == "" || u.Host == "" { - return fmt.Errorf("Missing scheme or host") + return fmt.Errorf("missing scheme or host") } return nil } diff --git a/cmd/zypper-migration/migration.go b/cmd/zypper-migration/migration.go index d70d4659..fd1b904e 100644 --- a/cmd/zypper-migration/migration.go +++ b/cmd/zypper-migration/migration.go @@ -661,7 +661,7 @@ func applyMigration(migration connect.MigrationPath, systemProducts []connect.Pr } if interrupted { - return fsInconsistent, fmt.Errorf("Preparing migration: %v", ErrInterrupted) + return fsInconsistent, fmt.Errorf("preparing migration: %v", ErrInterrupted) } // Disable all old repos in case of Leap -> SLES migration (bsc#1184237) @@ -685,7 +685,7 @@ func applyMigration(migration connect.MigrationPath, systemProducts []connect.Pr echo := util.SetSystemEcho(true) if err := zypper.RefreshRepos(baseProductVersion, true, false, false, false, autoImportRepoKeys); err != nil { - return fsInconsistent, fmt.Errorf("Refresh of repositories failed: %v", err) + return fsInconsistent, fmt.Errorf("refresh of repositories failed: %v", err) } if interrupted { return fsInconsistent, ErrInterrupted @@ -720,11 +720,11 @@ func checkFlagContradictions() error { flag.Visit(func(f *flag.Flag) { if strings.HasPrefix(f.Name, "no-") { if seen.Contains(f.Name[3:]) { - err = fmt.Errorf("Flags contradict: --%s and --%s", f.Name[3:], f.Name) + err = fmt.Errorf("flags contradict: --%s and --%s", f.Name[3:], f.Name) } } else { if seen.Contains("no-" + f.Name) { - err = fmt.Errorf("Flags contradict: --%s and --%s", f.Name, "no-"+f.Name) + err = fmt.Errorf("flags contradict: --%s and --%s", f.Name, "no-"+f.Name) } } seen.Add(f.Name) diff --git a/cmd/zypper-search-packages/search-packages.go b/cmd/zypper-search-packages/search-packages.go index a3ac1876..2d9e812a 100644 --- a/cmd/zypper-search-packages/search-packages.go +++ b/cmd/zypper-search-packages/search-packages.go @@ -387,7 +387,7 @@ func checkUnsupportedFlags() error { }) if reasons.Len() != 0 { - return fmt.Errorf("Cannot perform extended package search:\n\n%v", strings.Join(reasons.Strings(), "\n")) + return fmt.Errorf("cannot perform extended package search:\n\n%v", strings.Join(reasons.Strings(), "\n")) } return nil } diff --git a/internal/collectors/collectors.go b/internal/collectors/collectors.go index 7aaccaff..3504674d 100644 --- a/internal/collectors/collectors.go +++ b/internal/collectors/collectors.go @@ -58,9 +58,9 @@ func CollectInformation(architecture string, collectors []Collector) (Result, er // result set or is of different type func FromResult[R any](result Result, key string, def R) R { if value, ok := result[key]; ok { - switch value.(type) { + switch val := value.(type) { case R: - return value.(R) + return val default: return def } diff --git a/internal/collectors/lscpu.go b/internal/collectors/lscpu.go deleted file mode 100644 index 4d1f9cd4..00000000 --- a/internal/collectors/lscpu.go +++ /dev/null @@ -1,36 +0,0 @@ -package collectors - -import ( - "bufio" - "bytes" - "strings" - - "github.com/SUSE/connect-ng/internal/util" -) - -func lscpu() (map[string]string, error) { - output, err := util.Execute([]string{"lscpu"}, nil) - if err != nil { - return nil, err - } - - return lscpu2map(output), nil -} - -func lscpu2map(b []byte) map[string]string { - m := make(map[string]string) - scanner := bufio.NewScanner(bytes.NewReader(b)) - - for scanner.Scan() { - line := scanner.Text() - parts := strings.SplitN(line, ":", 2) - if len(parts) != 2 { - continue - } - - key, val := strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) - m[key] = val - } - - return m -} diff --git a/internal/collectors/sap_test.go b/internal/collectors/sap_test.go index 5e147524..8422dee5 100644 --- a/internal/collectors/sap_test.go +++ b/internal/collectors/sap_test.go @@ -9,10 +9,9 @@ import ( ) type MockDirEntry struct { - name string - isDir bool - fullPath string - info os.FileInfo + name string + isDir bool + info os.FileInfo } func (m MockDirEntry) Name() string { diff --git a/internal/collectors/uuid.go b/internal/collectors/uuid.go index 6e8fd620..e7ad6020 100644 --- a/internal/collectors/uuid.go +++ b/internal/collectors/uuid.go @@ -80,7 +80,7 @@ func machineID() (string, error) { u, err := uuid.Parse(string(bytes.TrimSpace(out))) if err != nil { - return "", fmt.Errorf("Unable to determine UUID for s390x: %v", err) + return "", fmt.Errorf("unable to determine UUID for s390x: %v", err) } return u.String(), nil diff --git a/internal/connect/activation.go b/internal/connect/activation.go index 697c335a..8a16f4e1 100644 --- a/internal/connect/activation.go +++ b/internal/connect/activation.go @@ -19,7 +19,3 @@ func (a Activation) toTriplet() string { p := a.Service.Product return p.Name + "/" + p.Version + "/" + p.Arch } - -func (a Activation) isFree() bool { - return a.Service.Product.Free -} diff --git a/internal/connect/client.go b/internal/connect/client.go index 356c7e20..460007aa 100644 --- a/internal/connect/client.go +++ b/internal/connect/client.go @@ -180,7 +180,7 @@ func Deregister(jsonOutput bool) error { if util.FileExists("/usr/sbin/registercloudguest") && CFG.Product.isEmpty() { return fmt.Errorf("SUSE::Connect::UnsupportedOperation: " + "De-registration is disabled for on-demand instances. " + - "Use `registercloudguest --clean` instead.") + "Use `registercloudguest --clean` instead") } if !IsRegistered() { diff --git a/internal/connect/client_test.go b/internal/connect/client_test.go index 293d1657..9903b431 100644 --- a/internal/connect/client_test.go +++ b/internal/connect/client_test.go @@ -36,21 +36,6 @@ func mockInstallReleasePackage(t *testing.T, expected bool) { } } -func mockRemoveOrRefreshService(t *testing.T, expected bool) { - counter := 0 - localRemoveOrRefreshService = func(Service, bool) error { - counter += 1 - return nil - } - - if !expected { - if counter > 1 { - t.Errorf("Expected removeOrRefreshService not to be called.") - } - } - -} - func mockMakeSysInfoBody(t *testing.T, expectedIncludeUptimeLog bool) { localMakeSysInfoBody = func(distroTarget, namespace string, instanceData []byte, includeUptimeLog bool) ([]byte, error) { if includeUptimeLog != expectedIncludeUptimeLog { diff --git a/internal/connect/connection.go b/internal/connect/connection.go index 615158a0..6043e2a2 100644 --- a/internal/connect/connection.go +++ b/internal/connect/connection.go @@ -200,7 +200,7 @@ func downloadFile(url string) ([]byte, error) { return nil, err } if !successCode(resp.StatusCode) { - return nil, fmt.Errorf("Downloading %s failed (code: %d): %s", url, resp.StatusCode, resBody) + return nil, fmt.Errorf("downloading %s failed (code: %d): %s", url, resp.StatusCode, resBody) } return resBody, nil } diff --git a/internal/connect/errors.go b/internal/connect/errors.go index 6ceced8d..128fd3d7 100644 --- a/internal/connect/errors.go +++ b/internal/connect/errors.go @@ -7,10 +7,10 @@ import ( // export errors that package main needs var ( - ErrSystemNotRegistered = errors.New("System not registered") - ErrPingFromUnregistered = errors.New("Keepalive ping not allowed from unregistered system.") - ErrBaseProductDeactivation = errors.New("Unable to deactivate base product") - ErrListExtensionsUnregistered = errors.New("System not registered") + ErrSystemNotRegistered = errors.New("system not registered") + ErrPingFromUnregistered = errors.New("keepalive ping not allowed from unregistered system") + ErrBaseProductDeactivation = errors.New("unable to deactivate base product") + ErrListExtensionsUnregistered = errors.New("system not registered") ) // APIError is returned on failed HTTP requests diff --git a/internal/connect/eula.go b/internal/connect/eula.go index fc3de182..155c8e0f 100644 --- a/internal/connect/eula.go +++ b/internal/connect/eula.go @@ -133,7 +133,7 @@ func fetchEULAFrom(extension Product) ([]byte, error) { // This should happen only if there are no license files in index if lang == "" { - return nil, fmt.Errorf("No EULAs found at: %s", extension.EULAURL) + return nil, fmt.Errorf("no EULAs found at: %s", extension.EULAURL) } // Download EULA text @@ -172,7 +172,7 @@ func promptUser(text []byte, extensionName string) error { answer, err := reader.ReadString('\n') if err != nil { return fmt.Errorf("\nStandard input seems to be closed, please restart " + - "the operation in interactive mode or use '--auto-agree-with-licenses' option.") + "the operation in interactive mode or use '--auto-agree-with-licenses' option") } answer = strings.ToLower(strings.TrimSpace(answer)) diff --git a/internal/connect/product.go b/internal/connect/product.go index 55f891f9..2cad3a14 100644 --- a/internal/connect/product.go +++ b/internal/connect/product.go @@ -201,5 +201,5 @@ func (p Product) findExtension(query Product) (Product, error) { } } } - return Product{}, fmt.Errorf("Extension not found") + return Product{}, fmt.Errorf("extension not found") } diff --git a/internal/connect/product_test.go b/internal/connect/product_test.go index 21e6eb54..e93aac4d 100644 --- a/internal/connect/product_test.go +++ b/internal/connect/product_test.go @@ -124,7 +124,7 @@ func TestSplitTriplet(t *testing.T) { if p.ToTriplet() != expected.ToTriplet() { t.Errorf("Expected: %v, got: %v", expected, p) } - p, err = SplitTriplet("SLES") + _, err = SplitTriplet("SLES") if err == nil { t.Fatal("Expected error, got nil") } diff --git a/internal/connect/registry_auth.go b/internal/connect/registry_auth.go index 8042da83..3596f3e2 100644 --- a/internal/connect/registry_auth.go +++ b/internal/connect/registry_auth.go @@ -117,7 +117,7 @@ func (cfg *RegistryAuthConfig) Set(registry string, login string, password strin } func (cfg *RegistryAuthConfig) Get(registry string) (string, string, bool) { - if cfg.isConfigured(registry) == false { + if !cfg.isConfigured(registry) { return "", "", false } auth := cfg.AuthConfigs[registry].Auth @@ -181,7 +181,7 @@ func removeRegistryAuthentication(login string, password string) { // Make sure to only delete if the credentials actually match, // to not accidentially remove registry configuration which was // manually added - if found == true && login == l && password == p { + if found && login == l && password == p { config.Remove(DEFAULT_SUSE_REGISTRY) if err := config.SaveTo(path); err != nil { diff --git a/internal/connect/registry_auth_test.go b/internal/connect/registry_auth_test.go index 9bcf542c..d9ddd70f 100644 --- a/internal/connect/registry_auth_test.go +++ b/internal/connect/registry_auth_test.go @@ -55,7 +55,7 @@ func mockWriteFile(t *testing.T, matcherfile string) { func mockMkDirAll(t *testing.T) { mkDirAll = func(_ string, perm os.FileMode) error { if perm != 0755 { - t.Log(fmt.Sprintf("mkdir: %s is unlikely the right directory permission. Are you sure?", perm)) + t.Logf("mkdir: %s is unlikely the right directory permission. Are you sure?", perm) } return nil } @@ -128,8 +128,7 @@ func TestRegistryAuthRemoveReadFailed(t *testing.T) { } writeFile = func(_ string, _ []byte, _ os.FileMode) error { - fmt.Errorf("Expected writeFile to never be called") - return nil + return fmt.Errorf("Expected writeFile to never be called") } removeRegistryAuthentication(sampleLogin, samplePassword) diff --git a/internal/credentials/error.go b/internal/credentials/error.go index a855a726..7cdb8ec3 100644 --- a/internal/credentials/error.go +++ b/internal/credentials/error.go @@ -4,7 +4,7 @@ import "errors" // errors var ( - ErrNoProxyCredentials = errors.New("Unable to read proxy credentials") - ErrMalformedSccCredFile = errors.New("Cannot parse credentials file") - ErrMissingCredentialsFile = errors.New("Credentials file is missing") + ErrNoProxyCredentials = errors.New("unable to read proxy credentials") + ErrMalformedSccCredFile = errors.New("cannot parse credentials file") + ErrMissingCredentialsFile = errors.New("credentials file is missing") ) diff --git a/internal/util/system.go b/internal/util/system.go index b773fc2f..136e9b6b 100644 --- a/internal/util/system.go +++ b/internal/util/system.go @@ -127,7 +127,7 @@ func ReadOnlyFilesystem(root string) error { } statfs := &syscall.Statfs_t{} if err := syscall.Statfs(path, statfs); err != nil { - return fmt.Errorf("Checking whether %v is mounted read-only failed: %v", path, err) + return fmt.Errorf("checking whether %v is mounted read-only failed: %v", path, err) } if (statfs.Flags & ST_RDONLY) == ST_RDONLY { @@ -139,7 +139,7 @@ func ReadOnlyFilesystem(root string) error { // `transactional-update` calling SUSEConnect but rather a user // directly which is dicouraged. if root == "" { - return errors.New("This is a transactional system, please use `transactional-update register` to manage your product activations") + return errors.New("this is a transactional system, please use `transactional-update register` to manage your product activations") } return nil