diff --git a/.github/workflows/codeql-analysis.yaml b/.github/workflows/codeql-analysis.yaml new file mode 100644 index 0000000..4520ebf --- /dev/null +++ b/.github/workflows/codeql-analysis.yaml @@ -0,0 +1,67 @@ +# For most projects, this workflow file will not need changing; you simply need +# to commit it to your repository. +# +# You may wish to alter this file to override the set of languages analyzed, +# or to provide custom queries or build logic. +# +# ******** NOTE ******** +# We have attempted to detect the languages in your repository. Please check +# the `language` matrix defined below to confirm you have the correct set of +# supported CodeQL languages. +# +name: "CodeQL" + +on: + push: + pull_request: + schedule: + - cron: '15 9 * * 2' + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + + strategy: + fail-fast: false + matrix: + language: [ 'go' ] + # CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby' ] + # Learn more about CodeQL language support at https://git.io/codeql-language-support + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: ${{ matrix.language }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. + # queries: ./path/to/local/query, your-org/your-repo/queries@main + + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). + # If this step fails, then you should remove it and run the build manually (see below) + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + + # ℹī¸ Command-line programs to run using the OS shell. + # 📚 https://git.io/JvXDl + + # ✏ī¸ If the Autobuild fails above, remove it and uncomment the following three lines + # and modify them (or add more) to build your code if your project + # uses a compiled language + + #- run: | + # make bootstrap + # make release + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml new file mode 100644 index 0000000..bbcfd9e --- /dev/null +++ b/.github/workflows/golangci-lint.yaml @@ -0,0 +1,32 @@ +# +# This file is part of Astarte. +# +# Copyright 2020 Ispirata Srl +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +name: "Static code checking" +on: + pull_request: + push: + +jobs: + golangci-lint: + runs-on: ubuntu-20.04 + steps: + - uses: actions/checkout@v2 + # Run golint-ci + - uses: golangci/golangci-lint-action@v2 + with: + version: v1.44 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..2d78d61 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,176 @@ +# This file contains all available configuration options +# with their default values. + +# options for analysis running +run: + # timeout for analysis, e.g. 30s, 5m, default is 1m + # Allow 10m, within actions it might take a lot + timeout: 10m + + # which dirs to skip: issues from them won't be reported + skip-dirs: + - external + + +# output configuration options +output: + # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number" + format: line-number + +# all available settings of specific linters +linters-settings: + dogsled: + # checks assignments with too many blank identifiers; default is 2 + max-blank-identifiers: 2 + dupl: + # tokens count to trigger issue, 150 by default + threshold: 100 + 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: false + + # [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: SetPrerelease + funlen: + lines: 150 + statements: 40 + gocognit: + # Keep 30, so we report only truly insane things. Reconciliation functions will always be + # a little bit more complex than needed + min-complexity: 30 + goconst: + # minimal length of string constant, 3 by default + min-len: 3 + # minimal occurrences count to trigger, 3 by default + min-occurrences: 3 + gocritic: + disabled-checks: + # This is a little bit *too* strict, disable it + - commentFormatting + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 15 + 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 + - NOTE + - OPTIMIZE # marks code that should be optimized before merging + - HACK # marks hack-arounds that should be removed before merging + 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/astarte-platform/astarte-device-sdk-go + 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] + govet: + # report about shadowed variables + check-shadowing: true + enable-all: true + disable: + - fieldalignment + 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 + misspell: + # Correct spellings using locale preferences for US or UK. + # Default is to use a neutral variety of English. + # Setting locale to US will correct the British spelling of 'colour' to 'color'. + locale: US + nakedret: + prealloc: + rowserrcheck: + unparam: + unused: + 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 + 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: false + # 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 + +linters: + enable: + - bodyclose + - dupl + - funlen + - gocognit + - goconst + - gocritic + - gocyclo + - goimports + - goprintffuncname + - gosec + #- lll + - unconvert + - unparam + + fast: false + + # Excluding configuration per-path, per-linter, per-text and per-source + exclude-rules: + # Exclude some linters from running on tests files. + - path: _test\.go + linters: + - gocyclo + - errcheck + - dupl + - gosec + - funlen + + # Exclude lll issues for long lines with go:generate + - linters: + - lll + source: "^//go:generate " + + # 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 diff --git a/device/crypto.go b/device/crypto.go index 698847c..ea6ec62 100644 --- a/device/crypto.go +++ b/device/crypto.go @@ -97,7 +97,9 @@ func (d *Device) getTLSConfig() (*tls.Config, error) { func (d *Device) getCryptoDir() string { cryptoDir := filepath.Join(d.persistencyDir, "crypto") - os.MkdirAll(cryptoDir, 0700) + if err := os.MkdirAll(cryptoDir, 0700); err != nil { + fmt.Println("WARNING - could not access crypto dir!") + } return cryptoDir } @@ -183,7 +185,9 @@ func (d *Device) ensureKeyPair() error { // We need to generate the key // First of all, clear the crypto dir, just to be sure. - d.ClearCrypto() + if err := d.ClearCrypto(); err != nil { + return err + } reader := rand.Reader // Certificates are short-lived, 2048 is fine. diff --git a/device/device.go b/device/device.go index d045ba2..774204d 100644 --- a/device/device.go +++ b/device/device.go @@ -21,11 +21,11 @@ import ( "io/ioutil" "path/filepath" - mqtt "github.com/ispirata/paho.mqtt.golang" "github.com/astarte-platform/astarte-go/client" "github.com/astarte-platform/astarte-go/interfaces" "github.com/astarte-platform/astarte-go/misc" backoff "github.com/cenkalti/backoff/v4" + mqtt "github.com/ispirata/paho.mqtt.golang" "gorm.io/driver/sqlite" "gorm.io/gorm" ) @@ -115,7 +115,6 @@ func newDevice(deviceID, realm, credentialsSecret string, pairingBaseURL string, } if err := d.migrateDb(); err != nil { - errors.New("Database migration failed") return nil, err } @@ -123,6 +122,7 @@ func newDevice(deviceID, realm, credentialsSecret string, pairingBaseURL string, } // Connect connects the device through a goroutine +//nolint func (d *Device) Connect(result chan<- error) { go func(result chan<- error) { // Are we connected already? @@ -177,7 +177,7 @@ func (d *Device) Connect(result chan<- error) { } // initialize the client - if err := d.initializeMQTTClient(); err != nil { + if err = d.initializeMQTTClient(); err != nil { if result != nil { result <- err } diff --git a/device/protocol_mqtt_v1.go b/device/protocol_mqtt_v1.go index c7a995c..0b9c637 100644 --- a/device/protocol_mqtt_v1.go +++ b/device/protocol_mqtt_v1.go @@ -50,7 +50,7 @@ func (d *Device) initializeMQTTClient() error { opts.SetTLSConfig(tlsConfig) opts.SetOnConnectHandler(func(client mqtt.Client, sessionPresent bool) { - astarteOnConnectHandler(d, client, sessionPresent) + astarteOnConnectHandler(d, sessionPresent) }) opts.SetConnectionLostHandler(func(client mqtt.Client, err error) { @@ -64,120 +64,127 @@ func (d *Device) initializeMQTTClient() error { }) // This is our message handler - opts.SetDefaultPublishHandler(func(client mqtt.Client, msg mqtt.Message) { - if !strings.HasPrefix(msg.Topic(), d.getBaseTopic()) { - if d.OnErrors != nil { - d.OnErrors(d, fmt.Errorf("Received message on unexpected topic %s. This is an internal error", msg.Topic())) - } - return + opts.SetDefaultPublishHandler(d.astarteGoSDKDefaultPublishHandler) + + d.m = mqtt.NewClient(opts) + + return nil +} + +func (d *Device) astarteGoSDKDefaultPublishHandler(client mqtt.Client, msg mqtt.Message) { + if !strings.HasPrefix(msg.Topic(), d.getBaseTopic()) { + if d.OnErrors != nil { + d.OnErrors(d, fmt.Errorf("Received message on unexpected topic %s. This is an internal error", msg.Topic())) } + return + } - // We split up to 4 since this will give us the path in the correct format. - tokens := strings.SplitN(msg.Topic(), "/", 4) - if len(tokens) > 2 { - // Is it a control message? - if tokens[2] == "control" { - err := d.handleControlMessages(strings.Join(tokens[3:], "/"), msg.Payload()) - if err != nil { - d.OnErrors(d, err) - } - return + // We split up to 4 since this will give us the path in the correct format. + tokens := strings.SplitN(msg.Topic(), "/", 4) + if len(tokens) > 2 { + // Is it a control message? + if tokens[2] == "control" { + err := d.handleControlMessages(strings.Join(tokens[3:], "/"), msg.Payload()) + if err != nil { + d.OnErrors(d, err) } + return + } - // It's a data message. Grab the interface name. - interfaceName := tokens[2] - // Parse the payload - parsed, err := parseBSONPayload(msg.Payload()) - if err != nil { - if d.OnErrors != nil { - d.OnErrors(d, err) - } - return + // It's a data message. Grab the interface name. + interfaceName := tokens[2] + // Parse the payload + parsed, err := parseBSONPayload(msg.Payload()) + if err != nil { + if d.OnErrors != nil { + d.OnErrors(d, err) } - timestamp := time.Time{} - if t, ok := parsed["t"]; ok { - // We have a timestamp - if pT, ok := t.(primitive.DateTime); ok { - timestamp = pT.Time() - } + return + } + timestamp := time.Time{} + if t, ok := parsed["t"]; ok { + // We have a timestamp + if pT, ok := t.(primitive.DateTime); ok { + timestamp = pT.Time() } + } - if iface, ok := d.interfaces[interfaceName]; ok { - // Is it individual? - switch { - case len(tokens) != 4: - if d.OnErrors != nil { - d.OnErrors(d, fmt.Errorf("could not parse incoming message on topic structure %s", tokens)) - } - return - case iface.Aggregation == interfaces.IndividualAggregation: - interfacePath := "/" + tokens[3] + if iface, ok := d.interfaces[interfaceName]; ok { + d.processIncomingMessage(iface, tokens, msg.Payload(), parsed, timestamp) + } else if d.OnErrors != nil { + // Something is off. + d.OnErrors(d, fmt.Errorf("Received message for unregistered interface %s", interfaceName)) + } + } +} - if iface.Type == interfaces.PropertiesType { - d.storeProperty(iface.Name, interfacePath, iface.MajorVersion, msg.Payload()) - } +func (d *Device) processIncomingMessage(iface interfaces.AstarteInterface, tokens []string, payload []byte, parsed map[string]interface{}, timestamp time.Time) { + // Is it individual? + switch { + case len(tokens) != 4: + if d.OnErrors != nil { + d.OnErrors(d, fmt.Errorf("could not parse incoming message on topic structure %s", tokens)) + } + return + case iface.Aggregation == interfaces.IndividualAggregation: + interfacePath := "/" + tokens[3] - // Create the message - m := IndividualMessage{ - Interface: iface, - Path: interfacePath, - Value: parsed["v"], - Timestamp: timestamp, - } - if d.OnIndividualMessageReceived != nil { - d.OnIndividualMessageReceived(d, m) - } - case iface.Aggregation == interfaces.ObjectAggregation: - interfacePath := "/" + tokens[3] - - if val, ok := parsed["v"].(map[string]interface{}); !ok { - d.OnErrors(d, fmt.Errorf("could not parse aggregate message payload")) - } else { - // We have to check whether we have some nested arrays or not in here. - for k, v := range val { - if bsonArray, ok := v.(primitive.A); ok { - // That is, in fact, the case. Convert to a generic Go slice first. - bsonArraySlice := []interface{}(bsonArray) - // Now reflect the heck out of it and specialize the slice - specializedSlice := reflect.MakeSlice(reflect.SliceOf(reflect.TypeOf(bsonArraySlice[0])), len(bsonArraySlice), cap(bsonArraySlice)) - for i := 0; i < specializedSlice.Len(); i++ { - specializedSlice.Index(i).Set(reflect.ValueOf(bsonArraySlice[i])) - } - val[k] = specializedSlice.Interface() - } - } - - // N.B.: properties with object aggregation are not yet supported by Astarte - if iface.Type == interfaces.PropertiesType { - d.storeProperty(iface.Name, interfacePath, iface.MajorVersion, msg.Payload()) - } - - // Create the message - m := AggregateMessage{ - Interface: iface, - Path: interfacePath, - Values: val, - Timestamp: timestamp, - } - - if d.OnAggregateMessageReceived != nil { - d.OnAggregateMessageReceived(d, m) - } + if iface.Type == interfaces.PropertiesType { + d.storeProperty(iface.Name, interfacePath, iface.MajorVersion, payload) + } + + // Create the message + m := IndividualMessage{ + Interface: iface, + Path: interfacePath, + Value: parsed["v"], + Timestamp: timestamp, + } + if d.OnIndividualMessageReceived != nil { + d.OnIndividualMessageReceived(d, m) + } + case iface.Aggregation == interfaces.ObjectAggregation: + interfacePath := "/" + tokens[3] + + if val, ok := parsed["v"].(map[string]interface{}); !ok { + d.OnErrors(d, fmt.Errorf("could not parse aggregate message payload")) + } else { + // We have to check whether we have some nested arrays or not in here. + for k, v := range val { + if bsonArray, ok := v.(primitive.A); ok { + // That is, in fact, the case. Convert to a generic Go slice first. + bsonArraySlice := []interface{}(bsonArray) + // Now reflect the heck out of it and specialize the slice + specializedSlice := reflect.MakeSlice(reflect.SliceOf(reflect.TypeOf(bsonArraySlice[0])), len(bsonArraySlice), cap(bsonArraySlice)) + for i := 0; i < specializedSlice.Len(); i++ { + specializedSlice.Index(i).Set(reflect.ValueOf(bsonArraySlice[i])) } + val[k] = specializedSlice.Interface() } - } else if d.OnErrors != nil { - // Something is off. - d.OnErrors(d, fmt.Errorf("Received message for unregistered interface %s", interfaceName)) } - } - }) - d.m = mqtt.NewClient(opts) + // N.B.: properties with object aggregation are not yet supported by Astarte + if iface.Type == interfaces.PropertiesType { + d.storeProperty(iface.Name, interfacePath, iface.MajorVersion, payload) + } - return nil + // Create the message + m := AggregateMessage{ + Interface: iface, + Path: interfacePath, + Values: val, + Timestamp: timestamp, + } + + if d.OnAggregateMessageReceived != nil { + d.OnAggregateMessageReceived(d, m) + } + } + } } func (d *Device) handleControlMessages(message string, payload []byte) error { + //nolint switch message { case "consumer/properties": return d.handlePurgeProperties(payload) @@ -187,7 +194,7 @@ func (d *Device) handleControlMessages(message string, payload []byte) error { return nil } -func astarteOnConnectHandler(d *Device, client mqtt.Client, sessionPresent bool) { +func astarteOnConnectHandler(d *Device, sessionPresent bool) { // Generate Introspection first introspection := d.generateDeviceIntrospection() @@ -467,7 +474,7 @@ func getIndividualMappingFromAggregate(astarteInterface interfaces.AstarteInterf return astarteInterface.Mappings[0], nil } -func (d *Device) publishMessage(message astarteMessageInfo) error { +func (d *Device) publishMessage(message astarteMessageInfo) { topic := fmt.Sprintf("%s/%s%s", d.getBaseTopic(), message.InterfaceName, message.Path) // MQTT client returns `true` to IsConnected() @@ -499,8 +506,6 @@ func (d *Device) publishMessage(message astarteMessageInfo) error { } } } - - return nil } func (d *Device) storeMessage(message astarteMessageInfo) { @@ -617,68 +622,3 @@ func parseBSONPayload(payload []byte) (map[string]interface{}, error) { err := bson.Unmarshal(payload, parsed) return parsed, err } - -func bsonRawValueToInterface(v bson.RawValue, valueType string) (interface{}, error) { - switch valueType { - case "string": - var val string - err := v.Unmarshal(val) - return val, err - case "double": - var val float64 - err := v.Unmarshal(val) - return val, err - case "integer": - var val int32 - err := v.Unmarshal(val) - return val, err - case "boolean": - var val bool - err := v.Unmarshal(val) - return val, err - case "longinteger": - var val int64 - err := v.Unmarshal(val) - return val, err - case "binaryblob": - var val []byte - err := v.Unmarshal(val) - return val, err - case "datetime": - // TODO: verify this is true. - var val time.Time - err := v.Unmarshal(val) - return val, err - case "stringarray": - var val []string - err := v.Unmarshal(val) - return val, err - case "doublearray": - var val []float64 - err := v.Unmarshal(val) - return val, err - case "integerarray": - var val []int32 - err := v.Unmarshal(val) - return val, err - case "booleanarray": - var val []bool - err := v.Unmarshal(val) - return val, err - case "longintegerarray": - var val []int64 - err := v.Unmarshal(val) - return val, err - case "binaryblobarray": - var val [][]byte - err := v.Unmarshal(val) - return val, err - case "datetimearray": - // TODO: verify this is true. - var val []time.Time - err := v.Unmarshal(val) - return val, err - } - - return nil, fmt.Errorf("Could not decode for type %s", valueType) -} diff --git a/device/store.go b/device/store.go index 4f1c76e..bae8df0 100644 --- a/device/store.go +++ b/device/store.go @@ -54,7 +54,9 @@ type property struct { func (d *Device) getDbDir() string { dbDir := filepath.Join(d.persistencyDir, "db") - os.MkdirAll(dbDir, 0700) + if err := os.MkdirAll(dbDir, 0700); err != nil { + fmt.Println("WARNING - could not access store dir!") + } return dbDir } @@ -195,10 +197,21 @@ func (d *Device) handlePurgeProperties(payload []byte) error { } buf := new(bytes.Buffer) - _, err = io.Copy(buf, flateReader) - if err != nil { - return err + // G110: Copy in chunks + var totalRead, n int64 + for { + n, err = io.CopyN(buf, flateReader, 1024) + totalRead += n + if err != nil { + if err == io.EOF { + // We're done + break + } + // Actual error + return err + } } + if e := flateReader.Close(); e != nil { return e } diff --git a/e2e_tests/e2e_test.go b/e2e_tests/e2e_test.go index 2ac4a4a..f9998b0 100644 --- a/e2e_tests/e2e_test.go +++ b/e2e_tests/e2e_test.go @@ -27,10 +27,12 @@ import ( "testing" "time" - "github.com/astarte-platform/astarte-device-sdk-go/device" + "github.com/stretchr/testify/suite" + "github.com/astarte-platform/astarte-go/client" "github.com/astarte-platform/astarte-go/interfaces" - "github.com/stretchr/testify/suite" + + "github.com/astarte-platform/astarte-device-sdk-go/device" ) var ( @@ -87,7 +89,9 @@ func (suite *EndToEndSuite) TearDownSuite() { func (suite *EndToEndSuite) TestDatastreamIndividualDevice() { // send everything for k, v := range expectedDatastreamIndividual { - suite.d.SendIndividualMessageWithTimestamp("org.astarte-platform.device.individual.datastream.Everything", k, v, time.Now()) + if err := suite.d.SendIndividualMessageWithTimestamp("org.astarte-platform.device.individual.datastream.Everything", k, v, time.Now()); err != nil { + suite.Fail("Error sending message", err) + } fmt.Printf("Sent %v on %s\n", v, k) time.Sleep(1 * time.Second) } @@ -181,6 +185,7 @@ func (suite *EndToEndSuite) setupDevice() { suite.d = d } +//nolint func individualValueToAstarteType(value interface{}, astarteType string) interface{} { // cast like there's no tomorrow yolo switch astarteType {