From 33b74d458a6983fc4ae21614e761f8696995a6a8 Mon Sep 17 00:00:00 2001 From: Firas Qutishat Date: Wed, 4 Apr 2018 10:30:27 -0400 Subject: [PATCH] [FAB-9312] Resolve metalinter warnings Change-Id: Ic4ade36c78268b9870504b339fcdad3413dcab1a Signed-off-by: Firas Qutishat --- pkg/client/resmgmt/resmgmt.go | 1 + pkg/client/resmgmt/resmgmt_test.go | 29 ++++++------- pkg/context/context.go | 22 +++++++--- pkg/core/config/comm/comm_test.go | 3 +- pkg/core/config/cryptoutil/cryptoutils.go | 2 +- pkg/core/config/endpoint/endpoint.go | 3 -- pkg/core/config/endpoint/endpoint_test.go | 50 +++++++++++++---------- test/scripts/check_lint.sh | 23 +++++++++++ test/scripts/dependencies.sh | 7 +++- 9 files changed, 91 insertions(+), 49 deletions(-) diff --git a/pkg/client/resmgmt/resmgmt.go b/pkg/client/resmgmt/resmgmt.go index 77783eecc9..f152421c3e 100644 --- a/pkg/client/resmgmt/resmgmt.go +++ b/pkg/client/resmgmt/resmgmt.go @@ -4,6 +4,7 @@ Copyright SecureKey Technologies Inc. All Rights Reserved. SPDX-License-Identifier: Apache-2.0 */ +// Package resmgmt enables ability to update resources in a Fabric network. package resmgmt import ( diff --git a/pkg/client/resmgmt/resmgmt_test.go b/pkg/client/resmgmt/resmgmt_test.go index c564ee8312..739f0f10fb 100644 --- a/pkg/client/resmgmt/resmgmt_test.go +++ b/pkg/client/resmgmt/resmgmt_test.go @@ -494,12 +494,12 @@ func TestInstallCCWithOpts(t *testing.T) { assert.Nil(t, err, "marshal should not have failed") // Setup targets - peer := fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", + peer1 := fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", Status: http.StatusOK, MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP", Payload: responseBytes} // Already installed chaincode request req := InstallCCRequest{Name: "name", Version: "version", Path: "path", Package: &api.CCPackage{Type: 1, Code: []byte("code")}} - responses, err := rc.InstallCC(req, WithTargets(&peer)) + responses, err := rc.InstallCC(req, WithTargets(&peer1)) if err != nil { t.Fatal(err) } @@ -512,18 +512,18 @@ func TestInstallCCWithOpts(t *testing.T) { t.Fatal("Should have 'already installed' info set") } - if responses[0].Target != peer.MockURL { - t.Fatalf("Expecting %s target URL, got %s", peer.MockURL, responses[0].Target) + if responses[0].Target != peer1.MockURL { + t.Fatalf("Expecting %s target URL, got %s", peer1.MockURL, responses[0].Target) } // Chaincode not found request (it will be installed) req = InstallCCRequest{Name: "ID", Version: "v0", Path: "path", Package: &api.CCPackage{Type: 1, Code: []byte("code")}} - responses, err = rc.InstallCC(req, WithTargets(&peer)) + responses, err = rc.InstallCC(req, WithTargets(&peer1)) if err != nil { t.Fatal(err) } - if responses[0].Target != peer.MockURL { + if responses[0].Target != peer1.MockURL { t.Fatal("Wrong target URL set") } @@ -542,11 +542,11 @@ func TestInstallCCWithOpts(t *testing.T) { response.Chaincodes = chaincodes responseBytes, _ = proto.Marshal(response) - peer = fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", + peer1 = fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", Status: http.StatusOK, MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP", Payload: responseBytes} req = InstallCCRequest{Name: "error", Version: "v0", Path: "", Package: &api.CCPackage{Type: 1, Code: []byte("code")}} - _, err = rc.InstallCC(req, WithTargets(&peer)) + _, err = rc.InstallCC(req, WithTargets(&peer1)) if err == nil { t.Fatalf("Should have failed since install cc returns an error in the client") } @@ -571,12 +571,12 @@ func TestInstallError(t *testing.T) { func TestInstallCC(t *testing.T) { rc := setupDefaultResMgmtClient(t) - peer := fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", + peer2 := fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", Status: http.StatusOK, MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP"} // Chaincode that is not installed already (it will be installed) req := InstallCCRequest{Name: "ID", Version: "v0", Path: "path", Package: &api.CCPackage{Type: 1, Code: []byte("code")}} - responses, err := rc.InstallCC(req, WithTargets(&peer)) + responses, err := rc.InstallCC(req, WithTargets(&peer2)) if err != nil { t.Fatal(err) } @@ -701,8 +701,7 @@ func TestInstallCCWithOptsRequiredParameters(t *testing.T) { // Setup targets var peers []fab.Peer - peer := fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP"} - peers = append(peers, &peer) + peers = append(peers, &fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP"}) // Test both targets and filter provided (error condition) _, err = rc.InstallCC(req, WithTargets(peers...), WithTargetFilter(&mspFilter{mspID: "Org1MSP"})) @@ -877,8 +876,7 @@ func TestInstantiateCCWithOptsRequiredParameters(t *testing.T) { // Setup targets var peers []fab.Peer - peer := fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP"} - peers = append(peers, &peer) + peers = append(peers, &fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP"}) // Test both targets and filter provided (error condition) _, err = rc.InstantiateCC("mychannel", req, WithTargets(peers...), WithTargetFilter(&mspFilter{mspID: "Org1MSP"})) @@ -1083,8 +1081,7 @@ func TestUpgradeCCWithOptsRequiredParameters(t *testing.T) { // Setup targets var peers []fab.Peer - peer := fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP"} - peers = append(peers, &peer) + peers = append(peers, &fcmocks.MockPeer{MockName: "Peer1", MockURL: "http://peer1.com", MockRoles: []string{}, MockCert: nil, MockMSP: "Org1MSP"}) // Test both targets and filter provided (error condition) _, err = rc.UpgradeCC("mychannel", req, WithTargets(peers...), WithTargetFilter(&mspFilter{mspID: "Org1MSP"})) diff --git a/pkg/context/context.go b/pkg/context/context.go index b08176f05c..8ece10d4c2 100644 --- a/pkg/context/context.go +++ b/pkg/context/context.go @@ -251,21 +251,33 @@ func NewChannel(clientProvider context.ClientProvider, channelID string) (*Chann channelService: channelService, channelID: channelID, } + err = initialize(channel, channelService, discoveryService, selectionService) + if err != nil { + return nil, err + } + return channel, nil +} +func initialize(channel *Channel, channelService fab.ChannelService, discoveryService fab.DiscoveryService, selectionService fab.SelectionService) error { //initialize if pi, ok := channelService.(serviceInit); ok { - pi.Initialize(channel) + if err := pi.Initialize(channel); err != nil { + return err + } } if pi, ok := discoveryService.(serviceInit); ok { - pi.Initialize(channel) + if err := pi.Initialize(channel); err != nil { + return err + } } if pi, ok := selectionService.(serviceInit); ok { - pi.Initialize(channel) + if err := pi.Initialize(channel); err != nil { + return err + } } - - return channel, nil + return nil } type reqContextKey string diff --git a/pkg/core/config/comm/comm_test.go b/pkg/core/config/comm/comm_test.go index b35b177751..b174825948 100644 --- a/pkg/core/config/comm/comm_test.go +++ b/pkg/core/config/comm/comm_test.go @@ -132,8 +132,7 @@ func TestTlsCertHash(t *testing.T) { if err != nil { t.Fatalf("Unexpected error decoding cert fingerprint %v", err) } - - if bytes.Compare(tlsCertHash, expectedHash) != 0 { + if !bytes.Equal(tlsCertHash, expectedHash) { t.Fatal("Cert hash calculated incorrectly") } } diff --git a/pkg/core/config/cryptoutil/cryptoutils.go b/pkg/core/config/cryptoutil/cryptoutils.go index 1bc7d31627..2bed3a689d 100644 --- a/pkg/core/config/cryptoutil/cryptoutils.go +++ b/pkg/core/config/cryptoutil/cryptoutils.go @@ -43,7 +43,7 @@ func GetPrivateKeyFromCert(cert []byte, cs core.CryptoSuite) (core.Key, error) { return nil, errors.WithMessage(err, "Could not find matching key for SKI") } - if key != nil && key.Private() == false { + if key != nil && !key.Private() { return nil, errors.Errorf("Found key is not private, SKI: %s", certPubK.SKI()) } diff --git a/pkg/core/config/endpoint/endpoint.go b/pkg/core/config/endpoint/endpoint.go index d1c79d8d89..c4a232127e 100644 --- a/pkg/core/config/endpoint/endpoint.go +++ b/pkg/core/config/endpoint/endpoint.go @@ -15,12 +15,9 @@ import ( "regexp" "github.com/hyperledger/fabric-sdk-go/pkg/common/errors/status" - "github.com/hyperledger/fabric-sdk-go/pkg/common/logging" "github.com/pkg/errors" ) -var logger = logging.NewLogger("fabsdk/core") - // IsTLSEnabled is a generic function that expects a URL and verifies if it has // a prefix HTTPS or GRPCS to return true for TLS Enabled URLs or false otherwise func IsTLSEnabled(url string) bool { diff --git a/pkg/core/config/endpoint/endpoint_test.go b/pkg/core/config/endpoint/endpoint_test.go index 7a4ad02de8..c5029e7ae9 100644 --- a/pkg/core/config/endpoint/endpoint_test.go +++ b/pkg/core/config/endpoint/endpoint_test.go @@ -137,7 +137,7 @@ O94CDp7l2k7hMQI0zQ== } } -func TestTLSConfig_TLSCert(t *testing.T) { +func TestTLSConfig_TLSCertPostive(t *testing.T) { tlsConfig := &TLSConfig{ Path: "../../../../test/fixtures/config/mutual_tls/client_sdk_go.pem", Pem: "", @@ -151,26 +151,6 @@ func TestTLSConfig_TLSCert(t *testing.T) { t.Fatalf("cert's TLSCert() call returned empty certificate") } - // test with wrong path - tlsConfig.Path = "dummy/path" - c, e = tlsConfig.TLSCert() - if e == nil { - t.Fatal("expected error loading certificate for wrong cert path") - } - if c != nil { - t.Fatalf("cert's TLSCert() call returned non empty certificate for wrong cert path") - } - - // test with empty path and empty pem - tlsConfig.Path = "" - c, e = tlsConfig.TLSCert() - if e == nil { - t.Fatal("expected error loading certificate for empty cert path and empty pem") - } - if c != nil { - t.Fatalf("cert's TLSCert() call returned non empty certificate for wrong cert path and empty pem") - } - // test with both correct pem and path set tlsConfig.Path = "../../../../test/fixtures/config/mutual_tls/client_sdk_go.pem" tlsConfig.Pem = `-----BEGIN CERTIFICATE----- @@ -196,6 +176,33 @@ O94CDp7l2k7hMQI0zQ== t.Fatalf("cert's TLSCert() call returned empty certificate") } +} + +func TestTLSConfig_TLSCertNegative(t *testing.T) { + + // test with wrong path + tlsConfig := &TLSConfig{ + Path: "dummy/path", + Pem: "", + } + c, e := tlsConfig.TLSCert() + if e == nil { + t.Fatal("expected error loading certificate for wrong cert path") + } + if c != nil { + t.Fatalf("cert's TLSCert() call returned non empty certificate for wrong cert path") + } + + // test with empty path and empty pem + tlsConfig.Path = "" + c, e = tlsConfig.TLSCert() + if e == nil { + t.Fatal("expected error loading certificate for empty cert path and empty pem") + } + if c != nil { + t.Fatalf("cert's TLSCert() call returned non empty certificate for wrong cert path and empty pem") + } + // test with wrong pem and empty path tlsConfig.Path = "" tlsConfig.Pem = "wrongcertpem" @@ -206,4 +213,5 @@ O94CDp7l2k7hMQI0zQ== if c != nil { t.Fatalf("cert's TLSCert() call returned non empty certificate") } + } diff --git a/test/scripts/check_lint.sh b/test/scripts/check_lint.sh index ee21572978..7818518514 100755 --- a/test/scripts/check_lint.sh +++ b/test/scripts/check_lint.sh @@ -8,10 +8,16 @@ set -e + + GO_CMD="${GO_CMD:-go}" GOLINT_CMD=golint GOFMT_CMD=gofmt GOIMPORTS_CMD=goimports +GOMETALINT_CMD=gometalinter + + + PROJECT_PATH=$GOPATH/src/github.com/hyperledger/fabric-sdk-go @@ -54,3 +60,20 @@ do exit 1 fi done + + + + +declare -a arr1=( +"./pkg/client" +"./pkg/common" +"./pkg/context" +) + + +echo "Running metalinters..." +for i in "${arr1[@]}" +do + echo "Checking $i" + $GOMETALINT_CMD $i/... +done \ No newline at end of file diff --git a/test/scripts/dependencies.sh b/test/scripts/dependencies.sh index c250642e4f..c9adb50f0a 100755 --- a/test/scripts/dependencies.sh +++ b/test/scripts/dependencies.sh @@ -23,9 +23,12 @@ if [ "$FABRIC_SDKGO_DEPEND_INSTALL" = "true" ]; then GOPATH=$TMP $GO_CMD get -u github.com/golang/lint/golint GOPATH=$TMP $GO_CMD get -u golang.org/x/tools/cmd/goimports GOPATH=$TMP $GO_CMD get -u github.com/golang/mock/mockgen - + GOPATH=$TMP $GO_CMD get -u github.com/alecthomas/gometalinter mkdir -p $GOPATH/bin cp $TMP/bin/* $GOPATH/bin + mkdir -p $GOPATH/src/github.com/alecthomas/gometalinter + cp -R $TMP/src/github.com/alecthomas/gometalinter/* $GOPATH/src/github.com/alecthomas/gometalinter + gometalinter --install fi # Install specific version of go dep (particularly for CI) @@ -49,6 +52,8 @@ type golint >/dev/null 2>&1 || { echo >& 2 "golint is not installed (go get -u g type goimports >/dev/null 2>&1 || { echo >& 2 "goimports is not installed (go get -u golang.org/x/tools/cmd/goimports)"; ABORT=1; } type mockgen >/dev/null 2>&1 || { echo >& 2 "mockgen is not installed (go get -u github.com/golang/mock/mockgen)"; ABORT=1; } type $GO_DEP_CMD >/dev/null 2>&1 || { echo >& 2 "dep is not installed (go get -u github.com/golang/dep/cmd/dep)"; ABORT=1; } +type gometalinter >/dev/null 2>&1 || { echo >& 2 "gometalinter is not installed (go get -u github.com/alecthomas/gometalinter)"; ABORT=1; } + if [ -n "$ABORT" ]; then echo "Missing dependency. Aborting. You can fix by installing the tool listed above or running make depend-install."