Skip to content

Commit

Permalink
[FAB-9312] Resolve metalinter warnings
Browse files Browse the repository at this point in the history
Change-Id: Ic4ade36c78268b9870504b339fcdad3413dcab1a
Signed-off-by: Firas Qutishat <firas.qutishat@securekey.com>
  • Loading branch information
fqutishat committed Apr 4, 2018
1 parent 582c2fc commit 33b74d4
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 49 deletions.
1 change: 1 addition & 0 deletions pkg/client/resmgmt/resmgmt.go
Expand Up @@ -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 (
Expand Down
29 changes: 13 additions & 16 deletions pkg/client/resmgmt/resmgmt_test.go
Expand Up @@ -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)
}
Expand All @@ -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")
}

Expand All @@ -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")
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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"}))
Expand Down Expand Up @@ -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"}))
Expand Down Expand Up @@ -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"}))
Expand Down
22 changes: 17 additions & 5 deletions pkg/context/context.go
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions pkg/core/config/comm/comm_test.go
Expand Up @@ -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")
}
}
2 changes: 1 addition & 1 deletion pkg/core/config/cryptoutil/cryptoutils.go
Expand Up @@ -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())
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/core/config/endpoint/endpoint.go
Expand Up @@ -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 {
Expand Down
50 changes: 29 additions & 21 deletions pkg/core/config/endpoint/endpoint_test.go
Expand Up @@ -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: "",
Expand All @@ -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-----
Expand All @@ -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"
Expand All @@ -206,4 +213,5 @@ O94CDp7l2k7hMQI0zQ==
if c != nil {
t.Fatalf("cert's TLSCert() call returned non empty certificate")
}

}
23 changes: 23 additions & 0 deletions test/scripts/check_lint.sh
Expand Up @@ -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

Expand Down Expand Up @@ -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
7 changes: 6 additions & 1 deletion test/scripts/dependencies.sh
Expand Up @@ -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)
Expand All @@ -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."
Expand Down

0 comments on commit 33b74d4

Please sign in to comment.