Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(cwg): replace deprecated errors module #12725

Merged
merged 2 commits into from
Jun 15, 2022
Merged

refactor(cwg): replace deprecated errors module #12725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca11795
refactor(cwf): Replaces errors.Wrap[f] with fmt.Errorf.
MoritzThomasHuebner May 16, 2022
b10f7a9
refactor(cwf): Replaces more pkg/errors usages with std lib.
MoritzThomasHuebner May 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cwf/cloud/go/go.mod
View file
Open in desktop
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
5 changes: 2 additions & 3 deletions cwf/cloud/go/services/cwf/obsidian/handlers/handlers.go
View file
Open in desktop
Expand Up @@ -20,7 +20,6 @@ import (
"net/http"

"github.com/labstack/echo"
"github.com/pkg/errors"

"magma/cwf/cloud/go/cwf"
"magma/cwf/cloud/go/serdes"
Expand Down Expand Up @@ -151,7 +150,7 @@ func getGateway(c echo.Context) error {
serdes.Entity,
)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, errors.Wrap(err, "failed to load cwf gateway"))
return echo.NewHTTPError(http.StatusInternalServerError, fmt.Errorf("failed to load cwf gateway: %w", err))
}

ret := &cwfModels.CwfGateway{
Expand Down Expand Up @@ -417,7 +416,7 @@ func updateHAPairHandler(c echo.Context) error {
// 404 if pair doesn't exist
exists, err := configurator.DoesEntityExist(reqCtx, networkID, cwf.CwfHAPairType, haPairID)
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, errors.Wrap(err, "Failed to check if ha pair exists"))
return echo.NewHTTPError(http.StatusInternalServerError, fmt.Errorf("Failed to check if ha pair exists: %w", err))
}
if !exists {
return echo.ErrNotFound
Expand Down
3 changes: 1 addition & 2 deletions cwf/cloud/go/services/cwf/obsidian/models/validate.go
View file
Open in desktop
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that the PR description says this would be substituted with errors.New from std lib. I think this way might be nicer (you don't need the Sprintf). But may be adapt the PR summary?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Also, I stumbled across a few cases like this in #12724, where there was a Sprintf inside an errors.New. When we just change to errors.New from the std lib, the linter will complain and suggest using fmt.Errorf instead, as it is done here.
But yes, we should add this case to the summary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the summary.

}
}
set[peer.IP] = append(set[peer.IP], swag.Uint32Value(peer.Key))
Expand Down
3 changes: 1 addition & 2 deletions cwf/cloud/go/services/cwf/servicers/protected/builder_servicer.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
15 changes: 7 additions & 8 deletions cwf/gateway/integ_tests/test_runner.go
View file
Open in desktop
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 All @@ -175,19 +174,19 @@ func (tr *TestRunner) ConfigUEsPerInstance(IMSIs []string, pcrfInstance, ocsInst

err = uesim.AddUE(ue)
if err != nil {
return nil, errors.Wrap(err, "Error adding UE to UESimServer")
return nil, fmt.Errorf("Error adding UE to UESimServer: %w", err)
}
err = addSubscriberToHSS(sub)
if err != nil {
return nil, errors.Wrap(err, "Error adding Subscriber to HSS")
return nil, fmt.Errorf("Error adding Subscriber to HSS: %w", err)
}
err = addSubscriberToPCRFPerInstance(pcrfInstance, sub.GetSid())
if err != nil {
return nil, errors.Wrap(err, "Error adding Subscriber to PCRF")
return nil, fmt.Errorf("Error adding Subscriber to PCRF: %w", err)
}
err = addSubscriberToOCSPerInstance(ocsInstance, sub.GetSid())
if err != nil {
return nil, errors.Wrap(err, "Error adding Subscriber to OCS")
return nil, fmt.Errorf("Error adding Subscriber to OCS: %w", err)
}

ues = append(ues, ue)
Expand All @@ -211,7 +210,7 @@ func (tr *TestRunner) Authenticate(imsi, calledStationID string) (*radius.Packet
encoded := res.GetRadiusPacket()
radiusP, err := radius.Parse(encoded, []byte(Secret))
if err != nil {
err = errors.Wrap(err, "Error while parsing encoded Radius packet")
err = fmt.Errorf("Error while parsing encoded Radius packet: %w", err)
fmt.Println(err)
return &radius.Packet{}, err
}
Expand All @@ -230,7 +229,7 @@ func (tr *TestRunner) Disconnect(imsi, calledStationID string) (*radius.Packet,
encoded := res.GetRadiusPacket()
radiusP, err := radius.Parse(encoded, []byte(Secret))
if err != nil {
err = errors.Wrap(err, "Error while parsing encoded Radius packet")
err = fmt.Errorf("Error while parsing encoded Radius packet: %w", err)
fmt.Println(err)
return &radius.Packet{}, err
}
Expand Down
4 changes: 2 additions & 2 deletions cwf/gateway/services/uesim/servicers/config.go
View file
Open in desktop
Expand Up @@ -15,13 +15,13 @@ package servicers

import (
"encoding/hex"
"fmt"
"net"

"magma/cwf/gateway/registry"
"magma/orc8r/lib/go/service/config"

"github.com/golang/glog"
"github.com/pkg/errors"
)

const (
Expand All @@ -41,7 +41,7 @@ var (
func GetUESimConfig() (*UESimConfig, error) {
uecfg, err := config.GetServiceConfig("", registry.UeSim)
if err != nil {
glog.Error(errors.Wrap(err, "No service config found, using default config"))
glog.Error(fmt.Errorf("No service config found, using default config: %w", err))
return getDefaultUESimConfig(), nil
}
authAddr, err := uecfg.GetString("radius_auth_address")
Expand Down
8 changes: 4 additions & 4 deletions cwf/gateway/services/uesim/servicers/eap.go
View file
Open in desktop
Expand Up @@ -14,11 +14,11 @@
package servicers

import (
"fmt"

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 @@ -30,7 +30,7 @@ const (
func (srv *UESimServer) HandleEap(ue *cwfprotos.UEConfig, req eap.Packet) (eap.Packet, error) {
err := req.Validate()
if err != nil {
return nil, errors.Wrap(err, "Error validating EAP packet")
return nil, fmt.Errorf("Error validating EAP packet: %w", err)
}

switch fegprotos.EapType(req.Type()) {
Expand All @@ -39,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
27 changes: 13 additions & 14 deletions cwf/gateway/services/uesim/servicers/eap_aka.go
View file
Open in desktop
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,15 +53,15 @@ 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])
}
}

// Given a UE and the EAP-AKA identity request, generates the EAP response.
func (srv *UESimServer) eapAkaIdentityRequest(ue *protos.UEConfig, req eap.Packet) (eap.Packet, error) {
scanner, err := eap.NewAttributeScanner(req)
if err != nil {
return nil, errors.Wrap(err, "Error creating new attribute scanner")
return nil, fmt.Errorf("Error creating new attribute scanner: %w", err)
}

var a eap.Attribute
Expand Down Expand Up @@ -90,14 +89,14 @@ func (srv *UESimServer) eapAkaIdentityRequest(ue *protos.UEConfig, req eap.Packe
),
)
if err != nil {
return nil, errors.Wrap(err, "Error appending attribute to packet")
return nil, fmt.Errorf("Error appending attribute to packet: %w", err)
}
return p, nil
default:
glog.Info(fmt.Sprintf("Unexpected EAP-AKA Identity Request Attribute type %d", a.Type()))
}
}
return nil, errors.Wrap(err, "Error while processing EAP-AKA Identity Request")
return nil, fmt.Errorf("Error while processing EAP-AKA Identity Request: %w", err)
}

type challengeAttributes struct {
Expand All @@ -110,10 +109,10 @@ type challengeAttributes struct {
func (srv *UESimServer) eapAkaChallengeRequest(ue *protos.UEConfig, req eap.Packet) (eap.Packet, error) {
attrs, err := parseChallengeAttributes(req)
if err != io.EOF {
return nil, errors.Wrap(err, "Error while parsing attributes of request packet")
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 All @@ -139,18 +138,18 @@ func (srv *UESimServer) eapAkaChallengeRequest(ue *protos.UEConfig, req eap.Pack
// Calculate RES and other keys.
milenage, err := milenage.NewCipher(srv.cfg.amf)
if err != nil {
return nil, errors.Wrap(err, "Error creating milenage cipher")
return nil, fmt.Errorf("Error creating milenage cipher: %w", err)
}
intermediateVec, err := milenage.GenerateSIPAuthVectorWithRand(rand, key, opc[:], sqn)
if err != nil {
return nil, errors.Wrap(err, "Error calculating authentication vector")
return nil, fmt.Errorf("Error calculating authentication vector: %w", err)
}
// Make copy of packet and zero out MAC value.
copyReq := make([]byte, len(req))
copy(copyReq, req)
copyAttrs, err := parseChallengeAttributes(eap.Packet(copyReq))
if err != io.EOF {
return nil, errors.Wrap(err, "Error while parsing attributes of copied request packet")
return nil, fmt.Errorf("Error while parsing attributes of copied request packet: %w", err)
}
copyMacBytes := copyAttrs.mac.Marshaled()
for i := aka.ATT_HDR_LEN; i < len(copyMacBytes); i++ {
Expand All @@ -168,7 +167,7 @@ func (srv *UESimServer) eapAkaChallengeRequest(ue *protos.UEConfig, req eap.Pack
receivedSqn := extractSqnFromAutn(expectedAutn, intermediateVec.AnonymityKey[:])
resultVec, err := milenage.GenerateSIPAuthVectorWithRand(rand, key, opc[:], receivedSqn)
if err != nil {
return nil, errors.Wrap(err, "Error calculating authentication vector")
return nil, fmt.Errorf("Error calculating authentication vector: %w", err)
}
if !reflect.DeepEqual(expectedAutn[MacAStart:], resultVec.Autn[MacAStart:]) {
return nil, fmt.Errorf("Invalid MacA in AUTN: Received MacA %x; Calculated MacA: %x",
Expand Down Expand Up @@ -205,7 +204,7 @@ func (srv *UESimServer) eapAkaChallengeRequest(ue *protos.UEConfig, req eap.Pack
),
)
if err != nil {
return nil, errors.Wrap(err, "Error appending attribute to packet")
return nil, fmt.Errorf("Error appending attribute to packet: %w", err)
}

// Add the CHECKCODE attribute.
Expand All @@ -226,7 +225,7 @@ func (srv *UESimServer) eapAkaChallengeRequest(ue *protos.UEConfig, req eap.Pack
),
)
if err != nil {
return nil, errors.Wrap(err, "Error appending attribute to packet")
return nil, fmt.Errorf("Error appending attribute to packet: %w", err)
}

// Calculate and Copy MAC into packet.
Expand All @@ -242,7 +241,7 @@ func parseChallengeAttributes(req eap.Packet) (challengeAttributes, error) {

scanner, err := eap.NewAttributeScanner(req)
if err != nil {
return attrs, errors.Wrap(err, "Error creating new attribute scanner")
return attrs, fmt.Errorf("Error creating new attribute scanner: %w", err)
}
var a eap.Attribute
for a, err = scanner.Next(); err == nil; a, err = scanner.Next() {
Expand Down
8 changes: 3 additions & 5 deletions cwf/gateway/services/uesim/servicers/radius.go
View file
Open in desktop
Expand Up @@ -25,8 +25,6 @@ import (
"fbc/lib/go/radius/rfc2866"
"fbc/lib/go/radius/rfc2869"
"magma/feg/gateway/services/eap"

"github.com/pkg/errors"
)

// todo Replace constants with configurable fields
Expand All @@ -41,12 +39,12 @@ func (srv *UESimServer) HandleRadius(imsi string, calledStationID string, p *rad
// Extract EAP packet.
eapMessage, err := packet.NewPacketFromRadius(p)
if err != nil {
err = errors.Wrap(err, "Error extracting EAP message from Radius packet")
err = fmt.Errorf("Error extracting EAP message from Radius packet: %w", err)
return nil, err
}
eapBytes, err := eapMessage.Bytes()
if err != nil {
err = errors.Wrap(err, "Error converting EAP packet to bytes")
err = fmt.Errorf("Error converting EAP packet to bytes: %w", err)
return nil, err
}

Expand Down Expand Up @@ -97,7 +95,7 @@ func (srv *UESimServer) EapToRadius(eapP eap.Packet, imsi string, calledStationI

encoded, err := radiusP.Encode()
if err != nil {
return nil, errors.Wrap(err, "Error encoding Radius packet")
return nil, fmt.Errorf("Error encoding Radius packet: %w", err)
}

// Add Message-Authenticator Attribute.
Expand Down