Skip to content

Commit

Permalink
refactor(cwf): Replaces more pkg/errors usages with std lib.
Browse files Browse the repository at this point in the history
Also fixes an incorrect use of commit/rollback error handling in ue_store_utils.go and uesim.go.

Signed-off-by: Moritz Huebner <moritz.huebner@tngtech.com>
  • Loading branch information
MoritzThomasHuebner committed May 30, 2022
1 parent 3d0c1d7 commit 1ff0966
Show file tree
Hide file tree
Showing 10 changed files with 24 additions and 27 deletions.
2 changes: 1 addition & 1 deletion cwf/cloud/go/go.mod
Expand Up @@ -29,7 +29,6 @@ require (
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
github.com/golang/protobuf v1.5.2
github.com/labstack/echo v3.3.10+incompatible
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.5.1
github.com/prometheus/common v0.9.1
github.com/stretchr/testify v1.7.1
Expand Down Expand Up @@ -82,6 +81,7 @@ require (
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/oklog/ulid v1.3.1 // indirect
github.com/olivere/elastic/v7 v7.0.6 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/procfs v0.0.8 // indirect
Expand Down
3 changes: 1 addition & 2 deletions cwf/cloud/go/services/cwf/obsidian/models/validate.go
Expand Up @@ -20,7 +20,6 @@ import (

"github.com/go-openapi/strfmt"
"github.com/go-openapi/swag"
"github.com/pkg/errors"
)

func (m *CwfNetwork) ValidateModel(context.Context) error {
Expand All @@ -45,7 +44,7 @@ func (m *GatewayCwfConfigs) ValidateModel(context.Context) error {
for _, peer := range m.AllowedGrePeers {
for _, key := range set[peer.IP] {
if swag.Uint32Value(peer.Key) == key {
return errors.New(fmt.Sprintf("Found duplicate peer %s with key %d", peer.IP, key))
return fmt.Errorf("Found duplicate peer %s with key %d", peer.IP, key)
}
}
set[peer.IP] = append(set[peer.IP], swag.Uint32Value(peer.Key))
Expand Down
Expand Up @@ -20,7 +20,6 @@ import (

"github.com/go-openapi/swag"
"github.com/golang/protobuf/proto"
"github.com/pkg/errors"

"magma/cwf/cloud/go/cwf"
cwf_mconfig "magma/cwf/cloud/go/protos/mconfig"
Expand Down Expand Up @@ -90,7 +89,7 @@ func (s *builderServicer) Build(ctx context.Context, request *builder_protos.Bui
}
vals, err := buildFromConfigs(nwConfig, gwConfig.Config.(*models.GatewayCwfConfigs), haPairConfigs)
if err != nil {
return nil, errors.WithStack(err)
return nil, err
}
ret.ConfigsByKey, err = mconfig.MarshalConfigs(vals)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions cwf/gateway/go.mod
Expand Up @@ -36,8 +36,8 @@ require (
github.com/go-redis/redis v6.15.5+incompatible
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
github.com/golang/protobuf v1.5.2
github.com/hashicorp/go-multierror v1.1.1
github.com/magma/milenage v1.0.2
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.11.0
github.com/shirou/gopsutil/v3 v3.21.5
github.com/sparrc/go-ping v0.0.0-20190613174326-4e5b6552494c
Expand Down Expand Up @@ -86,7 +86,6 @@ require (
github.com/google/uuid v1.1.2 // indirect
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
github.com/hashicorp/errwrap v1.0.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
github.com/ishidawataru/sctp v0.0.0-20191218070446-00ab2ac2db07 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
Expand All @@ -109,6 +108,7 @@ require (
github.com/oklog/ulid v1.3.1 // indirect
github.com/olivere/elastic/v7 v7.0.6 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/common v0.26.0 // indirect
Expand Down
2 changes: 1 addition & 1 deletion cwf/gateway/integ_tests/service_wrappers.go
Expand Up @@ -15,6 +15,7 @@ package integration

import (
"context"
"errors"
"fmt"

"magma/cwf/gateway/registry"
Expand All @@ -29,7 +30,6 @@ import (

"github.com/go-redis/redis"
"github.com/golang/glog"
"github.com/pkg/errors"
"google.golang.org/grpc"
)

Expand Down
3 changes: 1 addition & 2 deletions cwf/gateway/integ_tests/test_runner.go
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/golang/protobuf/ptypes/wrappers"
"github.com/magma/milenage"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"

"fbc/lib/go/radius"
Expand Down Expand Up @@ -162,7 +161,7 @@ func (tr *TestRunner) ConfigUEsPerInstance(IMSIs []string, pcrfInstance, ocsInst
for _, imsi := range IMSIs {
// If IMSIs were generated properly they should never give an error here
if _, present := tr.imsis[imsi]; present {
return nil, errors.Errorf("IMSI %s already exist in database, use generateRandomIMSIS(num, tr.imsis) to create unique list", imsi)
return nil, fmt.Errorf("IMSI %s already exist in database, use generateRandomIMSIS(num, tr.imsis) to create unique list", imsi)
}
key, opc, err := getRandKeyOpcFromOp([]byte(Op))
if err != nil {
Expand Down
4 changes: 1 addition & 3 deletions cwf/gateway/services/uesim/servicers/eap.go
Expand Up @@ -19,8 +19,6 @@ import (
cwfprotos "magma/cwf/cloud/go/protos"
fegprotos "magma/feg/gateway/services/aaa/protos"
"magma/feg/gateway/services/eap"

"github.com/pkg/errors"
)

// todo Replace constants with configurable fields
Expand All @@ -41,7 +39,7 @@ func (srv *UESimServer) HandleEap(ue *cwfprotos.UEConfig, req eap.Packet) (eap.P
case fegprotos.EapType_AKA:
return srv.handleEapAka(ue, req)
}
return nil, errors.Errorf("Unsupported Eap Type: %d", req[eap.EapMsgMethodType])
return nil, fmt.Errorf("Unsupported Eap Type: %d", req[eap.EapMsgMethodType])
}

func (srv *UESimServer) eapIdentityRequest(ue *cwfprotos.UEConfig, req eap.Packet) (res eap.Packet, err error) {
Expand Down
5 changes: 2 additions & 3 deletions cwf/gateway/services/uesim/servicers/eap_aka.go
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/golang/glog"
"github.com/magma/milenage"
"github.com/pkg/errors"

"magma/cwf/cloud/go/protos"
"magma/feg/gateway/services/eap"
Expand Down Expand Up @@ -54,7 +53,7 @@ func (srv *UESimServer) handleEapAka(ue *protos.UEConfig, req eap.Packet) (eap.P
case aka.SubtypeChallenge:
return srv.eapAkaChallengeRequest(ue, req)
default:
return nil, errors.Errorf("Unsupported Subtype: %d", req[eap.EapSubtype])
return nil, fmt.Errorf("Unsupported Subtype: %d", req[eap.EapSubtype])
}
}

Expand Down Expand Up @@ -113,7 +112,7 @@ func (srv *UESimServer) eapAkaChallengeRequest(ue *protos.UEConfig, req eap.Pack
return nil, fmt.Errorf("Error while parsing attributes of request packet: %w", err)
}
if attrs.rand == nil || attrs.autn == nil || attrs.mac == nil {
return nil, errors.Errorf("Missing one or more expected attributes\nRAND: %s\nAUTN: %s\nMAC: %s\n", attrs.rand, attrs.autn, attrs.mac)
return nil, fmt.Errorf("Missing one or more expected attributes\nRAND: %s\nAUTN: %s\nMAC: %s\n", attrs.rand, attrs.autn, attrs.mac)
}

// Parse out RAND, expected AUTN, and expected MAC values.
Expand Down
13 changes: 7 additions & 6 deletions cwf/gateway/services/uesim/servicers/ue_store_utils.go
Expand Up @@ -19,6 +19,8 @@ import (
cwfprotos "magma/cwf/cloud/go/protos"
"magma/orc8r/cloud/go/blobstore"
"magma/orc8r/lib/go/protos"

"github.com/hashicorp/go-multierror"
)

func addUeToStore(srvstore blobstore.StoreFactory, ue *cwfprotos.UEConfig) {
Expand All @@ -30,16 +32,15 @@ func addUeToStore(srvstore blobstore.StoreFactory, ue *cwfprotos.UEConfig) {
return
}
defer func() {
switch err {
case nil:
if err == nil {
if commitErr := store.Commit(); commitErr != nil {
err = fmt.Errorf("Error while committing transaction: %w", err)
err = fmt.Errorf("Error while committing transaction: %w", commitErr)
err = ConvertStorageErrorToGrpcStatus(err)
}
default:
} else {
if rollbackErr := store.Rollback(); rollbackErr != nil {
err = fmt.Errorf("Error while rolling back transaction: %w", err)
err = ConvertStorageErrorToGrpcStatus(err)
errs := multierror.Append(err, fmt.Errorf("Error while rolling back transaction: %w", rollbackErr))
err = ConvertStorageErrorToGrpcStatus(errs.ErrorOrNil())
}
}
}()
Expand Down
12 changes: 7 additions & 5 deletions cwf/gateway/services/uesim/servicers/uesim.go
Expand Up @@ -31,6 +31,7 @@ import (
"magma/orc8r/lib/go/protos"

"github.com/golang/glog"
"github.com/hashicorp/go-multierror"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
Expand Down Expand Up @@ -266,14 +267,15 @@ func getUE(blobStoreFactory blobstore.StoreFactory, imsi string) (ue *cwfprotos.
return
}
defer func() {
switch err {
case nil:
if err == nil {
if commitErr := store.Commit(); commitErr != nil {
err = fmt.Errorf("Error while committing transaction: %w", err)
err = fmt.Errorf("Error while committing transaction: %w", commitErr)
}
default:
} else {
if rollbackErr := store.Rollback(); rollbackErr != nil {
glog.Errorf("Error while rolling back transaction: %s", err)
glog.Errorf("Error while rolling back transaction: %s", rollbackErr)
errs := multierror.Append(err, fmt.Errorf("Error while rolling back transaction: %w", rollbackErr))
err = errs.ErrorOrNil()
}
}
}()
Expand Down

0 comments on commit 1ff0966

Please sign in to comment.