diff --git a/gometalinter.json b/gometalinter.json index e315443ca9..5245972c98 100644 --- a/gometalinter.json +++ b/gometalinter.json @@ -8,6 +8,7 @@ "Enable": [ "deadcode", "gocyclo", + "gofmt", "gotype", "goimports", "golint", diff --git a/pkg/common/errors/retry/defaults.go b/pkg/common/errors/retry/defaults.go index 3729103d2c..226ec63694 100644 --- a/pkg/common/errors/retry/defaults.go +++ b/pkg/common/errors/retry/defaults.go @@ -68,19 +68,19 @@ var DefaultResMgmtOpts = Opts{ // DefaultRetryableCodes these are the error codes, grouped by source of error, // that are considered to be transient error conditions by default var DefaultRetryableCodes = map[status.Group][]status.Code{ - status.EndorserClientStatus: []status.Code{ + status.EndorserClientStatus: { status.EndorsementMismatch, status.PrematureChaincodeExecution, }, - status.EndorserServerStatus: []status.Code{ + status.EndorserServerStatus: { status.Code(common.Status_SERVICE_UNAVAILABLE), status.Code(common.Status_INTERNAL_SERVER_ERROR), }, - status.OrdererServerStatus: []status.Code{ + status.OrdererServerStatus: { status.Code(common.Status_SERVICE_UNAVAILABLE), status.Code(common.Status_INTERNAL_SERVER_ERROR), }, - status.EventServerStatus: []status.Code{ + status.EventServerStatus: { status.Code(pb.TxValidationCode_DUPLICATE_TXID), status.Code(pb.TxValidationCode_ENDORSEMENT_POLICY_FAILURE), status.Code(pb.TxValidationCode_MVCC_READ_CONFLICT), @@ -88,7 +88,7 @@ var DefaultRetryableCodes = map[status.Group][]status.Code{ }, // TODO: gRPC introduced retries in v1.8.0. This can be replaced with the // gRPC fail fast option, once available - status.GRPCTransportStatus: []status.Code{ + status.GRPCTransportStatus: { status.Code(grpcCodes.Unavailable), }, } @@ -96,21 +96,21 @@ var DefaultRetryableCodes = map[status.Group][]status.Code{ // ResMgmtDefaultRetryableCodes are the suggested codes that should be treated as // transient by fabric-sdk-go/pkg/client/resmgmt.Client var ResMgmtDefaultRetryableCodes = map[status.Group][]status.Code{ - status.EndorserClientStatus: []status.Code{ + status.EndorserClientStatus: { status.EndorsementMismatch, status.PrematureChaincodeExecution, }, - status.EndorserServerStatus: []status.Code{ + status.EndorserServerStatus: { status.Code(common.Status_SERVICE_UNAVAILABLE), status.Code(common.Status_INTERNAL_SERVER_ERROR), }, - status.OrdererServerStatus: []status.Code{ + status.OrdererServerStatus: { status.Code(common.Status_SERVICE_UNAVAILABLE), status.Code(common.Status_INTERNAL_SERVER_ERROR), status.Code(common.Status_BAD_REQUEST), status.Code(common.Status_NOT_FOUND), }, - status.EventServerStatus: []status.Code{ + status.EventServerStatus: { status.Code(pb.TxValidationCode_DUPLICATE_TXID), status.Code(pb.TxValidationCode_ENDORSEMENT_POLICY_FAILURE), status.Code(pb.TxValidationCode_MVCC_READ_CONFLICT), @@ -118,7 +118,7 @@ var ResMgmtDefaultRetryableCodes = map[status.Group][]status.Code{ }, // TODO: gRPC introduced retries in v1.8.0. This can be replaced with the // gRPC fail fast option, once available - status.GRPCTransportStatus: []status.Code{ + status.GRPCTransportStatus: { status.Code(grpcCodes.Unavailable), }, } @@ -126,22 +126,22 @@ var ResMgmtDefaultRetryableCodes = map[status.Group][]status.Code{ // ChannelClientRetryableCodes are the suggested codes that should be treated as // transient by fabric-sdk-go/pkg/client/channel.Client var ChannelClientRetryableCodes = map[status.Group][]status.Code{ - status.EndorserClientStatus: []status.Code{ + status.EndorserClientStatus: { status.ConnectionFailed, status.EndorsementMismatch, status.PrematureChaincodeExecution, }, - status.EndorserServerStatus: []status.Code{ + status.EndorserServerStatus: { status.Code(common.Status_SERVICE_UNAVAILABLE), status.Code(common.Status_INTERNAL_SERVER_ERROR), }, - status.OrdererClientStatus: []status.Code{ + status.OrdererClientStatus: { status.ConnectionFailed, }, - status.OrdererServerStatus: []status.Code{ + status.OrdererServerStatus: { status.Code(common.Status_SERVICE_UNAVAILABLE), status.Code(common.Status_INTERNAL_SERVER_ERROR), }, - status.EventServerStatus: []status.Code{ + status.EventServerStatus: { status.Code(pb.TxValidationCode_DUPLICATE_TXID), status.Code(pb.TxValidationCode_ENDORSEMENT_POLICY_FAILURE), status.Code(pb.TxValidationCode_MVCC_READ_CONFLICT), @@ -149,12 +149,12 @@ var ChannelClientRetryableCodes = map[status.Group][]status.Code{ }, // TODO: gRPC introduced retries in v1.8.0. This can be replaced with the // gRPC fail fast option, once available - status.GRPCTransportStatus: []status.Code{ + status.GRPCTransportStatus: { status.Code(grpcCodes.Unavailable), }, } // ChannelConfigRetryableCodes error codes to be taken into account for query channel config retry var ChannelConfigRetryableCodes = map[status.Group][]status.Code{ - status.EndorserClientStatus: []status.Code{status.EndorsementMismatch}, + status.EndorserClientStatus: {status.EndorsementMismatch}, } diff --git a/pkg/core/logging/metadata/callerInfo.go b/pkg/core/logging/metadata/callerInfo.go index 819a78c71e..0a505db172 100644 --- a/pkg/core/logging/metadata/callerInfo.go +++ b/pkg/core/logging/metadata/callerInfo.go @@ -50,10 +50,10 @@ func (l *CallerInfo) IsCallerInfoEnabled(module string, level api.Level) bool { //getDefaultCallerInfoSetting default setting for callerinfo func (l *CallerInfo) getDefaultCallerInfoSetting() map[callerInfoKey]bool { return map[callerInfoKey]bool{ - callerInfoKey{"", api.CRITICAL}: true, - callerInfoKey{"", api.ERROR}: true, - callerInfoKey{"", api.WARNING}: true, - callerInfoKey{"", api.INFO}: true, - callerInfoKey{"", api.DEBUG}: true, + {"", api.CRITICAL}: true, + {"", api.ERROR}: true, + {"", api.WARNING}: true, + {"", api.INFO}: true, + {"", api.DEBUG}: true, } } diff --git a/pkg/fab/events/eventhubclient/connection/connection_test.go b/pkg/fab/events/eventhubclient/connection/connection_test.go index e29062dcde..f6df2b81cc 100755 --- a/pkg/fab/events/eventhubclient/connection/connection_test.go +++ b/pkg/fab/events/eventhubclient/connection/connection_test.go @@ -64,7 +64,7 @@ func TestSend(t *testing.T) { Event: &pb.Event_Register{ Register: &pb.Register{ Events: []*pb.Interest{ - &pb.Interest{EventType: pb.EventType_FILTEREDBLOCK}, + {EventType: pb.EventType_FILTEREDBLOCK}, }, }, }, @@ -100,7 +100,7 @@ func TestSend(t *testing.T) { Event: &pb.Event_Unregister{ Unregister: &pb.Unregister{ Events: []*pb.Interest{ - &pb.Interest{EventType: pb.EventType_FILTEREDBLOCK}, + {EventType: pb.EventType_FILTEREDBLOCK}, }, }, }, @@ -155,7 +155,7 @@ func TestDisconnected(t *testing.T) { Event: &pb.Event_Register{ Register: &pb.Register{ Events: []*pb.Interest{ - &pb.Interest{EventType: pb.EventType_FILTEREDBLOCK}, + {EventType: pb.EventType_FILTEREDBLOCK}, }, }, }, diff --git a/pkg/fab/events/eventhubclient/dispatcher/dispatcher_test.go b/pkg/fab/events/eventhubclient/dispatcher/dispatcher_test.go index 29aabe93ee..506058f014 100755 --- a/pkg/fab/events/eventhubclient/dispatcher/dispatcher_test.go +++ b/pkg/fab/events/eventhubclient/dispatcher/dispatcher_test.go @@ -66,7 +66,7 @@ func TestRegisterInterests(t *testing.T) { // Unregister interests dispatcherEventch <- NewUnregisterInterestsEvent( []*pb.Interest{ - &pb.Interest{ + { EventType: pb.EventType_FILTEREDBLOCK, }, }, @@ -108,7 +108,7 @@ func TestRegisterInterests(t *testing.T) { func registerFilteredBlockEvent(dispatcherEventch chan<- interface{}, errch chan error, t *testing.T) chan<- interface{} { dispatcherEventch <- NewRegisterInterestsEvent( []*pb.Interest{ - &pb.Interest{ + { EventType: pb.EventType_FILTEREDBLOCK, }, }, @@ -127,7 +127,7 @@ func registerFilteredBlockEvent(dispatcherEventch chan<- interface{}, errch chan func checkFailedRegisterInterest(dispatcherEventch chan<- interface{}, errch chan error, t *testing.T) chan<- interface{} { dispatcherEventch <- NewRegisterInterestsEvent( []*pb.Interest{ - &pb.Interest{ + { EventType: pb.EventType_FILTEREDBLOCK, }, }, @@ -164,7 +164,7 @@ func TestRegisterInterestsInvalid(t *testing.T) { // Unregister interests dispatcherEventch <- NewUnregisterInterestsEvent( []*pb.Interest{ - &pb.Interest{ + { EventType: pb.EventType_FILTEREDBLOCK, }, }, @@ -278,7 +278,7 @@ func TestTimedOutRegister(t *testing.T) { // Register interests dispatcherEventch <- NewRegisterInterestsEvent( []*pb.Interest{ - &pb.Interest{ + { EventType: pb.EventType_FILTEREDBLOCK, }, }, diff --git a/pkg/fab/events/eventhubclient/opts.go b/pkg/fab/events/eventhubclient/opts.go index 063358f504..d39367b942 100755 --- a/pkg/fab/events/eventhubclient/opts.go +++ b/pkg/fab/events/eventhubclient/opts.go @@ -13,8 +13,8 @@ import ( pb "github.com/hyperledger/fabric-sdk-go/third_party/github.com/hyperledger/fabric/protos/peer" ) -var blockInterests = []*pb.Interest{&pb.Interest{EventType: pb.EventType_BLOCK}} -var filteredBlockInterests = []*pb.Interest{&pb.Interest{EventType: pb.EventType_FILTEREDBLOCK}} +var blockInterests = []*pb.Interest{{EventType: pb.EventType_BLOCK}} +var filteredBlockInterests = []*pb.Interest{{EventType: pb.EventType_FILTEREDBLOCK}} type params struct { connProvider api.ConnectionProvider diff --git a/pkg/fab/events/service/mocks/mockevents.go b/pkg/fab/events/service/mocks/mockevents.go index 7955f019fb..6f555c9c87 100755 --- a/pkg/fab/events/service/mocks/mockevents.go +++ b/pkg/fab/events/service/mocks/mockevents.go @@ -92,7 +92,7 @@ func NewFilteredTxWithCCEvent(txID, ccID, event string) *pb.FilteredTransaction Data: &pb.FilteredTransaction_TransactionActions{ TransactionActions: &pb.FilteredTransactionActions{ ChaincodeActions: []*pb.FilteredChaincodeAction{ - &pb.FilteredChaincodeAction{ + { ChaincodeEvent: &pb.ChaincodeEvent{ ChaincodeId: ccID, EventName: event, diff --git a/pkg/fab/mocks/mockbroadcastserver.go b/pkg/fab/mocks/mockbroadcastserver.go index dbf6182bb5..9d09478bf2 100644 --- a/pkg/fab/mocks/mockbroadcastserver.go +++ b/pkg/fab/mocks/mockbroadcastserver.go @@ -102,7 +102,7 @@ func StartMockBroadcastServer(broadcastTestURL string, grpcServer *grpc.Server) po.RegisterAtomicBroadcastServer(grpcServer, broadcastServer) go func() { if err := grpcServer.Serve(lis); err != nil { - panic(err.Error()) + fmt.Printf("StartMockBroadcastServer failed to start %v", err.Error()) } }() diff --git a/pkg/fab/mocks/mockendorserserver.go b/pkg/fab/mocks/mockendorserserver.go index 03d36e0cce..88aeae3564 100644 --- a/pkg/fab/mocks/mockendorserserver.go +++ b/pkg/fab/mocks/mockendorserserver.go @@ -49,9 +49,9 @@ func (m *MockEndorserServer) createProposalResponsePayload() []byte { if m.AddkvWrite { txRwSet.NsRwSets = []*rwsetutil.NsRwSet{ - &rwsetutil.NsRwSet{NameSpace: "ns1", KvRwSet: &kvrwset.KVRWSet{ - Reads: []*kvrwset.KVRead{&kvrwset.KVRead{Key: "key1", Version: &kvrwset.Version{BlockNum: 1, TxNum: 1}}}, - Writes: []*kvrwset.KVWrite{&kvrwset.KVWrite{Key: "key2", IsDelete: false, Value: []byte("value2")}}, + {NameSpace: "ns1", KvRwSet: &kvrwset.KVRWSet{ + Reads: []*kvrwset.KVRead{{Key: "key1", Version: &kvrwset.Version{BlockNum: 1, TxNum: 1}}}, + Writes: []*kvrwset.KVWrite{{Key: "key2", IsDelete: false, Value: []byte("value2")}}, }}} } diff --git a/pkg/fab/mocks/mockeventserver.go b/pkg/fab/mocks/mockeventserver.go index bddeb19754..fdf97ddc36 100644 --- a/pkg/fab/mocks/mockeventserver.go +++ b/pkg/fab/mocks/mockeventserver.go @@ -35,7 +35,7 @@ func StartMockEventServer(testAddress string) (*MockEventServer, error) { fmt.Printf("Starting mock event server\n") go func() { if err := grpcServer.Serve(lis); err != nil { - panic(err.Error()) + fmt.Printf("StartMockEventServer failed %v", err.Error()) } }() diff --git a/pkg/fab/txn/proposal_test.go b/pkg/fab/txn/proposal_test.go index 8513e8542a..da509a2e1f 100644 --- a/pkg/fab/txn/proposal_test.go +++ b/pkg/fab/txn/proposal_test.go @@ -72,7 +72,7 @@ func TestSendTransactionProposal(t *testing.T) { request := fab.ChaincodeInvokeRequest{ ChaincodeID: "cc", Fcn: "Hello", - Args: [][]byte{[]byte{1, 2, 3}}, + Args: [][]byte{{1, 2, 3}}, } txh, err := NewHeader(ctx, testChannel) diff --git a/pkg/msp/caclient.go b/pkg/msp/caclient.go index 61940966d2..df828d23d4 100644 --- a/pkg/msp/caclient.go +++ b/pkg/msp/caclient.go @@ -41,9 +41,9 @@ func NewCAClient(orgName string, ctx contextApi.Client) (*CAClientImpl, error) { } if orgName == "" { - clientConfig, err := ctx.IdentityConfig().Client() - if err != nil { - return nil, errors.Wrapf(err, "client config retrieval failed") + clientConfig, err1 := ctx.IdentityConfig().Client() + if err1 != nil { + return nil, errors.Wrapf(err1, "client config retrieval failed") } orgName = clientConfig.Organization } diff --git a/pkg/msp/caclient_test.go b/pkg/msp/caclient_test.go index b395852e6e..79a648b812 100644 --- a/pkg/msp/caclient_test.go +++ b/pkg/msp/caclient_test.go @@ -52,7 +52,7 @@ func TestEnrollAndReenroll(t *testing.T) { // Successful enrollment enrollUsername := createRandomName() - enrolledUserData, err := f.userStore.Load(msp.IdentityIdentifier{MSPID: orgMSPID, ID: enrollUsername}) + _, err = f.userStore.Load(msp.IdentityIdentifier{MSPID: orgMSPID, ID: enrollUsername}) if err != msp.ErrUserNotFound { t.Fatalf("Expected to not find user in user store") } @@ -60,7 +60,7 @@ func TestEnrollAndReenroll(t *testing.T) { if err != nil { t.Fatalf("identityManager Enroll return error %v", err) } - enrolledUserData, err = f.userStore.Load(msp.IdentityIdentifier{MSPID: orgMSPID, ID: enrollUsername}) + enrolledUserData, err := f.userStore.Load(msp.IdentityIdentifier{MSPID: orgMSPID, ID: enrollUsername}) if err != nil { t.Fatalf("Expected to load user from user store") } @@ -75,6 +75,10 @@ func TestEnrollAndReenroll(t *testing.T) { } // Reenroll with appropriate user + reenrollWithAppropriateUser(f, t, enrolledUserData) +} + +func reenrollWithAppropriateUser(f textFixture, t *testing.T, enrolledUserData *msp.UserData) { iManager, ok := f.identityManagerProvider.IdentityManager("org1") if !ok { t.Fatalf("failed to get identity manager") diff --git a/pkg/msp/certfileuserstore.go b/pkg/msp/certfileuserstore.go index 6718668be1..1349b0dc15 100644 --- a/pkg/msp/certfileuserstore.go +++ b/pkg/msp/certfileuserstore.go @@ -21,13 +21,6 @@ type CertFileUserStore struct { store core.KVStore } -func userIdentifierFromUser(user msp.UserData) msp.IdentityIdentifier { - return msp.IdentityIdentifier{ - MSPID: user.MSPID, - ID: user.ID, - } -} - func storeKeyFromUserIdentifier(key msp.IdentityIdentifier) string { return key.ID + "@" + key.MSPID + "-cert.pem" } diff --git a/pkg/msp/certfileuserstore_test.go b/pkg/msp/certfileuserstore_test.go index 86443ead50..7add36134a 100644 --- a/pkg/msp/certfileuserstore_test.go +++ b/pkg/msp/certfileuserstore_test.go @@ -73,44 +73,52 @@ func TestStore(t *testing.T) { EnrollmentCertificate: []byte(testCert2), } - if err := store.Store(user1); err != nil { - t.Fatalf("Store %s failed [%s]", user1.ID, err) - } - if err := store.Store(user2); err != nil { - t.Fatalf("Store %s failed [%s]", user2.ID, err) - } + createStore(store, user1, t, user2) // Check key1, value1 - if err := checkStoreValue(store, user1, user1.EnrollmentCertificate); err != nil { + if err = checkStoreValue(store, user1, user1.EnrollmentCertificate); err != nil { t.Fatalf("checkStoreValue %s failed [%s]", user1.ID, err) } - if err := store.Delete(msp.IdentityIdentifier{MSPID: user1.MSPID, ID: user1.ID}); err != nil { + if err = store.Delete(msp.IdentityIdentifier{MSPID: user1.MSPID, ID: user1.ID}); err != nil { t.Fatalf("Delete %s failed [%s]", user1.ID, err) } - if err := checkStoreValue(store, user2, user2.EnrollmentCertificate); err != nil { + if err = checkStoreValue(store, user2, user2.EnrollmentCertificate); err != nil { t.Fatalf("checkStoreValue %s failed [%s]", user2.ID, err) } - if err := checkStoreValue(store, user1, nil); err != msp.ErrUserNotFound { + if err = checkStoreValue(store, user1, nil); err != msp.ErrUserNotFound { t.Fatalf("checkStoreValue %s failed, expected core.ErrUserNotFound, got: %v", user1.ID, err) } // Check ke2, value2 - if err := checkStoreValue(store, user2, user2.EnrollmentCertificate); err != nil { + if err = checkStoreValue(store, user2, user2.EnrollmentCertificate); err != nil { t.Fatalf("checkStoreValue %s failed [%s]", user2.ID, err) } - if err := store.Delete(msp.IdentityIdentifier{MSPID: user2.MSPID, ID: user2.ID}); err != nil { + if err = store.Delete(msp.IdentityIdentifier{MSPID: user2.MSPID, ID: user2.ID}); err != nil { t.Fatalf("Delete %s failed [%s]", user2.ID, err) } - if err := checkStoreValue(store, user2, nil); err != msp.ErrUserNotFound { + if err = checkStoreValue(store, user2, nil); err != msp.ErrUserNotFound { t.Fatalf("checkStoreValue %s failed, expected core.ErrUserNotFound, got: %v", user2.ID, err) } // Check non-existing key + checkNonExistingKey(store, t) +} + +func createStore(store *CertFileUserStore, user1 *msp.UserData, t *testing.T, user2 *msp.UserData) { + if err := store.Store(user1); err != nil { + t.Fatalf("Store %s failed [%s]", user1.ID, err) + } + if err := store.Store(user2); err != nil { + t.Fatalf("Store %s failed [%s]", user2.ID, err) + } +} + +func checkNonExistingKey(store *CertFileUserStore, t *testing.T) { nonExistingKey := msp.IdentityIdentifier{ MSPID: "Orgx", ID: "userx", } - _, err = store.Load(nonExistingKey) + _, err := store.Load(nonExistingKey) if err == nil || err != msp.ErrUserNotFound { t.Fatal("fetching value for non-existing key should return ErrUserNotFound") } @@ -139,7 +147,7 @@ func checkStoreValue(store *CertFileUserStore, user *msp.UserData, expected []by return err } if expected == nil { - _, err := os.Stat(file) + _, err = os.Stat(file) if err == nil { return fmt.Errorf("path shouldn't exist [%s]", file) } @@ -167,8 +175,12 @@ func compare(v interface{}, expected []byte) error { return errors.New("value is not []byte") } } - if bytes.Compare(vbytes, expected) != 0 { + if !bytes.Equal(vbytes, expected) { return errors.New("value from store comparison failed") } return nil } + +func userIdentifier(userData *msp.UserData) msp.IdentityIdentifier { + return msp.IdentityIdentifier{MSPID: userData.MSPID, ID: userData.ID} +} diff --git a/pkg/msp/enrollment_test.go b/pkg/msp/enrollment_test.go index 81fb3f09a4..bd0af2128a 100644 --- a/pkg/msp/enrollment_test.go +++ b/pkg/msp/enrollment_test.go @@ -15,6 +15,7 @@ import ( "github.com/golang/mock/gomock" "github.com/hyperledger/fabric-sdk-go/internal/github.com/hyperledger/fabric-ca/util" "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/core" + providersFab "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab" "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/msp" "github.com/hyperledger/fabric-sdk-go/pkg/core/config" "github.com/hyperledger/fabric-sdk-go/pkg/core/cryptosuite" @@ -95,40 +96,39 @@ func TestGetSigningIdentityWithEnrollment(t *testing.T) { cleanupTestPath(t, credentialStorePath) defer cleanupTestPath(t, credentialStorePath) + checkSigningIdentityWithEnrollment(cryptoConfig, t, identityConfig, orgName, endpointConfig, clientConfig) +} + +func checkSigningIdentityWithEnrollment(cryptoConfig core.CryptoSuiteConfig, t *testing.T, identityConfig msp.IdentityConfig, orgName string, endpointConfig providersFab.EndpointConfig, clientConfig *msp.ClientConfig) { cs, err := sw.GetSuiteByConfig(cryptoConfig) + if err != nil { + t.Fatalf("Failed to get suite by config: %s", err) + } userStore := userStoreFromConfig(t, identityConfig) - identityMgr, err := NewIdentityManager(orgName, userStore, cs, endpointConfig) if err != nil { t.Fatalf("Failed to setup credential manager: %s", err) } - - if err := checkSigningIdentity(identityMgr, "User1"); err != nil { + if err = checkSigningIdentity(identityMgr, "User1"); err != nil { t.Fatalf("checkSigningIdentity failed: %s", err) } - // Refers to the same location used by the IdentityManager enrollmentTestUserStore, err = NewCertFileUserStore(clientConfig.CredentialStore.Path) if err != nil { t.Fatalf("Failed to setup userStore: %s", err) } - - if err := checkSigningIdentity(identityMgr, userToEnroll); err == nil { + if err = checkSigningIdentity(identityMgr, userToEnroll); err == nil { t.Fatalf("checkSigningIdentity should fail for user who hasn't been enrolled") } - // Enroll the user - ctrl := gomock.NewController(t) defer ctrl.Finish() caClient := apimocks.NewMockCAClient(ctrl) prepareForEnroll(t, caClient, cs) - err = caClient.Enroll(userToEnroll, "enrollmentSecret") if err != nil { t.Fatalf("fabricCAClient Enroll failed: %v", err) } - if err := checkSigningIdentity(identityMgr, userToEnroll); err != nil { t.Fatalf("checkSigningIdentity shouldn't fail for user who hasn been just enrolled: %s", err) } diff --git a/pkg/msp/getsigid.go b/pkg/msp/getsigid.go index aa6e34e128..46505d1e6a 100644 --- a/pkg/msp/getsigid.go +++ b/pkg/msp/getsigid.go @@ -70,7 +70,7 @@ func (mgr *IdentityManager) GetSigningIdentity(id string) (msp.SigningIdentity, } // GetUser returns a user for the given user name -func (mgr *IdentityManager) GetUser(username string) (*User, error) { +func (mgr *IdentityManager) GetUser(username string) (*User, error) { //nolint u, err := mgr.loadUserFromStore(username) if err != nil { @@ -157,7 +157,7 @@ func (mgr *IdentityManager) getEmbeddedPrivateKey(username string) (core.Key, er pemBytes = []byte(keyPem) } else if keyPath != "" { // Try importing from the Embedded Path - _, err := os.Stat(keyPath) + _, err = os.Stat(keyPath) if err != nil { if !os.IsNotExist(err) { return nil, errors.WithMessage(err, "OS stat embedded path failed") diff --git a/pkg/msp/getsigid_test.go b/pkg/msp/getsigid_test.go index d7106a4844..cde0c9ed54 100644 --- a/pkg/msp/getsigid_test.go +++ b/pkg/msp/getsigid_test.go @@ -15,7 +15,10 @@ import ( "fmt" fabricCaUtil "github.com/hyperledger/fabric-sdk-go/internal/github.com/hyperledger/fabric-ca/util" + "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/core" + providersFab "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab" "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/msp" + "github.com/hyperledger/fabric-sdk-go/pkg/core/config" "github.com/hyperledger/fabric-sdk-go/pkg/core/cryptosuite" "github.com/hyperledger/fabric-sdk-go/pkg/core/cryptosuite/bccsp/sw" @@ -51,33 +54,8 @@ XdsmTcdRvJ3TS/6HCA== func TestGetSigningIdentity(t *testing.T) { - configBackend, err := config.FromFile("../../pkg/core/config/testdata/config_test.yaml")() - if err != nil { - t.Fatalf(err.Error()) - } - - cryptoConfig := cryptosuite.ConfigFromBackend(configBackend) - - endpointConfig, err := fab.ConfigFromBackend(configBackend) - if err != nil { - panic(fmt.Sprintf("Failed to read config: %v", err)) - } - - identityConfig, err := ConfigFromBackend(configBackend) - if err != nil { - panic(fmt.Sprintf("Failed to read config: %v", err)) - } - - netConfig, err := endpointConfig.NetworkConfig() - if err != nil { - t.Fatalf("Failed to setup netConfig: %s", err) - } - orgConfig, ok := netConfig.Organizations[strings.ToLower(orgName)] - if !ok { - t.Fatalf("Failed to setup orgConfig: %s", err) - } + cryptoConfig, endpointConfig, identityConfig, orgConfig := getConfigs(t) mspID := orgConfig.MSPID - clientCofig, err := identityConfig.Client() if err != nil { t.Fatalf("Unable to retrieve client config: %v", err) @@ -113,12 +91,42 @@ func TestGetSigningIdentity(t *testing.T) { testUsername := createRandomName() // Should not find the user - if err := checkSigningIdentity(mgr, testUsername); err != msp.ErrUserNotFound { - t.Fatalf("expected ErrUserNotFound, got: %s", err) + if err1 := checkSigningIdentity(mgr, testUsername); err1 != msp.ErrUserNotFound { + t.Fatalf("expected ErrUserNotFound, got: %s", err1) } // "Manually" enroll User1 - _, err = fabricCaUtil.ImportBCCSPKeyFromPEMBytes([]byte(testPrivKey), cryptoSuite, false) + enrollUser1(cryptoSuite, t, mspID, testUsername, userStore, mgr) +} + +func getConfigs(t *testing.T) (core.CryptoSuiteConfig, providersFab.EndpointConfig, msp.IdentityConfig, providersFab.OrganizationConfig) { + configBackend, err := config.FromFile("../../pkg/core/config/testdata/config_test.yaml")() + if err != nil { + t.Fatalf(err.Error()) + } + cryptoConfig := cryptosuite.ConfigFromBackend(configBackend) + endpointConfig, err := fab.ConfigFromBackend(configBackend) + if err != nil { + panic(fmt.Sprintf("Failed to read config: %v", err)) + } + identityConfig, err := ConfigFromBackend(configBackend) + if err != nil { + panic(fmt.Sprintf("Failed to read config: %v", err)) + } + netConfig, err := endpointConfig.NetworkConfig() + if err != nil { + t.Fatalf("Failed to setup netConfig: %s", err) + } + orgConfig, ok := netConfig.Organizations[strings.ToLower(orgName)] + if !ok { + t.Fatalf("Failed to setup orgConfig: %s", err) + } + + return cryptoConfig, endpointConfig, identityConfig, orgConfig +} + +func enrollUser1(cryptoSuite core.CryptoSuite, t *testing.T, mspID string, testUsername string, userStore msp.UserStore, mgr *IdentityManager) { + _, err := fabricCaUtil.ImportBCCSPKeyFromPEMBytes([]byte(testPrivKey), cryptoSuite, false) if err != nil { t.Fatalf("ImportBCCSPKeyFromPEMBytes failed [%s]", err) } @@ -131,7 +139,6 @@ func TestGetSigningIdentity(t *testing.T) { if err != nil { t.Fatalf("userStore.Store: %s", err) } - // Should succeed after enrollment if err := checkSigningIdentity(mgr, testUsername); err != nil { t.Fatalf("checkSigningIdentity failed: %s", err) @@ -210,28 +217,27 @@ func TestGetSigningIdentityFromEmbeddedCryptoConfig(t *testing.T) { t.Fatalf("Failed to setup credential manager: %s", err) } - _, err = mgr.GetSigningIdentity("") + checkSigningIdentityFromEmbeddedCryptoConfig(mgr, t) +} + +func checkSigningIdentityFromEmbeddedCryptoConfig(mgr *IdentityManager, t *testing.T) { + _, err := mgr.GetSigningIdentity("") if err == nil { t.Fatalf("Should get error for empty user name") } - _, err = mgr.GetSigningIdentity("Non-Existent") if err != msp.ErrUserNotFound { t.Fatalf("Should get ErrUserNotFound for non-existent user, got %v", err) } - if err := checkSigningIdentity(mgr, "EmbeddedUser"); err != nil { t.Fatalf("checkSigningIdentity failes: %s", err) } - if err := checkSigningIdentity(mgr, "EmbeddedUserWithPaths"); err != nil { t.Fatalf("checkSigningIdentity failes: %s", err) } - if err := checkSigningIdentity(mgr, "EmbeddedUserMixed"); err != nil { t.Fatalf("checkSigningIdentity failes: %s", err) } - if err := checkSigningIdentity(mgr, "EmbeddedUserMixed2"); err != nil { t.Fatalf("checkSigningIdentity failes: %s", err) } diff --git a/pkg/msp/identityconfig.go b/pkg/msp/identityconfig.go index 0148619d88..866b7fad54 100644 --- a/pkg/msp/identityconfig.go +++ b/pkg/msp/identityconfig.go @@ -15,6 +15,8 @@ import ( "sort" + "regexp" + "github.com/hyperledger/fabric-sdk-go/pkg/common/errors/status" "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/core" "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/fab" @@ -85,9 +87,7 @@ func (c *IdentityConfig) CAServerCertPems(org string) ([]string, error) { } certFilesPem := config.CertificateAuthorities[caName].TLSCACerts.Pem certPems := make([]string, len(certFilesPem)) - for i, v := range certFilesPem { - certPems[i] = string(v) - } + copy(certPems, certFilesPem) return certPems, nil } @@ -251,10 +251,6 @@ func (c *IdentityConfig) networkConfig() (*fab.NetworkConfig, error) { } func (c *IdentityConfig) tryMatchingCAConfig(caName string) (*msp.CAConfig, string, error) { - networkConfig, err := c.networkConfig() - if err != nil { - return nil, "", err - } //Return if no caMatchers are configured caMatchers := c.endpointConfig.CAMatchers() if len(caMatchers) == 0 { @@ -272,40 +268,48 @@ func (c *IdentityConfig) tryMatchingCAConfig(caName string) (*msp.CAConfig, stri for _, k := range keys { v := caMatchers[k] if v.MatchString(caName) { - // get the matching Config from the index number - certAuthorityMatchConfig := networkConfig.EntityMatchers["certificateauthority"][k] - //Get the certAuthorityMatchConfig from mapped host - caConfig, ok := networkConfig.CertificateAuthorities[strings.ToLower(certAuthorityMatchConfig.MappedHost)] - if !ok { - return nil, certAuthorityMatchConfig.MappedHost, errors.New("failed to load config from matched CertAuthority") - } - _, isPortPresentInCAName := c.getPortIfPresent(caName) - //if substitution url is empty, use the same network certAuthority url - if certAuthorityMatchConfig.URLSubstitutionExp == "" { - port, isPortPresent := c.getPortIfPresent(caConfig.URL) - - caConfig.URL = caName - //append port of matched config - if isPortPresent && !isPortPresentInCAName { - caConfig.URL += ":" + strconv.Itoa(port) - } - } else { - //else, replace url with urlSubstitutionExp if it doesnt have any variable declarations like $ - if strings.Index(certAuthorityMatchConfig.URLSubstitutionExp, "$") < 0 { - caConfig.URL = certAuthorityMatchConfig.URLSubstitutionExp - } else { - //if the urlSubstitutionExp has $ variable declarations, use regex replaceallstring to replace networkhostname with substituionexp pattern - caConfig.URL = v.ReplaceAllString(caName, certAuthorityMatchConfig.URLSubstitutionExp) - } - } - - return &caConfig, certAuthorityMatchConfig.MappedHost, nil + return c.findMatchingCert(caName, v, k) } } return nil, "", errors.WithStack(status.New(status.ClientStatus, status.NoMatchingCertificateAuthorityEntity.ToInt32(), "no matching certAuthority config found", nil)) } +func (c *IdentityConfig) findMatchingCert(caName string, v *regexp.Regexp, k int) (*msp.CAConfig, string, error) { + networkConfig, err := c.networkConfig() + if err != nil { + return nil, "", err + } + // get the matching Config from the index number + certAuthorityMatchConfig := networkConfig.EntityMatchers["certificateauthority"][k] + //Get the certAuthorityMatchConfig from mapped host + caConfig, ok := networkConfig.CertificateAuthorities[strings.ToLower(certAuthorityMatchConfig.MappedHost)] + if !ok { + return nil, certAuthorityMatchConfig.MappedHost, errors.New("failed to load config from matched CertAuthority") + } + _, isPortPresentInCAName := c.getPortIfPresent(caName) + //if substitution url is empty, use the same network certAuthority url + if certAuthorityMatchConfig.URLSubstitutionExp == "" { + port, isPortPresent := c.getPortIfPresent(caConfig.URL) + + caConfig.URL = caName + //append port of matched config + if isPortPresent && !isPortPresentInCAName { + caConfig.URL += ":" + strconv.Itoa(port) + } + } else { + //else, replace url with urlSubstitutionExp if it doesnt have any variable declarations like $ + if !strings.Contains(certAuthorityMatchConfig.URLSubstitutionExp, "$") { + caConfig.URL = certAuthorityMatchConfig.URLSubstitutionExp + } else { + //if the urlSubstitutionExp has $ variable declarations, use regex replaceallstring to replace networkhostname with substituionexp pattern + caConfig.URL = v.ReplaceAllString(caName, certAuthorityMatchConfig.URLSubstitutionExp) + } + } + + return &caConfig, certAuthorityMatchConfig.MappedHost, nil +} + func (c *IdentityConfig) getPortIfPresent(url string) (int, bool) { s := strings.Split(url, ":") if len(s) > 1 { diff --git a/pkg/msp/identityconfig_test.go b/pkg/msp/identityconfig_test.go index c718b48a1d..300df89fe1 100644 --- a/pkg/msp/identityconfig_test.go +++ b/pkg/msp/identityconfig_test.go @@ -14,6 +14,7 @@ import ( "os" "strings" + "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/core" "github.com/hyperledger/fabric-sdk-go/pkg/core/config" "github.com/hyperledger/fabric-sdk-go/pkg/core/config/endpoint" "github.com/hyperledger/fabric-sdk-go/pkg/core/mocks" @@ -80,17 +81,25 @@ func TestCAConfigFailsByNetworkConfig(t *testing.T) { } //Testing CA Server Cert Files failure scenario + testCAServerCertFailureScenario(sampleIdentityConfig, t) + + //Testing CAConfig failure scenario + testCAConfigFailureScenario(sampleIdentityConfig, t) + +} + +func testCAServerCertFailureScenario(sampleIdentityConfig *IdentityConfig, t *testing.T) { sCertFiles, err := sampleIdentityConfig.CAServerCertPaths("peerorg1") if len(sCertFiles) > 0 || err == nil { t.Fatal("Getting CA server cert files supposed to fail") } +} - //Testing CAConfig failure scenario +func testCAConfigFailureScenario(sampleIdentityConfig *IdentityConfig, t *testing.T) { caConfig, err := sampleIdentityConfig.CAConfig("peerorg1") if caConfig != nil || err == nil { t.Fatal("Get CA Config supposed to fail") } - } func TestTLSCAConfigFromPems(t *testing.T) { @@ -131,6 +140,9 @@ func TestTLSCAConfigFromPems(t *testing.T) { } _, err = identityConfig.endpointConfig.TLSCACertPool(badCert) + if err != nil { + t.Fatalf("TLSCACertPool failed %v", err) + } keyPem, _ := identityConfig.CAClientKeyPem(org1) @@ -168,7 +180,7 @@ func TestInitConfigFromRawWithPem(t *testing.T) { t.Fatalf("Failed to load orderers from config. Error: %s", err) } - if o == nil || len(o) == 0 { + if len(o) == 0 { t.Fatalf("orderer cannot be nil or empty") } @@ -195,16 +207,38 @@ SQtE5YgdxkUCIHReNWh/pluHTxeGu2jNCH1eh6o2ajSGeeizoapvdJbN if err != nil { t.Fatalf(err.Error()) } - if pc == nil || len(pc) == 0 { + if len(pc) == 0 { t.Fatalf("peers list of %s cannot be nil or empty", org1) } peer0 := "peer0.org1.example.com" - p0, err := idConfig.endpointConfig.PeerConfig(org1, peer0) + checkPeerPem(org1, idConfig, peer0, t) + + // get CA Server cert pems (embedded) for org1 + checkCAServerCertPems("org1", idConfig, t) + + // get the client cert pem (embedded) for org1 + checkClientCertPem(idConfig, "org1", t) + + // get CA Server certs paths for org1 + checkCAServerCertsPath("org1", idConfig, t) + + // get the client cert path for org1 + checkClientCertPath(idConfig, "org1", t) + + // get the client key pem (embedded) for org1 + checkClientKeyPem(idConfig, "org1", t) + + // get the client key file path for org1 + checkClientKeyFilePath(idConfig, "org1", t) +} + +func checkPeerPem(org string, idConfig *IdentityConfig, peer string, t *testing.T) { + p0, err := idConfig.endpointConfig.PeerConfig(org, peer) if err != nil { - t.Fatalf("Failed to load %s of %s from the config. Error: %s", peer0, org1, err) + t.Fatalf("Failed to load %s of %s from the config. Error: %s", peer, org, err) } if p0 == nil { - t.Fatalf("%s of %s cannot be nil", peer0, org1) + t.Fatalf("%s of %s cannot be nil", peer, org) } pPem := `-----BEGIN CERTIFICATE----- MIICSTCCAfCgAwIBAgIRAPQIzfkrCZjcpGwVhMSKd0AwCgYIKoZIzj0EAwIwdjEL @@ -221,50 +255,56 @@ V842OVjxCYYQwCjPIY+5e9ORR+8pxVzcMAoGCCqGSM49BAMCA0cAMEQCIGZ+KTfS eezqv0ml1VeQEmnAEt5sJ2RJA58+LegUYMd6AiAfEe6BKqdY03qFUgEYmtKG+3Dr O94CDp7l2k7hMQI0zQ== -----END CERTIFICATE-----` - - loadedPPem := strings.TrimSpace(p0.TLSCACerts.Pem) // viper's unmarshall adds a \n to the end of a string, hence the TrimeSpace + loadedPPem := strings.TrimSpace(p0.TLSCACerts.Pem) + // viper's unmarshall adds a \n to the end of a string, hence the TrimeSpace if loadedPPem != pPem { - t.Fatalf("%s Pem doesn't match. Expected \n'%s'\n, but got \n'%s'\n", peer0, pPem, loadedPPem) + t.Fatalf("%s Pem doesn't match. Expected \n'%s'\n, but got \n'%s'\n", peer, pPem, loadedPPem) } +} - // get CA Server cert pems (embedded) for org1 - certs, err := idConfig.CAServerCertPems("org1") +func checkCAServerCertPems(org string, idConfig *IdentityConfig, t *testing.T) { + certs, err := idConfig.CAServerCertPems(org) if err != nil { t.Fatalf("Failed to load CAServerCertPems from config. Error: %s", err) } if len(certs) == 0 { t.Fatalf("Got empty PEM certs for CAServerCertPems") } +} - // get the client cert pem (embedded) for org1 - idConfig.CAClientCertPem("org1") +func checkClientCertPem(idConfig *IdentityConfig, org string, t *testing.T) { + _, err := idConfig.CAClientCertPem(org) if err != nil { t.Fatalf("Failed to load CAClientCertPem from config. Error: %s", err) } +} - // get CA Server certs paths for org1 - certs, err = idConfig.CAServerCertPaths("org1") +func checkCAServerCertsPath(org string, idConfig *IdentityConfig, t *testing.T) { + certs, err := idConfig.CAServerCertPaths(org) if err != nil { t.Fatalf("Failed to load CAServerCertPaths from config. Error: %s", err) } if len(certs) == 0 { t.Fatalf("Got empty cert file paths for CAServerCertPaths") } +} - // get the client cert path for org1 - idConfig.CAClientCertPath("org1") +func checkClientCertPath(idConfig *IdentityConfig, org string, t *testing.T) { + _, err := idConfig.CAClientCertPath(org) if err != nil { t.Fatalf("Failed to load CAClientCertPath from config. Error: %s", err) } +} - // get the client key pem (embedded) for org1 - idConfig.CAClientKeyPem("org1") +func checkClientKeyPem(idConfig *IdentityConfig, org string, t *testing.T) { + _, err := idConfig.CAClientKeyPem(org) if err != nil { t.Fatalf("Failed to load CAClientKeyPem from config. Error: %s", err) } +} - // get the client key file path for org1 - idConfig.CAClientKeyPath("org1") +func checkClientKeyFilePath(idConfig *IdentityConfig, org string, t *testing.T) { + _, err := idConfig.CAClientKeyPath(org) if err != nil { t.Fatalf("Failed to load CAClientKeyPath from config. Error: %s", err) } @@ -282,7 +322,7 @@ func loadConfigBytesFromFile(t *testing.T, filePath string) ([]byte, error) { t.Fatalf("Failed to read config file stat. Error: %s", err) } s := fi.Size() - cBytes := make([]byte, s, s) + cBytes := make([]byte, s) n, err := f.Read(cBytes) if err != nil { t.Fatalf("Failed to read test config for bytes array testing. Error: %s", err) @@ -329,34 +369,54 @@ func TestCAConfig(t *testing.T) { } //Testing CA Server Cert Files - sCertFiles, err := identityConfig.CAServerCertPaths(org1) + testCAServerCertFiles(identityConfig, t, org1) + + //Testing MSPID + testMSPID(identityConfig, t, org1) + + //Testing CAConfig + testCAConfig(identityConfig, t, org1) + + // Test CA KeyStore Path + testCAKeyStorePath(backend, t, identityConfig) + + // test Client + testClient(identityConfig, t) - if sCertFiles == nil || len(sCertFiles) == 0 || err != nil { + // testing empty OrgMSP + testEmptyOrgMsp(identityConfig, t) +} + +func testCAServerCertFiles(identityConfig *IdentityConfig, t *testing.T, org string) { + sCertFiles, err := identityConfig.CAServerCertPaths(org) + if len(sCertFiles) == 0 || err != nil { t.Fatal("Getting CA server cert files failed") } +} - //Testing MSPID - mspID, err := identityConfig.endpointConfig.MSPID(org1) +func testMSPID(identityConfig *IdentityConfig, t *testing.T, org string) { + mspID, err := identityConfig.endpointConfig.MSPID(org) if mspID != "Org1MSP" || err != nil { t.Fatal("Get MSP ID failed") } +} - //Testing CAConfig - caConfig, err := identityConfig.CAConfig(org1) +func testCAConfig(identityConfig *IdentityConfig, t *testing.T, org string) { + caConfig, err := identityConfig.CAConfig(org) if caConfig == nil || err != nil { t.Fatal("Get CA Config failed") } +} +func testCAKeyStorePath(backend core.ConfigBackend, t *testing.T, identityConfig *IdentityConfig) { // Test User Store Path - val, ok = backend.Lookup("client.credentialStore.path") + val, ok := backend.Lookup("client.credentialStore.path") if !ok || val == nil { t.Fatal("expected valid value") } if val.(string) != identityConfig.CredentialStorePath() { t.Fatalf("Incorrect User Store path") } - - // Test CA KeyStore Path val, ok = backend.Lookup("client.credentialStore.cryptoStore.path") if !ok || val == nil { t.Fatal("expected valid value") @@ -364,8 +424,9 @@ func TestCAConfig(t *testing.T) { if val.(string) != identityConfig.CAKeyStorePath() { t.Fatalf("Incorrect CA keystore path") } +} - // test Client +func testClient(identityConfig *IdentityConfig, t *testing.T) { c, err := identityConfig.Client() if err != nil { t.Fatalf("Received error when fetching Client info, error is %s", err) @@ -373,9 +434,10 @@ func TestCAConfig(t *testing.T) { if c == nil { t.Fatal("Received empty client when fetching Client info") } +} - // testing empty OrgMSP - mspID, err = identityConfig.endpointConfig.MSPID("dummyorg1") +func testEmptyOrgMsp(identityConfig *IdentityConfig, t *testing.T) { + _, err := identityConfig.endpointConfig.MSPID("dummyorg1") if err == nil { t.Fatal("Get MSP ID did not fail for dummyorg1") } diff --git a/pkg/msp/main_test.go b/pkg/msp/main_test.go index 8358ef5efc..94879421e1 100644 --- a/pkg/msp/main_test.go +++ b/pkg/msp/main_test.go @@ -49,7 +49,7 @@ type textFixture struct { var caServer = &mockmsp.MockFabricCAServer{} -func (f *textFixture) setup(configBackend *mocks.MockConfigBackend) { +func (f *textFixture) setup(configBackend *mocks.MockConfigBackend) { //nolint if configBackend == nil { backend, err := getCustomBackend(configPath) @@ -108,9 +108,9 @@ func (f *textFixture) setup(configBackend *mocks.MockConfigBackend) { panic(fmt.Sprintf("failed to get network config: %v", err)) } for orgName := range netConfig.Organizations { - mgr, err := NewIdentityManager(orgName, f.userStore, f.cryptoSuite, f.endpointConfig) - if err != nil { - panic(fmt.Sprintf("failed to initialize identity manager for organization: %s, cause :%v", orgName, err)) + mgr, err1 := NewIdentityManager(orgName, f.userStore, f.cryptoSuite, f.endpointConfig) + if err1 != nil { + panic(fmt.Sprintf("failed to initialize identity manager for organization: %s, cause :%v", orgName, err1)) } identityManagers[orgName] = mgr } @@ -153,16 +153,6 @@ func readCert(t *testing.T) []byte { return cert } -func readConfigWithReplacement(path string, origURL, newURL string) []byte { - cfgRaw, err := ioutil.ReadFile(path) - if err != nil { - panic(fmt.Sprintf("Failed to read config [%s]", err)) - } - - updatedCfg := strings.Replace(string(cfgRaw), origURL, newURL, -1) - return []byte(updatedCfg) -} - func cleanup(storePath string) { err := os.RemoveAll(storePath) if err != nil { diff --git a/pkg/msp/test/mockmsp/mockfabriccaserver.go b/pkg/msp/test/mockmsp/mockfabriccaserver.go index fe7b53c472..a23081ba0b 100644 --- a/pkg/msp/test/mockmsp/mockfabriccaserver.go +++ b/pkg/msp/test/mockmsp/mockfabriccaserver.go @@ -112,25 +112,28 @@ func (s *MockFabricCAServer) Running() bool { func (s *MockFabricCAServer) addKeyToKeyStore(privateKey []byte) error { // Import private key that matches the cert we will return // from this mock service, so it can be looked up by SKI from the cert - _, err := util.ImportBCCSPKeyFromPEMBytes([]byte(privateKey), s.cryptoSuite, false) - if err != nil { - return err - } - return nil + _, err := util.ImportBCCSPKeyFromPEMBytes(privateKey, s.cryptoSuite, false) + return err } // Register user func (s *MockFabricCAServer) register(w http.ResponseWriter, req *http.Request) { resp := &api.RegistrationResponseNet{RegistrationResponse: api.RegistrationResponse{Secret: "mockSecretValue"}} - cfsslapi.SendResponse(w, resp) + if err := cfsslapi.SendResponse(w, resp); err != nil { + logger.Error(err) + } } // Enroll user func (s *MockFabricCAServer) enroll(w http.ResponseWriter, req *http.Request) { - s.addKeyToKeyStore([]byte(privateKey)) + if err := s.addKeyToKeyStore([]byte(privateKey)); err != nil { + logger.Error(err) + } resp := &enrollmentResponseNet{Cert: util.B64Encode([]byte(ecert))} fillCAInfo(&resp.ServerInfo) - cfapi.SendResponse(w, resp) + if err := cfapi.SendResponse(w, resp); err != nil { + logger.Error(err) + } } // Fill the CA info structure appropriately diff --git a/pkg/msp/test/mockmsp/mockuser.go b/pkg/msp/test/mockmsp/mockuser.go index 94a60ab77b..79b3e3c8b6 100644 --- a/pkg/msp/test/mockmsp/mockuser.go +++ b/pkg/msp/test/mockmsp/mockuser.go @@ -44,7 +44,7 @@ func (m MockSigningIdentity) Serialize() ([]byte, error) { // SetEnrollmentCertificate sets yhe enrollment certificate. func (m MockSigningIdentity) SetEnrollmentCertificate(cert []byte) { - m.enrollmentCertificate = cert + m.enrollmentCertificate = cert //nolint } // EnrollmentCertificate Returns the underlying ECert representing this user’s identity. @@ -64,7 +64,7 @@ func (m MockSigningIdentity) PublicVersion() msp.Identity { // SetPrivateKey sets the private key func (m MockSigningIdentity) SetPrivateKey(key core.Key) { - m.privateKey = key + m.privateKey = key //nolint } // PrivateKey returns the crypto suite representation of the private key diff --git a/pkg/msp/user.go b/pkg/msp/user.go index 75d1c8967a..1cb9ad6c31 100644 --- a/pkg/msp/user.go +++ b/pkg/msp/user.go @@ -23,10 +23,6 @@ type User struct { privateKey core.Key } -func userIdentifier(userData *msp.UserData) msp.IdentityIdentifier { - return msp.IdentityIdentifier{MSPID: userData.MSPID, ID: userData.ID} -} - // Identifier returns user identifier func (u *User) Identifier() *msp.IdentityIdentifier { return &msp.IdentityIdentifier{MSPID: u.mspID, ID: u.id} diff --git a/pkg/msp/user_test.go b/pkg/msp/user_test.go index 65b72f203f..e5eed81edd 100644 --- a/pkg/msp/user_test.go +++ b/pkg/msp/user_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/hyperledger/fabric-sdk-go/internal/github.com/hyperledger/fabric-ca/util" + "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/core" "github.com/hyperledger/fabric-sdk-go/pkg/common/providers/msp" "github.com/hyperledger/fabric-sdk-go/pkg/core/config" "github.com/hyperledger/fabric-sdk-go/pkg/core/cryptosuite" @@ -65,29 +66,31 @@ func TestUserMethods(t *testing.T) { // Import the key into the crypto suite's private key storage. // This is normally done when a new user in enrolled - generatedKey, err := util.ImportBCCSPKeyFromPEMBytes(generatedKeyBytes, cryptoSuite, false) + verifyUserIdentity(cryptoSuite, t, userData, testUserMSPID, testUsername) + +} +func verifyUserIdentity(cryptoSuite core.CryptoSuite, t *testing.T, userData *msp.UserData, testUserMSPID string, testUsername string) { + generatedKey, err := util.ImportBCCSPKeyFromPEMBytes(generatedKeyBytes, cryptoSuite, false) + if err != nil { + t.Fatalf("ImportBCCSPKeyFromPEMBytes failed %v", err) + } user, err := newUser(userData, cryptoSuite) if err != nil { t.Fatalf("newUser failed: %v", err) } - // Check MSPID if user.Identifier().MSPID != testUserMSPID { t.Fatal("user.SetMSPID Failed to MSP.") } - // Check Name if user.Identifier().ID != testUsername { t.Fatalf("NewUser create wrong user") } - // Check EnrolmentCert verifyBytes(t, user.EnrollmentCertificate(), generatedCertBytes) - // Check PrivateKey verifyBytes(t, user.PrivateKey().SKI(), generatedKey.SKI()) - } func verifyBytes(t *testing.T, v interface{}, expected []byte) error { @@ -101,7 +104,7 @@ func verifyBytes(t *testing.T, v interface{}, expected []byte) error { t.Fatalf("value is not []byte") } } - if bytes.Compare(vbytes, expected) != 0 { + if !bytes.Equal(vbytes, expected) { t.Fatalf("value from store comparison failed") } return nil diff --git a/pkg/util/concurrent/futurevalue/futurevalue.go b/pkg/util/concurrent/futurevalue/futurevalue.go index 94c28f3286..bf3e541486 100644 --- a/pkg/util/concurrent/futurevalue/futurevalue.go +++ b/pkg/util/concurrent/futurevalue/futurevalue.go @@ -80,7 +80,7 @@ func (f *Value) MustGet() interface{} { // IsSet returns true if the value has been set, otherwise false is returned func (f *Value) IsSet() bool { - isSet, _, _ := f.get() + isSet, _, _ := f.get() //nolint return isSet } @@ -98,5 +98,5 @@ func (f *Value) set(value interface{}, err error) { value: value, err: err, } - atomic.StorePointer(&f.ref, unsafe.Pointer(holder)) + atomic.StorePointer(&f.ref, unsafe.Pointer(holder)) //nolint } diff --git a/pkg/util/concurrent/futurevalue/futurevalue_test.go b/pkg/util/concurrent/futurevalue/futurevalue_test.go index bfc4e59a63..ebbd246fc0 100644 --- a/pkg/util/concurrent/futurevalue/futurevalue_test.go +++ b/pkg/util/concurrent/futurevalue/futurevalue_test.go @@ -64,10 +64,10 @@ func TestFutureValueGet(t *testing.T) { defer wg.Done() value, err := fv.Get() if err != nil { - t.Fatalf("received error: %s", err) + fail(t, "received error: %s", err) } if value != expectedValue { - t.Fatalf("expecting value [%s] but received [%s]", expectedValue, value) + fail(t, "expecting value [%s] but received [%s]", expectedValue, value) } }() } @@ -97,7 +97,7 @@ func TestFutureValueGetWithError(t *testing.T) { go func() { defer wg.Done() if _, err := fv.Get(); err == nil { - t.Fatalf("expecting error but received none") + fail(t, "expecting error but received none") } }() } @@ -119,12 +119,12 @@ func TestMustGetPanic(t *testing.T) { go func() { defer func() { if r := recover(); r == nil { - t.Fatalf("Expecting panic but got none") + fail(t, "Expecting panic but got none") } done <- true }() fv.MustGet() - t.Fatalf("Expecting panic but got none") + fail(t, "Expecting panic but got none") }() if _, err := fv.Initialize(); err == nil { @@ -132,3 +132,10 @@ func TestMustGetPanic(t *testing.T) { } <-done } + +// fail - as t.Fatalf() is not goroutine safe, this function behaves like t.Fatalf(). +func fail(t *testing.T, template string, args ...interface{}) { + fmt.Printf(template, args...) + fmt.Println() + t.Fail() +} diff --git a/pkg/util/concurrent/lazycache/lazycache.go b/pkg/util/concurrent/lazycache/lazycache.go index f5512cfb35..a2123b38c8 100644 --- a/pkg/util/concurrent/lazycache/lazycache.go +++ b/pkg/util/concurrent/lazycache/lazycache.go @@ -142,8 +142,8 @@ func (c *Cache) close(key string, f future) { logger.Debugf("%s - Reference for [%q] is not set", c.name, key) return } - - if value, _ := f.Get(); value != nil { + value, err := f.Get() + if err == nil && value != nil { if clos, ok := value.(closable); ok && c != nil { logger.Debugf("%s - Invoking Close on value for key [%q].", c.name, key) clos.Close() diff --git a/pkg/util/concurrent/lazycache/lazycache_test.go b/pkg/util/concurrent/lazycache/lazycache_test.go index ff77e95c0b..f46277a077 100644 --- a/pkg/util/concurrent/lazycache/lazycache_test.go +++ b/pkg/util/concurrent/lazycache/lazycache_test.go @@ -69,25 +69,25 @@ func TestGet(t *testing.T) { value, err := cache.Get(NewStringKey("Key1")) if err != nil { - t.Fatalf("Error returned: %s", err) + fail(t, "Error returned: %s", err) } expectedValue := "Value_for_key_Key1" if value != expectedValue { - t.Fatalf("Expecting value [%s] but got [%s]", expectedValue, value) + fail(t, "Expecting value [%s] but got [%s]", expectedValue, value) } value, err = cache.Get(NewStringKey("Key2")) if err != nil { - t.Fatalf("Error returned: %s", err) + fail(t, "Error returned: %s", err) } expectedValue = "Value_for_key_Key2" if value != expectedValue { - t.Fatalf("Expecting value [%s] but got [%s]", expectedValue, value) + fail(t, "Expecting value [%s] but got [%s]", expectedValue, value) } _, err = cache.Get(NewStringKey("error")) if err == nil { - t.Fatalf("Expecting error but got none") + fail(t, "Expecting error but got none") } }() } @@ -188,3 +188,10 @@ func TestClose(t *testing.T) { t.Fatalf("Expecting error since cache is closed") } } + +// fail - as t.Fatalf() is not goroutine safe, this function behaves like t.Fatalf(). +func fail(t *testing.T, template string, args ...interface{}) { + fmt.Printf(template, args...) + fmt.Println() + t.Fail() +} diff --git a/pkg/util/concurrent/lazyref/lazyref.go b/pkg/util/concurrent/lazyref/lazyref.go index 3482825b9b..d85cf8bb89 100644 --- a/pkg/util/concurrent/lazyref/lazyref.go +++ b/pkg/util/concurrent/lazyref/lazyref.go @@ -67,19 +67,19 @@ const ( // is closed (via a call to Close) or if it expires. (Note: The Finalizer function // is not called every time the value is refreshed with the periodic refresh feature.) type Reference struct { - lock sync.RWMutex - ref unsafe.Pointer - lastTimeAccessed unsafe.Pointer + initialInit time.Duration + wg sync.WaitGroup initializer Initializer finalizer Finalizer expirationHandler expirationHandler expirationProvider ExpirationProvider - initialInit time.Duration + ref unsafe.Pointer + lastTimeAccessed unsafe.Pointer expiryType ExpirationType closed bool - closech chan bool running bool - wg sync.WaitGroup + lock sync.RWMutex + closech chan bool } // New creates a new reference @@ -210,12 +210,12 @@ func (r *Reference) isSet() bool { } func (r *Reference) set(value interface{}) { - atomic.StorePointer(&r.ref, unsafe.Pointer(&valueHolder{value: value})) + atomic.StorePointer(&r.ref, unsafe.Pointer(&valueHolder{value: value})) //nolint } func (r *Reference) setLastAccessed() { now := time.Now() - atomic.StorePointer(&r.lastTimeAccessed, unsafe.Pointer(&now)) + atomic.StorePointer(&r.lastTimeAccessed, unsafe.Pointer(&now)) //nolint } func (r *Reference) lastAccessed() time.Time { @@ -223,12 +223,6 @@ func (r *Reference) lastAccessed() time.Time { return *(*time.Time)(p) } -func (r *Reference) timerRunning() bool { - r.lock.RLock() - defer r.lock.RUnlock() - return r.running -} - func (r *Reference) setTimerRunning() bool { r.lock.Lock() defer r.lock.Unlock() @@ -260,54 +254,56 @@ func (r *Reference) ensureTimerStarted(initialExpiration time.Duration) { r.setLastAccessed() - go func() { - if !r.setTimerRunning() { - logger.Debugf("Timer is already running") - return - } - defer r.setTimerStopped() + go checkTimeStarted(r, initialExpiration) +} - logger.Debugf("Starting timer") +func checkTimeStarted(r *Reference, initialExpiration time.Duration) { + if !r.setTimerRunning() { + logger.Debugf("Timer is already running") + return + } + defer r.setTimerStopped() - expiry := initialExpiration - for { - select { - case <-r.closech: - logger.Debugf("Got closed event. Exiting timer.") - return + logger.Debugf("Starting timer") - case <-time.After(expiry): - expiration := r.expirationProvider() + expiry := initialExpiration + for { + select { + case <-r.closech: + logger.Debugf("Got closed event. Exiting timer.") + return - if !r.isSet() && r.expiryType != Refreshing { - expiry = expiration - logger.Debugf("Reference is not set. Will expire again in %s", expiry) - continue - } + case <-time.After(expiry): + expiration := r.expirationProvider() - if r.expiryType == LastInitialized || r.expiryType == Refreshing { - logger.Debugf("Handling expiration...") + if !r.isSet() && r.expiryType != Refreshing { + expiry = expiration + logger.Debugf("Reference is not set. Will expire again in %s", expiry) + continue + } + + if r.expiryType == LastInitialized || r.expiryType == Refreshing { + logger.Debugf("Handling expiration...") + r.handleExpiration() + expiry = expiration + logger.Debugf("... finished handling expiration. Setting expiration to %s", expiry) + } else { + // Check how long it's been since last access + durSinceLastAccess := time.Since(r.lastAccessed()) + logger.Debugf("Duration since last access is %s", durSinceLastAccess) + if durSinceLastAccess > expiration { + logger.Debugf("... handling expiration...") r.handleExpiration() expiry = expiration logger.Debugf("... finished handling expiration. Setting expiration to %s", expiry) } else { - // Check how long it's been since last access - durSinceLastAccess := time.Now().Sub(r.lastAccessed()) - logger.Debugf("Duration since last access is %s", durSinceLastAccess) - if durSinceLastAccess > expiration { - logger.Debugf("... handling expiration...") - r.handleExpiration() - expiry = expiration - logger.Debugf("... finished handling expiration. Setting expiration to %s", expiry) - } else { - // Set another expiry for the remainder of the time - expiry = expiration - durSinceLastAccess - logger.Debugf("Not expired yet. Will check again in %s", expiry) - } + // Set another expiry for the remainder of the time + expiry = expiration - durSinceLastAccess + logger.Debugf("Not expired yet. Will check again in %s", expiry) } } } - }() + } } func (r *Reference) finalize() { diff --git a/test/scripts/check_lint.sh b/test/scripts/check_lint.sh index a4a735ab25..d8b362499b 100755 --- a/test/scripts/check_lint.sh +++ b/test/scripts/check_lint.sh @@ -22,7 +22,6 @@ GOMETALINT_CMD=gometalinter PROJECT_PATH=$GOPATH/src/github.com/hyperledger/fabric-sdk-go declare -a arr=( -"./pkg" "./test" ) @@ -65,12 +64,7 @@ done declare -a arr1=( -"./pkg/client" -"./pkg/common" -"./pkg/context" -"./pkg/core" -"./pkg/fab" -"./pkg/fabsdk" +"./pkg" )