Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade go to v1.23.1 #1664

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 40 additions & 59 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,29 @@ linters-settings:
- (github.com/sirupsen/logrus.FieldLogger).Warnf
- (github.com/sirupsen/logrus.FieldLogger).Errorf
- (github.com/sirupsen/logrus.FieldLogger).Fatalf
revive:
confidence: 0.8
rules:
- name: exported
- name: blank-imports
- name: context-as-argument
- name: context-keys-type
- name: dot-imports
- name: error-return
- name: error-strings
- name: error-naming
- name: exported
- name: increment-decrement
- name: var-naming
- name: package-comments
- name: range
- name: receiver-naming
- name: time-naming
- name: unexported-return
- name: indent-error-flow
- name: errorf
- name: superfluous-else
- name: unreachable-code
goimports:
local-prefixes: github.com/networkservicemesh/sdk
gocyclo:
Expand Down Expand Up @@ -57,81 +80,51 @@ linters-settings:
gosec:
excludes:
- G115
- G301 # Fixable
- G302 # Fixable
- G306 # Fixable
- G204
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we excluding G204

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

G204 was included in exclude-rules list before, but on the current linter version we are getting too many new gosec errors, so decided to put them into one place- general config, not individually

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, add a todo and refer to the issue where we plan to fix these issues.

- G301
- G302
- G306
gocritic:
enabled-checks:
- appendAssign
- assignOp
- appendCombine
- argOrder
- badCall
- badCond
- boolExprSimplify
- builtinShadow
- captLocal
- caseOrder
- codegenComment
- commentFormatting
- commentedOutCode
- commentedOutImport
- defaultCaseOrder
- deprecatedComment
- docStub
- dupArg
- dupBranchBody
- dupCase
- dupImport
- dupSubExpr
- elseif
- emptyFallthrough
- emptyStringTest
- equalFold
- evalOrder
- exitAfterDefer
- flagDeref
- flagName
- hexLiteral
- hugeParam
- ifElseChain
- importShadow
- indexAlloc
- initClause
- methodExprCall
- nestingReduce
- newDeref
- nilValReturn
- octalLiteral
- offBy1
- paramTypeCombine
- rangeExprCopy
- rangeValCopy
- regexpMust
- regexpPattern
- singleCaseSwitch
- sloppyLen
- sloppyReassign
- stringXbytes
- switchTrue
- typeAssertChain
- typeSwitchVar
- typeUnparen
- unlabelStmt
- unnamedResult
- unnecessaryBlock
- underef
- unlambda
- unslice
- valSwap
- weakCond
- wrapperFunc
- yodaStyleExpr
linters:
disable-all: true
enable:
- goheader
- bodyclose
- unused
- depguard
- dogsled
- dupl
Expand All @@ -143,12 +136,14 @@ linters:
- gocyclo
- gofmt
- goimports
- revive
- gosec
- gosimple
- govet
- ineffassign
- misspell
- nakedret
- copyloopvar
- staticcheck
- stylecheck
- typecheck
Expand All @@ -162,9 +157,6 @@ issues:
exclude-rules:
# We really *do* want to pass a pointer to an interface in these cases. See comments in file for New{Server,Client}
# function for why
- path: .*\.gen\.go
linters:
- dupl
- path: .*registry.*.go
linters:
- dupl
Expand Down Expand Up @@ -216,30 +208,10 @@ issues:
linters:
- dupl
text: "lines are duplicate of"
- path: pkg/tools/spiffeutils/tls_peer.go
linters:
- gosec
text: "G402: TLS InsecureSkipVerify set true"
- path: pkg/networkservice/common/updatepath/server_test.go
linters:
- gochecknoinits
text: "don't use `init` function"
- path: pkg/tools/debug/self.go
linters:
- gosec
text: "G204: Subprocess launched with variable"
- path: pkg/tools/executils/start.go
linters:
- gosec
text: "G204: Subprocess launched with variable"
- path: pkg/tools/executils/run.go
linters:
- gosec
text: "G204: Subprocess launched with variable"
- path: pkg/tools/executils/output.go
linters:
- gosec
text: "G204: Subprocess launched with variable"
- path: pkg/tools/spire/start.go
linters:
- funlen
Expand Down Expand Up @@ -274,7 +246,16 @@ issues:
- path: pkg/networkservice/common/switchcase/.*_test.go
linters:
- dupl
- path: pkg/tools/ippool/ippool.go
linters:
- unused
- path: pkg/networkservice/common/switchcase/common_test.go
linters:
- revive
- path: pkg/tools/opa/policies.go
linters:
- revive
# TODO - remove deprecated opentelemetry functions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a ticket to refer here. Thanks.

- path: pkg/tools/tracing/grpcoptions.go
linters:
- staticcheck
text: "SA1019: otelgrpc.*"
3 changes: 2 additions & 1 deletion pkg/networkservice/common/clientinfo/client_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright (c) 2020-2021 Doc.ai and/or its affiliates.
//
// Copyright (c) 2024 Cisco and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -133,7 +135,6 @@ func TestClientInfo(t *testing.T) {
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
err := setEnvs(tc.envs)
require.NoError(t, err)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2021-2023 Cisco and/or its affiliates.
// Copyright (c) 2021-2024 Cisco and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -82,7 +82,6 @@ func (m *monitorConnectionServer) Send(event *networkservice.ConnectionEvent) (_
return
}
for id, filter := range m.filters {
id, filter := id, filter
e := event.Clone()
filter.executor.AsyncExec(func() {
var err error
Expand Down
1 change: 0 additions & 1 deletion pkg/networkservice/common/switchcase/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ func testSwitchClient(t *testing.T, conditions []switchcase.Condition, expected

var cases []*switchcase.ClientCase
for i, cond := range conditions {
i := i
cases = append(cases, &switchcase.ClientCase{
Condition: cond,
Client: checkcontext.NewClient(t, func(*testing.T, context.Context) {
Expand Down
1 change: 0 additions & 1 deletion pkg/networkservice/common/switchcase/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ func testSwitchServer(t *testing.T, conditions []switchcase.Condition, expected

var cases []*switchcase.ServerCase
for i, cond := range conditions {
i := i
cases = append(cases, &switchcase.ServerCase{
Condition: cond,
Server: checkcontext.NewServer(t, func(*testing.T, context.Context) {
Expand Down
8 changes: 0 additions & 8 deletions pkg/networkservice/connectioncontext/ipcontext/vl3/ipam.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,6 @@ func (p *IPAM) freeIfAllocated(ipNet string) {
}
}

func (p *IPAM) isExcluded(ipNet string) bool {
p.Lock()
defer p.Unlock()

_, r := p.excludedPrefixes[ipNet]
return r
}

// Reset resets IPAM's ippol by setting new prefix
func (p *IPAM) Reset(prefix string, excludePrefixes ...string) error {
p.Lock()
Expand Down
4 changes: 2 additions & 2 deletions pkg/registry/common/authorize/common.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2022-2023 Cisco and/or its affiliates.
// Copyright (c) 2022-2024 Cisco and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -32,7 +32,7 @@ import (
type RegistryOpaInput struct {
ResourceID string `json:"resource_id"`
ResourceName string `json:"resource_name"`
ResourcePathIdsMap map[string][]string `json:"resource_path_ids_map"`
ResourcePathIDsMap map[string][]string `json:"resource_path_ids_map"`
PathSegments []*grpcmetadata.PathSegment `json:"path_segments"`
Index uint32 `json:"index"`
}
Expand Down
22 changes: 11 additions & 11 deletions pkg/registry/common/authorize/ns_client.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2022-2023 Cisco and/or its affiliates.
// Copyright (c) 2022-2024 Cisco and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -35,14 +35,14 @@ import (

type authorizeNSClient struct {
policies policiesList
nsPathIdsMap *genericsync.Map[string, []string]
nsPathIDsMap *genericsync.Map[string, []string]
}

// NewNetworkServiceRegistryClient - returns a new authorization registry.NetworkServiceRegistryClient
// Authorize registry client checks spiffeID of NS.
func NewNetworkServiceRegistryClient(opts ...Option) registry.NetworkServiceRegistryClient {
o := &options{
resourcePathIdsMap: new(genericsync.Map[string, []string]),
resourcePathIDsMap: new(genericsync.Map[string, []string]),
}

for _, opt := range opts {
Expand All @@ -51,7 +51,7 @@ func NewNetworkServiceRegistryClient(opts ...Option) registry.NetworkServiceRegi

return &authorizeNSClient{
policies: o.policies,
nsPathIdsMap: o.resourcePathIdsMap,
nsPathIDsMap: o.resourcePathIDsMap,
}
}

Expand Down Expand Up @@ -79,17 +79,17 @@ func (c *authorizeNSClient) Register(ctx context.Context, ns *registry.NetworkSe

path = grpcmetadata.PathFromContext(ctx)
spiffeID := getSpiffeIDFromPath(ctx, path)
rawMap := getRawMap(c.nsPathIdsMap)
rawMap := getRawMap(c.nsPathIDsMap)

input := RegistryOpaInput{
ResourceID: spiffeID.String(),
ResourceName: resp.Name,
ResourcePathIdsMap: rawMap,
ResourcePathIDsMap: rawMap,
PathSegments: path.PathSegments,
Index: path.Index,
}
if err := c.policies.check(ctx, input); err != nil {
if _, load := c.nsPathIdsMap.Load(resp.Name); !load {
if _, load := c.nsPathIDsMap.Load(resp.Name); !load {
unregisterCtx, cancelUnregister := postponeCtxFunc()
defer cancelUnregister()

Expand All @@ -101,7 +101,7 @@ func (c *authorizeNSClient) Register(ctx context.Context, ns *registry.NetworkSe
return nil, err
}

c.nsPathIdsMap.Store(resp.Name, resp.PathIds)
c.nsPathIDsMap.Store(resp.Name, resp.PathIds)
return resp, nil
}

Expand Down Expand Up @@ -130,19 +130,19 @@ func (c *authorizeNSClient) Unregister(ctx context.Context, ns *registry.Network
}

spiffeID := getSpiffeIDFromPath(ctx, path)
rawMap := getRawMap(c.nsPathIdsMap)
rawMap := getRawMap(c.nsPathIDsMap)

input := RegistryOpaInput{
ResourceID: spiffeID.String(),
ResourceName: ns.Name,
ResourcePathIdsMap: rawMap,
ResourcePathIDsMap: rawMap,
PathSegments: path.PathSegments,
Index: path.Index,
}
if err := c.policies.check(ctx, input); err != nil {
return nil, err
}

c.nsPathIdsMap.Delete(ns.Name)
c.nsPathIDsMap.Delete(ns.Name)
return resp, nil
}
Loading
Loading