diff --git a/.github/workflows/golang-test-darwin.yml b/.github/workflows/golang-test-darwin.yml index 6cdaa239b..97fdeabe8 100644 --- a/.github/workflows/golang-test-darwin.yml +++ b/.github/workflows/golang-test-darwin.yml @@ -15,14 +15,14 @@ jobs: runs-on: macos-latest steps: - name: Install Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v4 with: go-version: "1.20.x" - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Cache Go modules - uses: actions/cache@v2 + uses: actions/cache@v3 with: path: ~/go/pkg/mod key: macos-go-${{ hashFiles('**/go.sum') }} diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml index d21ecd784..13061f6eb 100644 --- a/.github/workflows/golang-test-linux.yml +++ b/.github/workflows/golang-test-linux.yml @@ -18,13 +18,13 @@ jobs: runs-on: ubuntu-latest steps: - name: Install Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v4 with: go-version: "1.20.x" - name: Cache Go modules - uses: actions/cache@v2 + uses: actions/cache@v3 with: path: ~/go/pkg/mod key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} @@ -32,7 +32,7 @@ jobs: ${{ runner.os }}-go- - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Install dependencies run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev gcc-multilib @@ -47,13 +47,13 @@ jobs: runs-on: ubuntu-latest steps: - name: Install Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v4 with: go-version: "1.20.x" - name: Cache Go modules - uses: actions/cache@v2 + uses: actions/cache@v3 with: path: ~/go/pkg/mod key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} @@ -61,7 +61,7 @@ jobs: ${{ runner.os }}-go- - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Install dependencies run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index f5d1835ce..2a5c51c8a 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -8,14 +8,13 @@ jobs: name: lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - name: Checkout code + uses: actions/checkout@v3 - name: Install Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v4 with: go-version: "1.20.x" - name: Install dependencies run: sudo apt update && sudo apt install -y -q libgtk-3-dev libayatana-appindicator3-dev libgl1-mesa-dev xorg-dev - name: golangci-lint - uses: golangci/golangci-lint-action@v2 - with: - args: --timeout=6m \ No newline at end of file + uses: golangci/golangci-lint-action@v3 \ No newline at end of file diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f682fe274..3feefdd49 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -29,20 +29,24 @@ concurrency: jobs: release: runs-on: ubuntu-latest + env: + flags: "" steps: + - if: ${{ !startsWith(github.ref, 'refs/tags/v') }} + run: echo "flags=--snapshot" >> $GITHUB_ENV - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: fetch-depth: 0 # It is required for GoReleaser to work properly - name: Set up Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v4 with: go-version: "1.20" - name: Cache Go modules - uses: actions/cache@v1 + uses: actions/cache@v3 with: path: ~/go/pkg/mod key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} @@ -56,10 +60,10 @@ jobs: run: git --no-pager diff --exit-code - name: Set up QEMU - uses: docker/setup-qemu-action@v1 + uses: docker/setup-qemu-action@v2 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v2 - name: Login to Docker hub if: github.event_name != 'pull_request' @@ -82,10 +86,10 @@ jobs: run: rsrc -arch 386 -ico client/ui/netbird.ico -manifest client/manifest.xml -o client/resources_windows_386.syso - name: Run GoReleaser - uses: goreleaser/goreleaser-action@v2 + uses: goreleaser/goreleaser-action@v4 with: version: ${{ env.GORELEASER_VER }} - args: release --rm-dist + args: release --rm-dist ${{ env.flags }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} HOMEBREW_TAP_GITHUB_TOKEN: ${{ secrets.HOMEBREW_TAP_GITHUB_TOKEN }} @@ -93,7 +97,7 @@ jobs: UPLOAD_YUM_SECRET: ${{ secrets.PKG_UPLOAD_SECRET }} - name: upload non tags for debug purposes - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: release path: dist/ @@ -102,17 +106,19 @@ jobs: release_ui: runs-on: ubuntu-latest steps: + - if: ${{ !startsWith(github.ref, 'refs/tags/v') }} + run: echo "flags=--snapshot" >> $GITHUB_ENV - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: fetch-depth: 0 # It is required for GoReleaser to work properly - name: Set up Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v4 with: go-version: "1.20" - name: Cache Go modules - uses: actions/cache@v1 + uses: actions/cache@v3 with: path: ~/go/pkg/mod key: ${{ runner.os }}-ui-go-${{ hashFiles('**/go.sum') }} @@ -132,17 +138,17 @@ jobs: - name: Generate windows rsrc run: rsrc -arch amd64 -ico client/ui/netbird.ico -manifest client/ui/manifest.xml -o client/ui/resources_windows_amd64.syso - name: Run GoReleaser - uses: goreleaser/goreleaser-action@v2 + uses: goreleaser/goreleaser-action@v4 with: version: ${{ env.GORELEASER_VER }} - args: release --config .goreleaser_ui.yaml --rm-dist + args: release --config .goreleaser_ui.yaml --rm-dist ${{ env.flags }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} HOMEBREW_TAP_GITHUB_TOKEN: ${{ secrets.HOMEBREW_TAP_GITHUB_TOKEN }} UPLOAD_DEBIAN_SECRET: ${{ secrets.PKG_UPLOAD_SECRET }} UPLOAD_YUM_SECRET: ${{ secrets.PKG_UPLOAD_SECRET }} - name: upload non tags for debug purposes - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: release-ui path: dist/ @@ -151,19 +157,21 @@ jobs: release_ui_darwin: runs-on: macos-11 steps: + - if: ${{ !startsWith(github.ref, 'refs/tags/v') }} + run: echo "flags=--snapshot" >> $GITHUB_ENV - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: fetch-depth: 0 # It is required for GoReleaser to work properly - name: Set up Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v4 with: go-version: "1.20" - name: Cache Go modules - uses: actions/cache@v1 + uses: actions/cache@v3 with: path: ~/go/pkg/mod key: ${{ runner.os }}-ui-go-${{ hashFiles('**/go.sum') }} @@ -175,15 +183,15 @@ jobs: - name: Run GoReleaser id: goreleaser - uses: goreleaser/goreleaser-action@v2 + uses: goreleaser/goreleaser-action@v4 with: version: ${{ env.GORELEASER_VER }} - args: release --config .goreleaser_ui_darwin.yaml --rm-dist + args: release --config .goreleaser_ui_darwin.yaml --rm-dist ${{ env.flags }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: upload non tags for debug purposes - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: release-ui-darwin path: dist/ diff --git a/.github/workflows/test-infrastructure-files.yml b/.github/workflows/test-infrastructure-files.yml index fdebc882e..d196ce0e0 100644 --- a/.github/workflows/test-infrastructure-files.yml +++ b/.github/workflows/test-infrastructure-files.yml @@ -24,12 +24,12 @@ jobs: run: sudo apt-get install -y curl - name: Install Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v4 with: go-version: "1.20.x" - name: Cache Go modules - uses: actions/cache@v2 + uses: actions/cache@v3 with: path: ~/go/pkg/mod key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 000000000..5034db708 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,54 @@ +run: + # Timeout for analysis, e.g. 30s, 5m. + # Default: 1m + timeout: 6m + +# This file contains only configs which differ from defaults. +# All possible options can be found here https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml +linters-settings: + errcheck: + # Report about not checking of errors in type assertions: `a := b.(MyStruct)`. + # Such cases aren't reported by default. + # Default: false + check-type-assertions: false + + govet: + # Enable all analyzers. + # Default: false + enable-all: false + enable: + - nilness + +linters: + disable-all: true + enable: + ## enabled by default + - errcheck # checking for unchecked errors, these unchecked errors can be critical bugs in some cases + - gosimple # specializes in simplifying a code + - govet # reports suspicious constructs, such as Printf calls whose arguments do not align with the format string + - ineffassign # detects when assignments to existing variables are not used + - staticcheck # is a go vet on steroids, applying a ton of static analysis checks + - typecheck # like the front-end of a Go compiler, parses and type-checks Go code + - unused # checks for unused constants, variables, functions and types + ## disable by default but the have interesting results so lets add them + - bodyclose # checks whether HTTP response body is closed successfully + - nilerr # finds the code that returns nil even if it checks that the error is not nil + - nilnil # checks that there is no simultaneous return of nil error and an invalid value + - sqlclosecheck # checks that sql.Rows and sql.Stmt are closed + - wastedassign # wastedassign finds wasted assignment statements +issues: + # Maximum count of issues with the same text. + # Set to 0 to disable. + # Default: 3 + max-same-issues: 5 + + exclude-rules: + - path: sharedsock/filter.go + linters: + - unused + - path: client/firewall/iptables/rule.go + linters: + - unused + - path: mock.go + linters: + - nilnil \ No newline at end of file diff --git a/base62/base62.go b/base62/base62.go index d2525f704..efafbc768 100644 --- a/base62/base62.go +++ b/base62/base62.go @@ -18,10 +18,9 @@ func Encode(num uint32) string { } var encoded strings.Builder - remainder := uint32(0) for num > 0 { - remainder = num % base + remainder := num % base encoded.WriteByte(alphabet[remainder]) num /= base } diff --git a/client/cmd/status.go b/client/cmd/status.go index 5d741462b..9dfd042f8 100644 --- a/client/cmd/status.go +++ b/client/cmd/status.go @@ -109,9 +109,9 @@ func statusFunc(cmd *cobra.Command, args []string) error { ctx := internal.CtxInitState(context.Background()) - resp, _ := getStatus(ctx, cmd) + resp, err := getStatus(ctx, cmd) if err != nil { - return nil + return err } if resp.GetStatus() == string(internal.StatusNeedsLogin) || resp.GetStatus() == string(internal.StatusLoginFailed) { @@ -133,7 +133,7 @@ func statusFunc(cmd *cobra.Command, args []string) error { outputInformationHolder := convertToStatusOutputOverview(resp) - statusOutputString := "" + var statusOutputString string switch { case detailFlag: statusOutputString = parseToFullDetailSummary(outputInformationHolder) diff --git a/client/internal/acl/manager.go b/client/internal/acl/manager.go index d4b6930a0..9a9e624d6 100644 --- a/client/internal/acl/manager.go +++ b/client/internal/acl/manager.go @@ -146,12 +146,11 @@ func (d *DefaultManager) ApplyFiltering(networkMap *mgmProto.NetworkMap) { // if this rule is member of rule selection with more than DefaultIPsCountForSet // it's IP address can be used in the ipset for firewall manager which supports it ipset := ipsetByRuleSelectors[d.getRuleGroupingSelector(r)] - ipsetName := "" if ipset.name == "" { d.ipsetCounter++ ipset.name = fmt.Sprintf("nb%07d", d.ipsetCounter) } - ipsetName = ipset.name + ipsetName := ipset.name pairID, rulePair, err := d.protoRuleToFirewallRule(r, ipsetName) if err != nil { log.Errorf("failed to apply firewall rule: %+v, %v", r, err) diff --git a/client/internal/config_test.go b/client/internal/config_test.go index 25e8f7b2e..8bd8d8d61 100644 --- a/client/internal/config_test.go +++ b/client/internal/config_test.go @@ -23,9 +23,6 @@ func TestGetConfig(t *testing.T) { assert.Equal(t, config.ManagementURL.String(), DefaultManagementURL) assert.Equal(t, config.AdminURL.String(), DefaultAdminURL) - if err != nil { - return - } managementURL := "https://test.management.url:33071" adminURL := "https://app.admin.url:443" path := filepath.Join(t.TempDir(), "config.json") diff --git a/client/internal/dns/server_test.go b/client/internal/dns/server_test.go index 73349de89..119ac684c 100644 --- a/client/internal/dns/server_test.go +++ b/client/internal/dns/server_test.go @@ -777,7 +777,7 @@ func createWgInterfaceWithBind(t *testing.T) (*iface.WGIface, error) { newNet, err := stdnet.NewNet(nil) if err != nil { t.Fatalf("create stdnet: %v", err) - return nil, nil + return nil, err } wgIface, err := iface.NewWGIFace("utun2301", "100.66.100.2/24", iface.DefaultMTU, nil, newNet) diff --git a/client/internal/engine.go b/client/internal/engine.go index 038f39e5c..8a6c08642 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -992,14 +992,12 @@ func (e *Engine) parseNATExternalIPMappings() []string { log.Warnf("invalid external IP, %s, ignoring external IP mapping '%s'", external, mapping) break } - if externalIP != nil { - mappedIP := externalIP.String() - if internalIP != nil { - mappedIP = mappedIP + "/" + internalIP.String() - } - mappedIPs = append(mappedIPs, mappedIP) - log.Infof("parsed external IP mapping of '%s' as '%s'", mapping, mappedIP) + mappedIP := externalIP.String() + if internalIP != nil { + mappedIP = mappedIP + "/" + internalIP.String() } + mappedIPs = append(mappedIPs, mappedIP) + log.Infof("parsed external IP mapping of '%s' as '%s'", mapping, mappedIP) } if len(mappedIPs) != len(e.config.NATExternalIPs) { log.Warnf("one or more external IP mappings failed to parse, ignoring all mappings") diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index d1c46181c..9f17ff36b 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -1046,7 +1046,7 @@ func startManagement(dataDir string) (*grpc.Server, string, error) { peersUpdateManager := server.NewPeersUpdateManager() eventStore := &activity.InMemoryEventStore{} if err != nil { - return nil, "", nil + return nil, "", err } accountManager, err := server.BuildManager(store, peersUpdateManager, nil, "", "", eventStore) diff --git a/client/internal/routemanager/client.go b/client/internal/routemanager/client.go index cf4cbe91b..62fe4dfc1 100644 --- a/client/internal/routemanager/client.go +++ b/client/internal/routemanager/client.go @@ -155,7 +155,10 @@ func (c *clientNetwork) startPeersStatusChangeWatcher() { func (c *clientNetwork) removeRouteFromWireguardPeer(peerKey string) error { state, err := c.statusRecorder.GetPeer(peerKey) - if err != nil || state.ConnStatus != peer.StatusConnected { + if err != nil { + return err + } + if state.ConnStatus != peer.StatusConnected { return nil } diff --git a/client/server/server.go b/client/server/server.go index b7cca947f..6748f62ab 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -3,10 +3,11 @@ package server import ( "context" "fmt" - "github.com/netbirdio/netbird/client/internal/auth" "sync" "time" + "github.com/netbirdio/netbird/client/internal/auth" + log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" @@ -315,7 +316,7 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin tokenInfo, err := s.oauthAuthFlow.flow.WaitToken(waitCTX, flowInfo) if err != nil { if err == context.Canceled { - return nil, nil + return nil, nil //nolint:nilnil } s.mutex.Lock() s.oauthAuthFlow.expiresAt = time.Now() diff --git a/iface/module_linux.go b/iface/module_linux.go index 5f244d2c3..e943c0ba7 100644 --- a/iface/module_linux.go +++ b/iface/module_linux.go @@ -161,7 +161,7 @@ func getModulePath(name string) (string, error) { } if err != nil { // skip broken files - return nil + return nil //nolint:nilerr } if !info.Type().IsRegular() { diff --git a/iface/wg_configurer_nonandroid.go b/iface/wg_configurer_nonandroid.go index 70ec5dc04..6749c0966 100644 --- a/iface/wg_configurer_nonandroid.go +++ b/iface/wg_configurer_nonandroid.go @@ -146,9 +146,6 @@ func (c *wGConfigurer) removeAllowedIP(peerKey string, allowedIP string) error { } } - if err != nil { - return err - } peer := wgtypes.PeerConfig{ PublicKey: peerKeyParsed, UpdateOnly: true, diff --git a/management/server/account.go b/management/server/account.go index 26aeed3c5..4c707af3a 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -1022,7 +1022,7 @@ func (am *DefaultAccountManager) lookupUserInCacheByEmail(email string, accountI } } - return nil, nil + return nil, nil //nolint:nilnil } // lookupUserInCache looks up user in the IdP cache and returns it. If the user wasn't found, the function returns nil @@ -1045,7 +1045,7 @@ func (am *DefaultAccountManager) lookupUserInCache(userID string, account *Accou } } - return nil, nil + return nil, nil //nolint:nilnil } func (am *DefaultAccountManager) refreshCache(accountID string) ([]*idp.UserData, error) { diff --git a/management/server/account_test.go b/management/server/account_test.go index 6002b7a3a..64fd90524 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -784,10 +784,6 @@ func TestAccountManager_AddPeer(t *testing.T) { serial := account.Network.CurrentSerial() // should be 0 setupKey, err := manager.CreateSetupKey(account.Id, "test-key", SetupKeyReusable, time.Hour, nil, 999, userID, false) - if err != nil { - return - } - if err != nil { t.Fatal("error creating setup key") return @@ -931,10 +927,6 @@ func TestAccountManager_NetworkUpdates(t *testing.T) { } setupKey, err := manager.CreateSetupKey(account.Id, "test-key", SetupKeyReusable, time.Hour, nil, 999, userID, false) - if err != nil { - return - } - if err != nil { t.Fatal("error creating setup key") return @@ -1115,10 +1107,6 @@ func TestAccountManager_DeletePeer(t *testing.T) { } setupKey, err := manager.CreateSetupKey(account.Id, "test-key", SetupKeyReusable, time.Hour, nil, 999, userID, false) - if err != nil { - return - } - if err != nil { t.Fatal("error creating setup key") return diff --git a/management/server/activity/sqlite/sqlite.go b/management/server/activity/sqlite/sqlite.go index 02c2143e4..a4c85cf60 100644 --- a/management/server/activity/sqlite/sqlite.go +++ b/management/server/activity/sqlite/sqlite.go @@ -3,13 +3,14 @@ package sqlite import ( "database/sql" "encoding/json" - "fmt" + "github.com/netbirdio/netbird/management/server/activity" // sqlite driver - _ "github.com/mattn/go-sqlite3" "path/filepath" "time" + + _ "github.com/mattn/go-sqlite3" ) const ( @@ -24,15 +25,20 @@ const ( "meta TEXT," + " target_id TEXT);" - selectStatement = "SELECT id, activity, timestamp, initiator_id, target_id, account_id, meta" + - " FROM events WHERE account_id = ? ORDER BY timestamp %s LIMIT ? OFFSET ?;" - insertStatement = "INSERT INTO events(activity, timestamp, initiator_id, target_id, account_id, meta) " + + selectDescQuery = "SELECT id, activity, timestamp, initiator_id, target_id, account_id, meta" + + " FROM events WHERE account_id = ? ORDER BY timestamp DESC LIMIT ? OFFSET ?;" + selectAscQuery = "SELECT id, activity, timestamp, initiator_id, target_id, account_id, meta" + + " FROM events WHERE account_id = ? ORDER BY timestamp ASC LIMIT ? OFFSET ?;" + insertQuery = "INSERT INTO events(activity, timestamp, initiator_id, target_id, account_id, meta) " + "VALUES(?, ?, ?, ?, ?, ?)" ) // Store is the implementation of the activity.Store interface backed by SQLite type Store struct { - db *sql.DB + db *sql.DB + insertStatement *sql.Stmt + selectAscStatement *sql.Stmt + selectDescStatement *sql.Stmt } // NewSQLiteStore creates a new Store with an event table if not exists. @@ -48,7 +54,27 @@ func NewSQLiteStore(dataDir string) (*Store, error) { return nil, err } - return &Store{db: db}, nil + insertStmt, err := db.Prepare(insertQuery) + if err != nil { + return nil, err + } + + selectDescStmt, err := db.Prepare(selectDescQuery) + if err != nil { + return nil, err + } + + selectAscStmt, err := db.Prepare(selectAscQuery) + if err != nil { + return nil, err + } + + return &Store{ + db: db, + insertStatement: insertStmt, + selectDescStatement: selectDescStmt, + selectAscStatement: selectAscStmt, + }, nil } func processResult(result *sql.Rows) ([]*activity.Event, error) { @@ -90,13 +116,9 @@ func processResult(result *sql.Rows) ([]*activity.Event, error) { // Get returns "limit" number of events from index ordered descending or ascending by a timestamp func (store *Store) Get(accountID string, offset, limit int, descending bool) ([]*activity.Event, error) { - order := "DESC" + stmt := store.selectDescStatement if !descending { - order = "ASC" - } - stmt, err := store.db.Prepare(fmt.Sprintf(selectStatement, order)) - if err != nil { - return nil, err + stmt = store.selectAscStatement } result, err := stmt.Query(accountID, limit, offset) @@ -110,12 +132,6 @@ func (store *Store) Get(accountID string, offset, limit int, descending bool) ([ // Save an event in the SQLite events table func (store *Store) Save(event *activity.Event) (*activity.Event, error) { - - stmt, err := store.db.Prepare(insertStatement) - if err != nil { - return nil, err - } - var jsonMeta string if event.Meta != nil { metaBytes, err := json.Marshal(event.Meta) @@ -125,7 +141,7 @@ func (store *Store) Save(event *activity.Event) (*activity.Event, error) { jsonMeta = string(metaBytes) } - result, err := stmt.Exec(event.Activity, event.Timestamp, event.InitiatorID, event.TargetID, event.AccountID, jsonMeta) + result, err := store.insertStatement.Exec(event.Activity, event.Timestamp, event.InitiatorID, event.TargetID, event.AccountID, jsonMeta) if err != nil { return nil, err } diff --git a/management/server/ephemeral_test.go b/management/server/ephemeral_test.go index 554d2a028..a763f4cef 100644 --- a/management/server/ephemeral_test.go +++ b/management/server/ephemeral_test.go @@ -31,7 +31,7 @@ type MocAccountManager struct { func (a MocAccountManager) DeletePeer(accountID, peerID, userID string) (*Peer, error) { delete(a.store.account.Peers, peerID) - return nil, nil + return nil, nil //nolint:nilnil } func TestNewManager(t *testing.T) { diff --git a/management/server/http/groups_handler.go b/management/server/http/groups_handler.go index 30c78e21b..d409623df 100644 --- a/management/server/http/groups_handler.go +++ b/management/server/http/groups_handler.go @@ -231,10 +231,8 @@ func (h *GroupsHandler) GetGroup(w http.ResponseWriter, r *http.Request) { util.WriteJSONObject(w, toGroupResponse(account, group)) default: - if err != nil { - util.WriteError(status.Errorf(status.NotFound, "HTTP method not found"), w) - return - } + util.WriteError(status.Errorf(status.NotFound, "HTTP method not found"), w) + return } } diff --git a/management/server/http/middleware/auth_middleware_test.go b/management/server/http/middleware/auth_middleware_test.go index 8c8c941b0..608bf42fa 100644 --- a/management/server/http/middleware/auth_middleware_test.go +++ b/management/server/http/middleware/auth_middleware_test.go @@ -115,8 +115,10 @@ func TestAuthMiddleware_Handler(t *testing.T) { handlerToTest.ServeHTTP(rec, req) - if rec.Result().StatusCode != tc.expectedStatusCode { - t.Errorf("expected status code %d, got %d", tc.expectedStatusCode, rec.Result().StatusCode) + result := rec.Result() + defer result.Body.Close() + if result.StatusCode != tc.expectedStatusCode { + t.Errorf("expected status code %d, got %d", tc.expectedStatusCode, result.StatusCode) } }) } diff --git a/management/server/idp/auth0_test.go b/management/server/idp/auth0_test.go index 0814b4b69..0be401e65 100644 --- a/management/server/idp/auth0_test.go +++ b/management/server/idp/auth0_test.go @@ -133,6 +133,7 @@ func TestAuth0_RequestJWTToken(t *testing.T) { t.Fatal(err) } } + defer res.Body.Close() body, err := io.ReadAll(res.Body) assert.NoError(t, err, "unable to read the response body") diff --git a/management/server/idp/authentik.go b/management/server/idp/authentik.go index 586348fee..0898f1c94 100644 --- a/management/server/idp/authentik.go +++ b/management/server/idp/authentik.go @@ -3,10 +3,6 @@ package idp import ( "context" "fmt" - "github.com/golang-jwt/jwt" - "github.com/netbirdio/netbird/management/server/telemetry" - log "github.com/sirupsen/logrus" - "goauthentik.io/api/v3" "io" "net/http" "net/url" @@ -14,6 +10,11 @@ import ( "strings" "sync" "time" + + "github.com/golang-jwt/jwt" + "github.com/netbirdio/netbird/management/server/telemetry" + log "github.com/sirupsen/logrus" + "goauthentik.io/api/v3" ) // AuthentikManager authentik manager client instance. @@ -236,6 +237,7 @@ func (am *AuthentikManager) UpdateUserAppMetadata(userID string, appMetadata App if err != nil { return err } + defer resp.Body.Close() if am.appMetrics != nil { am.appMetrics.IDPMetrics().CountUpdateUserAppMetadata() @@ -267,6 +269,7 @@ func (am *AuthentikManager) GetUserDataByID(userID string, appMetadata AppMetada if err != nil { return nil, err } + defer resp.Body.Close() if am.appMetrics != nil { am.appMetrics.IDPMetrics().CountGetUserDataByID() @@ -294,6 +297,7 @@ func (am *AuthentikManager) GetAccount(accountID string) ([]*UserData, error) { if err != nil { return nil, err } + defer resp.Body.Close() if am.appMetrics != nil { am.appMetrics.IDPMetrics().CountGetAccount() @@ -330,6 +334,7 @@ func (am *AuthentikManager) GetAllAccounts() (map[string][]*UserData, error) { if err != nil { return nil, err } + defer resp.Body.Close() if am.appMetrics != nil { am.appMetrics.IDPMetrics().CountGetAllAccounts() @@ -389,6 +394,7 @@ func (am *AuthentikManager) CreateUser(email, name, accountID, invitedByEmail st if err != nil { return nil, err } + defer resp.Body.Close() if am.appMetrics != nil { am.appMetrics.IDPMetrics().CountCreateUser() @@ -416,6 +422,7 @@ func (am *AuthentikManager) GetUserByEmail(email string) ([]*UserData, error) { if err != nil { return nil, err } + defer resp.Body.Close() if am.appMetrics != nil { am.appMetrics.IDPMetrics().CountGetUserByEmail() @@ -469,10 +476,11 @@ func (am *AuthentikManager) getUserGroupByName(name string) (string, error) { return "", err } - groupList, _, err := am.apiClient.CoreApi.CoreGroupsList(ctx).Name(name).Execute() + groupList, resp, err := am.apiClient.CoreApi.CoreGroupsList(ctx).Name(name).Execute() if err != nil { return "", err } + defer resp.Body.Close() if groupList != nil { if len(groupList.Results) > 0 { @@ -485,6 +493,7 @@ func (am *AuthentikManager) getUserGroupByName(name string) (string, error) { if err != nil { return "", err } + defer resp.Body.Close() if resp.StatusCode != http.StatusCreated { return "", fmt.Errorf("unable to create user group, statusCode: %d", resp.StatusCode) diff --git a/management/server/idp/authentik_test.go b/management/server/idp/authentik_test.go index 5cf8f2b2c..c70a84efd 100644 --- a/management/server/idp/authentik_test.go +++ b/management/server/idp/authentik_test.go @@ -2,13 +2,14 @@ package idp import ( "fmt" - "github.com/netbirdio/netbird/management/server/telemetry" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "io" "strings" "testing" "time" + + "github.com/netbirdio/netbird/management/server/telemetry" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewAuthentikManager(t *testing.T) { @@ -133,6 +134,7 @@ func TestAuthentikRequestJWTToken(t *testing.T) { t.Fatal(err) } } else { + defer resp.Body.Close() body, err := io.ReadAll(resp.Body) assert.NoError(t, err, "unable to read the response body") diff --git a/management/server/idp/google_workspace.go b/management/server/idp/google_workspace.go index efe457fdd..2e65497dc 100644 --- a/management/server/idp/google_workspace.go +++ b/management/server/idp/google_workspace.go @@ -4,15 +4,16 @@ import ( "context" "encoding/base64" "fmt" + "net/http" + "strings" + "time" + "github.com/netbirdio/netbird/management/server/telemetry" log "github.com/sirupsen/logrus" "golang.org/x/oauth2/google" admin "google.golang.org/api/admin/directory/v1" "google.golang.org/api/googleapi" "google.golang.org/api/option" - "net/http" - "strings" - "time" ) // GoogleWorkspaceManager Google Workspace manager client instance. @@ -271,7 +272,8 @@ func getGoogleCredentials(serviceAccountKey string) (*google.Credentials, error) admin.AdminDirectoryUserScope, ) if err == nil { - return creds, err + // No need to fallback to the default Google credentials path + return creds, nil } log.Debugf("failed to retrieve Google credentials from ServiceAccountKey: %v", err) diff --git a/management/server/idp/idp.go b/management/server/idp/idp.go index 48afd5c32..a1b55b183 100644 --- a/management/server/idp/idp.go +++ b/management/server/idp/idp.go @@ -92,7 +92,7 @@ func NewManager(config Config, appMetrics telemetry.AppMetrics) (Manager, error) switch strings.ToLower(config.ManagerType) { case "none", "": - return nil, nil + return nil, nil //nolint:nilnil case "auth0": auth0ClientConfig := config.Auth0ClientCredentials if config.ClientConfig != nil { diff --git a/management/server/idp/keycloak_test.go b/management/server/idp/keycloak_test.go index 115306d7d..0c33fc137 100644 --- a/management/server/idp/keycloak_test.go +++ b/management/server/idp/keycloak_test.go @@ -145,6 +145,7 @@ func TestKeycloakRequestJWTToken(t *testing.T) { t.Fatal(err) } } else { + defer resp.Body.Close() body, err := io.ReadAll(resp.Body) assert.NoError(t, err, "unable to read the response body") diff --git a/management/server/idp/zitadel_test.go b/management/server/idp/zitadel_test.go index 28d26aedd..b558bba73 100644 --- a/management/server/idp/zitadel_test.go +++ b/management/server/idp/zitadel_test.go @@ -124,6 +124,7 @@ func TestZitadelRequestJWTToken(t *testing.T) { t.Fatal(err) } } else { + defer resp.Body.Close() body, err := io.ReadAll(resp.Body) assert.NoError(t, err, "unable to read the response body") diff --git a/management/server/jwtclaims/jwtValidator.go b/management/server/jwtclaims/jwtValidator.go index d15327566..b564e4f4e 100644 --- a/management/server/jwtclaims/jwtValidator.go +++ b/management/server/jwtclaims/jwtValidator.go @@ -141,7 +141,7 @@ func (m *JWTValidator) ValidateAndParse(token string) (*jwt.Token, error) { if m.options.CredentialsOptional { log.Debugf("no credentials found (CredentialsOptional=true)") // No error, just no token (and that is ok given that CredentialsOptional is true) - return nil, nil + return nil, nil //nolint:nilnil } // If we get here, the required token is missing @@ -219,7 +219,7 @@ func getPemCert(token *jwt.Token, jwks *Jwks) (string, error) { return generatePemFromJWK(jwks.Keys[k]) } - return "", errors.New("unable to find appropriate key") + return cert, errors.New("unable to find appropriate key") } func generatePemFromJWK(jwk JSONWebKey) (string, error) { diff --git a/management/server/management_proto_test.go b/management/server/management_proto_test.go index 792d05187..66661dbf8 100644 --- a/management/server/management_proto_test.go +++ b/management/server/management_proto_test.go @@ -141,11 +141,6 @@ func Test_SyncProtocol(t *testing.T) { return } - if err != nil { - t.Fatal(err) - return - } - sync, err := client.Sync(context.TODO(), &mgmtProto.EncryptedMessage{ WgPubKey: key.PublicKey().String(), Body: message, diff --git a/management/server/peer_test.go b/management/server/peer_test.go index 822856e6a..36e96df43 100644 --- a/management/server/peer_test.go +++ b/management/server/peer_test.go @@ -79,10 +79,6 @@ func TestAccountManager_GetNetworkMap(t *testing.T) { } setupKey, err := manager.CreateSetupKey(account.Id, "test-key", SetupKeyReusable, time.Hour, nil, 999, userId, false) - if err != nil { - return - } - if err != nil { t.Fatal("error creating setup key") return @@ -332,10 +328,6 @@ func TestAccountManager_GetPeerNetwork(t *testing.T) { } setupKey, err := manager.CreateSetupKey(account.Id, "test-key", SetupKeyReusable, time.Hour, nil, 999, userId, false) - if err != nil { - return - } - if err != nil { t.Fatal("error creating setup key") return @@ -406,14 +398,11 @@ func TestDefaultAccountManager_GetPeer(t *testing.T) { // two peers one added by a regular user and one with a setup key setupKey, err := manager.CreateSetupKey(account.Id, "test-key", SetupKeyReusable, time.Hour, nil, 999, adminUser, false) - if err != nil { - return - } - if err != nil { t.Fatal("error creating setup key") return } + peerKey1, err := wgtypes.GeneratePrivateKey() if err != nil { t.Fatal(err) diff --git a/management/server/route.go b/management/server/route.go index c02729a72..f51b7c2db 100644 --- a/management/server/route.go +++ b/management/server/route.go @@ -1,15 +1,16 @@ package server import ( + "net/netip" + "strconv" + "unicode/utf8" + "github.com/netbirdio/netbird/management/proto" "github.com/netbirdio/netbird/management/server/activity" "github.com/netbirdio/netbird/management/server/status" "github.com/netbirdio/netbird/route" "github.com/rs/xid" log "github.com/sirupsen/logrus" - "net/netip" - "strconv" - "unicode/utf8" ) const ( @@ -104,12 +105,6 @@ func (am *DefaultAccountManager) checkPrefixPeerExists(accountID, peerID string, routesWithPrefix := account.GetRoutesByPrefix(prefix) - if err != nil { - if s, ok := status.FromError(err); ok && s.Type() == status.NotFound { - return nil - } - return status.Errorf(status.InvalidArgument, "failed to parse prefix %s", prefix.String()) - } for _, prefixRoute := range routesWithPrefix { if prefixRoute.Peer == peerID { return status.Errorf(status.AlreadyExists, "failed to add route with prefix %s - peer already has this route", prefix.String()) diff --git a/management/server/user.go b/management/server/user.go index b3556957d..8ee036df7 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -405,13 +405,13 @@ func (am *DefaultAccountManager) CreatePAT(accountID string, initiatorUserID str return nil, err } - targetUser := account.Users[targetUserID] - if targetUser == nil { - return nil, status.Errorf(status.NotFound, "targetUser not found") + targetUser, ok := account.Users[targetUserID] + if !ok { + return nil, status.Errorf(status.NotFound, "user not found") } - executingUser := account.Users[initiatorUserID] - if targetUser == nil { + executingUser, ok := account.Users[initiatorUserID] + if !ok { return nil, status.Errorf(status.NotFound, "user not found") } @@ -447,13 +447,13 @@ func (am *DefaultAccountManager) DeletePAT(accountID string, initiatorUserID str return status.Errorf(status.NotFound, "account not found: %s", err) } - targetUser := account.Users[targetUserID] - if targetUser == nil { + targetUser, ok := account.Users[targetUserID] + if !ok { return status.Errorf(status.NotFound, "user not found") } - executingUser := account.Users[initiatorUserID] - if targetUser == nil { + executingUser, ok := account.Users[initiatorUserID] + if !ok { return status.Errorf(status.NotFound, "user not found") } @@ -497,13 +497,13 @@ func (am *DefaultAccountManager) GetPAT(accountID string, initiatorUserID string return nil, status.Errorf(status.NotFound, "account not found: %s", err) } - targetUser := account.Users[targetUserID] - if targetUser == nil { + targetUser, ok := account.Users[targetUserID] + if !ok { return nil, status.Errorf(status.NotFound, "user not found") } - executingUser := account.Users[initiatorUserID] - if targetUser == nil { + executingUser, ok := account.Users[initiatorUserID] + if !ok { return nil, status.Errorf(status.NotFound, "user not found") } @@ -529,13 +529,13 @@ func (am *DefaultAccountManager) GetAllPATs(accountID string, initiatorUserID st return nil, status.Errorf(status.NotFound, "account not found: %s", err) } - targetUser := account.Users[targetUserID] - if targetUser == nil { + targetUser, ok := account.Users[targetUserID] + if !ok { return nil, status.Errorf(status.NotFound, "user not found") } - executingUser := account.Users[initiatorUserID] - if targetUser == nil { + executingUser, ok := account.Users[initiatorUserID] + if !ok { return nil, status.Errorf(status.NotFound, "user not found") }