From f7174fbd4d9580814e1cdc7df4d7467b444ace92 Mon Sep 17 00:00:00 2001 From: "Arrobo, Gabriel" Date: Wed, 20 Mar 2024 15:42:14 -0700 Subject: [PATCH] Enable several linters --- .golangci.yml | 322 +++++++++++++++++++++++++- cmd/pfcpctl/main.go | 6 +- internal/pfcpctl/commands/helpers.go | 9 +- internal/pfcpctl/commands/services.go | 20 +- internal/pfcpctl/commands/sessions.go | 7 +- internal/pfcpctl/config/config.go | 2 +- internal/pfcpsim/helpers.go | 11 +- internal/pfcpsim/helpers_test.go | 30 ++- pkg/pfcpsim/pfcpsim.go | 9 +- pkg/pfcpsim/session/far_builder.go | 2 +- 10 files changed, 372 insertions(+), 46 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index f632cd1..357611a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,23 +1,321 @@ -# SPDX-License-Identifier: Apache-2.0 # Copyright 2022-present Open Networking Foundation +# SPDX-License-Identifier: Apache-2.0 +# -# golangci-lint configuration used for CI +# This file contains all available configuration options +# with their default values. +# options for analysis running run: + # default concurrency is a available CPU number + concurrency: 4 + # timeout for analysis, e.g. 30s, 5m, default is 1m + timeout: 1m + # exit code when at least one issue was found, default is 1 + issues-exit-code: 1 + # include test files or not, default is true tests: true - timeout: 10m + # list of build tags, all linters use it. Default is empty list. + build-tags: + # which dirs to skip: issues from them won't be reported; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but default dirs are skipped independently + # from this option's value (see skip-dirs-use-default). + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + skip-dirs: + # default is true. Enables skipping of directories: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs-use-default: true + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + # "/" will be replaced by current OS file path separator to properly work + # on Windows. skip-files: - ".*\\.pb\\.go" - skip-dirs-use-default: true + # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules": + # If invoked with -mod=readonly, the go command is disallowed from the implicit + # automatic updating of go.mod described above. Instead, it fails when any changes + # to go.mod are needed. This setting is most useful to check that go.mod does + # not need updates, such as in a continuous integration and testing system. + # If invoked with -mod=vendor, the go command assumes that the vendor + # directory holds the correct copies of dependencies and ignores + # the dependency descriptions in go.mod. + #modules-download-mode: readonly|release|vendor + # Allow multiple parallel golangci-lint instances running. + # If false (default) - golangci-lint acquires file lock on start. + allow-parallel-runners: true +# output configuration options +output: + # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number" + format: colored-line-number + # print lines of code with issue, default is true + print-issued-lines: true + # print linter name in the end of issue text, default is true + print-linter-name: true + # make issues output unique by line, default is true + uniq-by-line: true +# all available settings of specific linters +linters-settings: + errcheck: + # report about not checking of errors in type assertions: `a := b.(MyStruct)`; + # default is false: such cases aren't reported by default. + check-type-assertions: false + # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; + # default is false: such cases aren't reported by default. + check-blank: true + # [deprecated] comma-separated list of pairs of the form pkg:regex + # the regex is used to ignore names within pkg. (default "fmt:.*"). + # see https://github.com/kisielk/errcheck#the-deprecated-method for details + #ignore: fmt:.*,io/ioutil:^Read.* + # path to a file containing a list of functions to exclude from checking + # see https://github.com/kisielk/errcheck#excluding-functions for details + #exclude: /path/to/file.txt + funlen: + lines: 60 + statements: 40 + gocognit: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 10 + nestif: + # minimal complexity of if statements to report, 5 by default + min-complexity: 4 + goconst: + # minimal length of string constant, 3 by default + min-len: 3 + # minimal occurrences count to trigger, 3 by default + min-occurrences: 3 + gocritic: + # Which checks should be enabled; can't be combined with 'disabled-checks'; + # See https://go-critic.github.io/overview#checks-overview + # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run` + # By default list of stable checks is used. + enabled-checks: + #- rangeValCopy + # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty + disabled-checks: + - regexpMust + # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks. + # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". + enabled-tags: + - performance + disabled-tags: + - experimental + settings: # settings passed to gocritic + captLocal: # must be valid enabled check name + paramsOnly: true + rangeValCopy: + sizeThreshold: 32 + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 10 + godox: + # report any comments starting with keywords, this is useful for TODO or FIXME comments that + # might be left in the code accidentally and should be resolved before merging + keywords: # default keywords are TODO, BUG, and FIXME, these can be overwritten by this setting + #- TODO + - FIXME + - BUG + #- NOTE + #- OPTIMIZE # marks code that should be optimized before merging + #- HACK # marks hack-arounds that should be removed before merging + - XXX # Fatal! Important problem + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: true + goimports: + # put imports beginning with prefix after 3rd-party packages; + # it's a comma-separated list of prefixes + local-prefixes: github.com/org/project + golint: + # minimal confidence for issues, default is 0.8 + min-confidence: 0.8 + gomnd: + settings: + mnd: + # the list of enabled checks, see https://github.com/tommy-muehle/go-mnd/#checks for description. + checks: argument,case,condition,operation,return,assign + gomodguard: + allowed: + modules: # List of allowed modules + # - gopkg.in/yaml.v2 + domains: # List of allowed module domains + # - golang.org + blocked: + modules: # List of blocked modules + # - github.com/uudashr/go-module: # Blocked module + # recommendations: # Recommended modules that should be used instead (Optional) + # - golang.org/x/mod + # reason: "`mod` is the official go.mod parser library." # Reason why the recommended module should be used (Optional) + versions: # List of blocked module version constraints + # - github.com/mitchellh/go-homedir: # Blocked module with version constraint + # version: "< 1.1.0" # Version constraint, see https://github.com/Masterminds/semver#basic-comparisons + # reason: "testing if blocked version constraint works." # Reason why the version constraint exists. (Optional) + govet: + # report about shadowed variables + check-shadowing: true + # settings per analyzer + settings: + printf: # analyzer name, run `go tool vet help` to see all analyzers + funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf + # enable all analyzers + enable-all: true + disable: + - fieldalignment + disable-all: false + depguard: + list-type: blacklist + include-go-root: false + packages: + - github.com/sirupsen/logrus + packages-with-error-message: + # specify an error message to output when a blacklisted package is used + - github.com/sirupsen/logrus: "logging is allowed only by logutils.Log" + lll: + # max line length, lines longer will be reported. Default is 120. + # '\t' is counted as 1 character by default, and can be changed with the tab-width option + line-length: 120 + # tab width in spaces. Default to 1. + tab-width: 1 + maligned: + # print struct with more effective memory layout or not, false by default + suggest-new: true + nakedret: + # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 + max-func-lines: 30 + testpackage: + # regexp pattern to skip files + skip-regexp: (export|internal)_test\.go + unused: + # treat code as a program (not a library) and report unused exported identifiers; default is false. + # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find funcs usages. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + whitespace: + multi-if: false # Enforces newlines (or comments) after every multi-line if statement + multi-func: false # Enforces newlines (or comments) after every multi-line function signature + gci: + local-prefixes: "bitbucket.org" + misspell: + #locale: US + ignore-words: + wsl: + # If true append is only allowed to be cuddled if appending value is + # matching variables, fields or types on line above. Default is true. + strict-append: true + # Allow calls and assignments to be cuddled as long as the lines have any + # matching variables, fields or types. Default is true. + allow-assign-and-call: true + # Allow multiline assignments to be cuddled. Default is true. + allow-multiline-assign: true + # Allow declarations (var) to be cuddled. + allow-cuddle-declarations: false + # Allow trailing comments in ending of blocks + allow-trailing-comment: true + # Force newlines in end of case at this limit (0 = never). + force-case-trailing-whitespace: 0 + # Force cuddling of err checks with err var assignment + force-err-cuddling: false + # Allow leading comments to be separated with empty liens + allow-separated-leading-comment: false + custom: + # Each custom linter should have a unique name. linters: enable: - - goconst - - goerr113 - gofmt - - goimports - - gosec + - govet + - errcheck + - staticcheck + - unused + - gosimple + - ineffassign + - typecheck + # Additional + # - lll + - godox + #- gomnd + - goconst + # - gocognit + # - nestif + # - gomodguard + - nakedret + - gci - misspell - - nilerr - - nilnil - - unparam - - wsl + - gofumpt + - whitespace + - unconvert + - predeclared + - noctx + - dogsled + # - bodyclose + - asciicheck + #- stylecheck + # - unparam + # - wsl + + #disable-all: false + fast: true +issues: + # List of regexps of issue texts to exclude, empty list by default. + # But independently from this option we use default exclude patterns, + # it can be disabled by `exclude-use-default: false`. To list all + # excluded by default patterns execute `golangci-lint run --help` + exclude: + # Excluding configuration per-path, per-linter, per-text and per-source + exclude-rules: + # Exclude some linters from running on tests files. + # Independently from option `exclude` we use default exclude patterns, + # it can be disabled by this option. To list all + # excluded by default patterns execute `golangci-lint run --help`. + # Default value for this option is true. + exclude-use-default: false + # The default value is false. If set to true exclude and exclude-rules + # regular expressions become case sensitive. + exclude-case-sensitive: false + # The list of ids of default excludes to include or disable. By default it's empty. + include: + #- EXC0002 # disable excluding of issues about comments from golint + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + #max-issues-per-linter: 0 + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + #max-same-issues: 0 + # Show only new issues: if there are unstaged changes or untracked files, + # only those changes are analyzed, else only changes in HEAD~ are analyzed. + # It's a super-useful option for integration of golangci-lint into existing + # large codebase. It's not practical to fix all existing issues at the moment + # of integration: much better don't allow issues in new code. + # Default is false. + new: false + # Show only new issues created after git revision `REV` + new-from-rev: "" + # Show only new issues created in git patch with set file path. + #new-from-patch: path/to/patch/file +severity: + # Default value is empty string. + # Set the default severity for issues. If severity rules are defined and the issues + # do not match or no severity is provided to the rule this will be the default + # severity applied. Severities should match the supported severity names of the + # selected out format. + # - Code climate: https://docs.codeclimate.com/docs/issues#issue-severity + # - Checkstyle: https://checkstyle.sourceforge.io/property_types.html#severity + # - Github: https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message + default-severity: error + # The default value is false. + # If set to true severity-rules regular expressions become case sensitive. + case-sensitive: false + # Default value is empty list. + # When a list of severity rules are provided, severity information will be added to lint + # issues. Severity rules have the same filtering capability as exclude rules except you + # are allowed to specify one matcher per severity rule. + # Only affects out formats that support setting severity information. + rules: + - linters: + - gomnd + severity: ignore diff --git a/cmd/pfcpctl/main.go b/cmd/pfcpctl/main.go index 9264095..ee0a7ac 100644 --- a/cmd/pfcpctl/main.go +++ b/cmd/pfcpctl/main.go @@ -17,7 +17,6 @@ func main() { parser := flags.NewNamedParser(path.Base(os.Args[0]), flags.HelpFlag|flags.PassDoubleDash|flags.PassAfterNonOption) _, err := parser.AddGroup("Global Options", "", &config.GlobalOptions) - if err != nil { panic(err) } @@ -33,7 +32,10 @@ func main() { if ok { realF := err.(*flags.Error) if realF.Type == flags.ErrHelp { - os.Stdout.WriteString(err.Error() + "\n") + _, err = os.Stdout.WriteString(err.Error() + "\n") + if err != nil { + fmt.Println(err) + } return } } diff --git a/internal/pfcpctl/commands/helpers.go b/internal/pfcpctl/commands/helpers.go index 1b736c6..8a403d8 100644 --- a/internal/pfcpctl/commands/helpers.go +++ b/internal/pfcpctl/commands/helpers.go @@ -15,16 +15,19 @@ var conn *grpc.ClientConn func connect() pb.PFCPSimClient { // Create an insecure gRPC Channel - conn, err := grpc.Dial(config.GlobalConfig.Server, grpc.WithTransportCredentials(insecure.NewCredentials())) + connection, err := grpc.Dial(config.GlobalConfig.Server, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { log.Fatalf("Error dialing %v: %v", config.GlobalConfig.Server, err) } - return pb.NewPFCPSimClient(conn) + return pb.NewPFCPSimClient(connection) } func disconnect() { if conn != nil { - conn.Close() + err := conn.Close() + if err != nil { + log.Warnln(err) + } } } diff --git a/internal/pfcpctl/commands/services.go b/internal/pfcpctl/commands/services.go index 59f2e8c..82a9d0d 100644 --- a/internal/pfcpctl/commands/services.go +++ b/internal/pfcpctl/commands/services.go @@ -11,12 +11,14 @@ import ( log "github.com/sirupsen/logrus" ) -type associate struct{} -type disassociate struct{} -type configureRemoteAddresses struct { - RemotePeerAddress string `short:"r" long:"remote-peer-addr" default:"" description:"The remote PFCP agent address."` - N3InterfaceAddress string `short:"n" long:"n3-addr" default:"" description:"The IPv4 address of the UPF's N3 interface"` -} +type ( + associate struct{} + disassociate struct{} + configureRemoteAddresses struct { + RemotePeerAddress string `short:"r" long:"remote-peer-addr" default:"" description:"The remote PFCP agent address."` + N3InterfaceAddress string `short:"n" long:"n3-addr" default:"" description:"The IPv4 address of the UPF's N3 interface"` + } +) type serviceOptions struct { Associate associate `command:"associate"` @@ -25,7 +27,10 @@ type serviceOptions struct { } func RegisterServiceCommands(parser *flags.Parser) { - _, _ = parser.AddCommand("service", "configure pfcpsim", "Command to configure pfcpsim", &serviceOptions{}) + _, err := parser.AddCommand("service", "configure pfcpsim", "Command to configure pfcpsim", &serviceOptions{}) + if err != nil { + log.Warnln(err) + } } func (c *configureRemoteAddresses) Execute(args []string) error { @@ -37,7 +42,6 @@ func (c *configureRemoteAddresses) Execute(args []string) error { UpfN3Address: c.N3InterfaceAddress, RemotePeerAddress: c.RemotePeerAddress, }) - if err != nil { log.Fatalf("Error while configuring remote addresses: %v", err) } diff --git a/internal/pfcpctl/commands/sessions.go b/internal/pfcpctl/commands/sessions.go index a0e914e..eba267d 100644 --- a/internal/pfcpctl/commands/sessions.go +++ b/internal/pfcpctl/commands/sessions.go @@ -57,7 +57,10 @@ type SessionOptions struct { } func RegisterSessionCommands(parser *flags.Parser) { - _, _ = parser.AddCommand("session", "Handle sessions", "Command to create/modify/delete sessions", &SessionOptions{}) + _, err := parser.AddCommand("session", "Handle sessions", "Command to create/modify/delete sessions", &SessionOptions{}) + if err != nil { + log.Warnln(err) + } } func (s *sessionCreate) Execute(args []string) error { @@ -79,7 +82,6 @@ func (s *sessionCreate) Execute(args []string) error { AppFilters: s.Args.AppFilterString, Qfi: int32(s.Args.QFI), }) - if err != nil { log.Fatalf("Error while creating sessions: %v", err) } @@ -105,7 +107,6 @@ func (s *sessionModify) Execute(args []string) error { NotifyCPFlag: s.Args.NotifyCPFlag, AppFilters: s.Args.AppFilterString, }) - if err != nil { log.Fatalf("Error while modifying sessions: %v", err) } diff --git a/internal/pfcpctl/config/config.go b/internal/pfcpctl/config/config.go index 4cf8543..98138bb 100644 --- a/internal/pfcpctl/config/config.go +++ b/internal/pfcpctl/config/config.go @@ -43,7 +43,7 @@ func ProcessGlobalOptions() { log.Fatal("Server is not set. Please use the -s option") } - //Try to resolve hostname if provided for the server + // Try to resolve hostname if provided for the server if host, port, err := net.SplitHostPort(GlobalConfig.Server); err == nil { if addrs, err := net.LookupHost(host); err == nil { GlobalConfig.Server = net.JoinHostPort(addrs[0], port) diff --git a/internal/pfcpsim/helpers.go b/internal/pfcpsim/helpers.go index ea06c3a..ae58011 100644 --- a/internal/pfcpsim/helpers.go +++ b/internal/pfcpsim/helpers.go @@ -16,8 +16,10 @@ import ( "google.golang.org/grpc/status" ) -const sdfFilterFormatWPort = "permit out %v from %v to assigned %v-%v" -const sdfFilterFormatWOPort = "permit out %v from %v to assigned" +const ( + sdfFilterFormatWPort = "permit out %v from %v to assigned %v-%v" + sdfFilterFormatWOPort = "permit out %v from %v to assigned" +) func connectPFCPSim() error { if sim == nil { @@ -77,7 +79,10 @@ func getLocalAddress(interfaceName string) (net.IP, error) { return nil, err } - addrs, _ = interfaceAddrs.Addrs() + addrs, err = interfaceAddrs.Addrs() + if err != nil { + return nil, err + } } for _, address := range addrs { diff --git a/internal/pfcpsim/helpers_test.go b/internal/pfcpsim/helpers_test.go index fa82fa8..ab57993 100644 --- a/internal/pfcpsim/helpers_test.go +++ b/internal/pfcpsim/helpers_test.go @@ -27,7 +27,8 @@ func Test_parseAppFilter(t *testing.T) { want *want wantErr bool }{ - {name: "Correct app filter", + { + name: "Correct app filter", args: &args{ filterString: "udp:10.0.0.0/8:80-80:allow:100", }, @@ -37,7 +38,8 @@ func Test_parseAppFilter(t *testing.T) { precedence: 100, }, }, - {name: "Correct app filter with deny", + { + name: "Correct app filter with deny", args: &args{ filterString: "udp:10.0.0.0/8:80-80:deny:101", }, @@ -47,7 +49,8 @@ func Test_parseAppFilter(t *testing.T) { precedence: 101, }, }, - {name: "Correct app filter with deny-all policy", + { + name: "Correct app filter with deny-all policy", args: &args{ filterString: "ip:0.0.0.0/0:any:deny:102", }, @@ -57,7 +60,8 @@ func Test_parseAppFilter(t *testing.T) { precedence: 102, }, }, - {name: "Correct app filter with deny-all policy 2", + { + name: "Correct app filter with deny-all policy 2", args: &args{ filterString: "ip:any:any:deny:100", }, @@ -67,7 +71,8 @@ func Test_parseAppFilter(t *testing.T) { precedence: 100, }, }, - {name: "Correct app filter with allow-all policy", + { + name: "Correct app filter with allow-all policy", args: &args{ filterString: "ip:any:any:allow:100", }, @@ -77,7 +82,8 @@ func Test_parseAppFilter(t *testing.T) { precedence: 100, }, }, - {name: "Correct app filter with allow-all policy 2", + { + name: "Correct app filter with allow-all policy 2", args: &args{ filterString: "ip:0.0.0.0/0:any:allow:103", }, @@ -87,28 +93,32 @@ func Test_parseAppFilter(t *testing.T) { precedence: 103, }, }, - {name: "incorrect app filter bad protocol", + { + name: "incorrect app filter bad protocol", args: &args{ filterString: "test:10.0.0.0/8:80-80:allow", }, want: &want{}, wantErr: true, }, - {name: "incorrect app filter bad IP format", + { + name: "incorrect app filter bad IP format", args: &args{ filterString: "ip:10/8:80-80:allow", }, want: &want{}, wantErr: true, }, - {name: "incorrect app filter missing precedence", + { + name: "incorrect app filter missing precedence", args: &args{ filterString: "ip:10/8:80-80:allow", }, want: &want{}, wantErr: true, }, - {name: "incorrect app filter bad precedence", + { + name: "incorrect app filter bad precedence", args: &args{ filterString: "ip:10/8:80-80:allow:test", }, diff --git a/pkg/pfcpsim/pfcpsim.go b/pkg/pfcpsim/pfcpsim.go index 66e9bb6..b7cb0e2 100644 --- a/pkg/pfcpsim/pfcpsim.go +++ b/pkg/pfcpsim/pfcpsim.go @@ -164,7 +164,10 @@ func (c *PFCPClient) DisconnectN4() { c.cancelHeartbeats() } - c.conn.Close() + err := c.conn.Close() + if err != nil { + fmt.Println(err) + } } func (c *PFCPClient) PeekNextHeartbeatResponse() (*message.HeartbeatResponse, error) { @@ -410,8 +413,8 @@ func (c *PFCPClient) EstablishSession(pdrs []*ieLib.IE, fars []*ieLib.IE, qers [ return nil, NewInvalidResponseError(err) } - if cause, err := estResp.Cause.Cause(); err != nil || cause != ieLib.CauseRequestAccepted { - return nil, NewInvalidCauseError(err) + if cause, errCause := estResp.Cause.Cause(); errCause != nil || cause != ieLib.CauseRequestAccepted { + return nil, NewInvalidCauseError(errCause) } remoteSEID, err := estResp.UPFSEID.FSEID() diff --git a/pkg/pfcpsim/session/far_builder.go b/pkg/pfcpsim/session/far_builder.go index 32fa580..e6a1c3c 100644 --- a/pkg/pfcpsim/session/far_builder.go +++ b/pkg/pfcpsim/session/far_builder.go @@ -101,7 +101,7 @@ func (b *farBuilder) BuildFAR() *ie.IE { if b.zeroBasedOuterHeader { fwdParams.Add(ie.NewOuterHeaderCreation(S_TAG, 0, "0.0.0.0", "", 0, 0, 0)) - } else if b.downlinkIP != "" { //TODO revisit code and improve its structure + } else if b.downlinkIP != "" { // TODO revisit code and improve its structure // TEID and DownlinkIP are provided fwdParams.Add(ie.NewOuterHeaderCreation(S_TAG, b.teid, b.downlinkIP, "", 0, 0, 0)) }