Skip to content

Commit

Permalink
Fix wrapping errors (#2492)
Browse files Browse the repository at this point in the history
* Remove `github.com/pkg/errors` as it's abandoned

Signed-off-by: Mustafa Abdelrahman <[email protected]>

* Run `go mod tidy`

Signed-off-by: Mustafa Abdelrahman <[email protected]>

* Add test case for error wrapping

Signed-off-by: Mustafa Abdelrahman <[email protected]>

* Use yaml for test data

Signed-off-by: Mustafa Abdelrahman <[email protected]>

* Update github.com/sarslanhan/cronmask

Signed-off-by: Alexander Yastrebov <[email protected]>

* Revert to JSON testdata and remove yaml unmarshalling cause it's buggy

Signed-off-by: Mustafa Abdelrahman <[email protected]>

---------

Signed-off-by: Mustafa Abdelrahman <[email protected]>
Signed-off-by: Alexander Yastrebov <[email protected]>
Co-authored-by: Alexander Yastrebov <[email protected]>
  • Loading branch information
MustafaSaber and AlexanderYastrebov authored Aug 2, 2023
1 parent 5c90e60 commit 4d664c9
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 49 deletions.
15 changes: 15 additions & 0 deletions dataclients/kubernetes/definitions/definitions.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package definitions

import (
"strings"
"time"

"errors"
Expand Down Expand Up @@ -43,3 +44,17 @@ type WeightedBackend interface {
GetName() string
GetWeight() float64
}

// TODO: use https://pkg.go.dev/errors#Join with go1.21
func errorsJoin(errs ...error) error {
var errVals []string
for _, err := range errs {
if err != nil {
errVals = append(errVals, err.Error())
}
}
if len(errVals) > 0 {
return errors.New(strings.Join(errVals, "\n"))
}
return nil
}
16 changes: 16 additions & 0 deletions dataclients/kubernetes/definitions/definitions_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
package definitions_test

import (
"os"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/zalando/skipper/dataclients/kubernetes/definitions"
"github.com/zalando/skipper/dataclients/kubernetes/kubernetestest"
)

func TestRouteGroupValidation(t *testing.T) {
kubernetestest.FixturesToTest(t, "testdata/validation")
}

func TestValidateRouteGroups(t *testing.T) {
data, err := os.ReadFile("testdata/errorwrapdata/routegroups.json")
require.NoError(t, err)

rgl, err := definitions.ParseRouteGroupsJSON(data)
require.NoError(t, err)

err = definitions.ValidateRouteGroups(&rgl)

assert.EqualError(t, err, "route group without name\nerror in route group default/rg1: route group without backend")
}
24 changes: 4 additions & 20 deletions dataclients/kubernetes/definitions/ingressv1.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ package definitions

import (
"encoding/json"
"errors"
"fmt"
"strconv"

"github.com/pkg/errors"
"gopkg.in/yaml.v2"
)

type IngressV1List struct {
Expand Down Expand Up @@ -78,38 +76,24 @@ func ParseIngressV1JSON(d []byte) (IngressV1List, error) {
return il, err
}

// ParseIngressV1YAML parse YAML into an IngressV1List
func ParseIngressV1YAML(d []byte) (IngressV1List, error) {
var il IngressV1List
err := yaml.Unmarshal(d, &il)
return il, err
}

// TODO: implement once IngressItem has a validate method
// ValidateIngressV1 is a no-op
func ValidateIngressV1(_ *IngressV1Item) error {
return nil
}

// ValidateIngresses is a no-op
func ValidateIngressesV1(ingressList IngressV1List) error {
var err error
var errs []error
// discover all errors to avoid the user having to repeatedly validate
for _, i := range ingressList.Items {
nerr := ValidateIngressV1(i)
if nerr != nil {
name := i.Metadata.Name
namespace := i.Metadata.Namespace
nerr = fmt.Errorf("%s/%s: %w", name, namespace, nerr)
err = errors.Wrap(err, nerr.Error())
errs = append(errs, fmt.Errorf("%s/%s: %w", name, namespace, nerr))
}
}

if err != nil {
return err
}

return nil
return errorsJoin(errs...)
}

func GetHostsFromIngressRulesV1(ing *IngressV1Item) []string {
Expand Down
25 changes: 5 additions & 20 deletions dataclients/kubernetes/definitions/routegroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import (
"encoding/json"
"fmt"

"github.com/pkg/errors"
"errors"

"github.com/zalando/skipper/eskip"
"github.com/zalando/skipper/loadbalancer"
"gopkg.in/yaml.v2"
)

// adding Kubernetes specific backend types here. To be discussed.
Expand Down Expand Up @@ -446,32 +446,17 @@ func ParseRouteGroupsJSON(d []byte) (RouteGroupList, error) {
return rl, err
}

// ParseRouteGroupsYAML parses a YAML list of RouteGroups into RouteGroupList
func ParseRouteGroupsYAML(d []byte) (RouteGroupList, error) {
var rl RouteGroupList
err := yaml.Unmarshal(d, &rl)
return rl, err
}

// ValidateRouteGroup validates a RouteGroupItem
func ValidateRouteGroup(rg *RouteGroupItem) error {
return rg.validate()
}

// ValidateRouteGroups validates a RouteGroupList
func ValidateRouteGroups(rl *RouteGroupList) error {
var err error
var errs []error
// avoid the user having to repeatedly validate to discover all errors
for _, i := range rl.Items {
nerr := ValidateRouteGroup(i)
if nerr != nil {
err = errors.Wrap(err, nerr.Error())
}
errs = append(errs, ValidateRouteGroup(i))
}

if err != nil {
return err
}

return nil
return errorsJoin(errs...)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
{
"items": [
{
"apiVersion": "zalando.org/v1",
"kind": "RouteGroup",
"spec": {
"hosts": [
"test.example.com"
],
"routes": [
{
"filters": [
"inlineContent(\"/foo\")"
],
"path": "/foo"
}
]
}
},
{
"apiVersion": "zalando.org/v1",
"metadata": {
"name": "rg1"
},
"kind": "RouteGroup",
"spec": {
"hosts": [
"test.example.com"
],
"routes": [
{
"filters": [
"inlineContent(\"/foo\")"
],
"path": "/foo"
}
]
}
},
{
"apiVersion": "zalando.org/v1",
"metadata": {
"name": "rg1"
},
"kind": "RouteGroup",
"spec": {
"backends": [
{
"name": "shunt",
"type": "shunt"
}
],
"hosts": [
"test.example.com"
],
"routes": [
{
"backends": [
{
"backendName": "shunt"
}
],
"filters": [
"inlineContent(\"/foo\")"
],
"path": "/foo"
}
]
}
}
]
}
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ require (
github.com/oklog/ulid v1.3.1
github.com/opentracing/basictracer-go v1.1.0
github.com/opentracing/opentracing-go v1.2.0
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.16.0
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475
github.com/redis/go-redis/v9 v9.0.5
github.com/sarslanhan/cronmask v0.0.0-20190709075623-766eca24d011
github.com/sarslanhan/cronmask v0.0.0-20230801193303-54e29300a091
github.com/sirupsen/logrus v1.9.3
github.com/sony/gobreaker v0.5.0
github.com/stretchr/testify v1.8.4
Expand Down Expand Up @@ -99,6 +98,7 @@ require (
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/opencontainers/image-spec v1.1.0-rc2 // indirect
github.com/opencontainers/runc v1.1.5 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/pquerna/cachecontrol v0.0.0-20171018203845-0dec1b30a021 // indirect
github.com/prometheus/client_model v0.3.0 // indirect
Expand Down
9 changes: 2 additions & 7 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,6 @@ github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/influxdata/tdigest v0.0.1 h1:XpFptwYmnEKUqmkcDjrzffswZ3nvNeevbUSLPP/ZzIY=
github.com/influxdata/tdigest v0.0.1/go.mod h1:Z0kXnxzbTC2qrx4NaIzYkE1k66+6oEDQTvL95hQFh5Y=
github.com/instana/go-sensor v1.55.0 h1:9Dpo0S9hah1irJAkkpGMfiAoKMbLLOtYBbtdhJbGvUM=
github.com/instana/go-sensor v1.55.0/go.mod h1:19yQd89yv2d0O2+onnGL5WvtMS5c/HVzl14ko8eZgqo=
github.com/instana/go-sensor v1.55.2 h1:XdLN38mSFsHpaL+jIDkE/ZrW7pxgPeUC/bV9bSwVHyM=
github.com/instana/go-sensor v1.55.2/go.mod h1:Ks06EG9Da5O3hbdJiHIePG/vNmToovkaJjMlUBd70Yc=
github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
Expand Down Expand Up @@ -229,7 +227,6 @@ github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+
github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c h1:Lgl0gzECD8GnQ5QCWA8o6BtfL6mDH5rQgM4/fX3avOs=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
Expand All @@ -250,8 +247,8 @@ github.com/redis/go-redis/v9 v9.0.5 h1:CuQcn5HIEeK7BgElubPP8CGtE0KakrnbBSTLjathl
github.com/redis/go-redis/v9 v9.0.5/go.mod h1:WqMKv5vnQbRuZstUwxQI195wHy+t4PuXDOjzMvcuQHk=
github.com/rogpeppe/go-internal v1.8.1 h1:geMPLpDpQOgVyCg5z5GoRwLHepNdb71NXb67XFkP+Eg=
github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/sarslanhan/cronmask v0.0.0-20190709075623-766eca24d011 h1:S5j3KTsiGwmQSEJJBp0iIG87CDBCGCwbYLmVv8L/nuE=
github.com/sarslanhan/cronmask v0.0.0-20190709075623-766eca24d011/go.mod h1:NmI1tg7wwsf1hF6G5EtyGCrtNKsH2RIdYYoJa7GsnP8=
github.com/sarslanhan/cronmask v0.0.0-20230801193303-54e29300a091 h1:L644WnBAUw4546Wrt52yzuSPoV24t0ArlMwc5iRr8U0=
github.com/sarslanhan/cronmask v0.0.0-20230801193303-54e29300a091/go.mod h1:qZKxttzn8iyVLtc7edFrmQper3FUBJsc/rHCONN2wIQ=
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529 h1:nn5Wsu0esKSJiIVhscUtVbo7ada43DJhG55ua/hjS5I=
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
github.com/seccomp/libseccomp-golang v0.9.2-0.20220502022130-f33da4d89646/go.mod h1:JA8cRccbGaA1s33RQf7Y1+q9gHmZX1yB/z9WDN1C6fg=
Expand Down Expand Up @@ -287,8 +284,6 @@ github.com/szuecs/rate-limit-buffer v0.9.0 h1:65fBCVsaJFh0E1G5C6/sInEPlYR6dXtF9J
github.com/szuecs/rate-limit-buffer v0.9.0/go.mod h1:BxqrsmnHsCnWcvbtdcaDLEBmjNEvRFU5LQ8edoZ9B0M=
github.com/testcontainers/testcontainers-go v0.20.1 h1:mK15UPJ8c5P+NsQKmkqzs/jMdJt6JMs5vlw2y4j92c0=
github.com/testcontainers/testcontainers-go v0.20.1/go.mod h1:zb+NOlCQBkZ7RQp4QI+YMIHyO2CQ/qsXzNF5eLJ24SY=
github.com/tidwall/gjson v1.14.4 h1:uo0p8EbA09J7RQaflQ1aBRffTR7xedD2bcIVSYxLnkM=
github.com/tidwall/gjson v1.14.4/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/gjson v1.15.0 h1:5n/pM+v3r5ujuNl4YLZLsQ+UE5jlkLVm7jMzT5Mpolw=
github.com/tidwall/gjson v1.15.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
Expand Down

0 comments on commit 4d664c9

Please sign in to comment.