Skip to content

Commit

Permalink
fix(kuma-cp) supported versions fix (#2193)
Browse files Browse the repository at this point in the history
As we were using caret ranges `^` instead of tilde ranges `~`
to catch the latest versions (1.1.0, 1.2.0) both of them could
catch the 1.2.0 version. Versions are delivered to our gui
as a hashmap, and when gui on the 1.2.0 version was asking our api
for the supported version of envoy it could receive the response
from range `^1.2.0` and then, everything was fine, and sometimes
it could receive a response from the `^1.1.0` range, and then
it was displaying warning.

It was happening because in the gui, we are iterating over
the hashmap with versions, and there is no consistency in
the order of keys (which is normal).

Also added tests to make sure there is always only one constraint
per version, to not allow similar situation to happen again.

Signed-off-by: Bart Smykla <bartek@smykla.com>
  • Loading branch information
bartsmykla authored Jun 22, 2021
1 parent ec83eee commit e2bd183
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 55 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.16

require (
cirello.io/pglock v1.8.0
github.com/Masterminds/semver/v3 v3.1.1
github.com/Masterminds/sprig v2.22.0+incompatible
github.com/Nordix/simple-ipam v1.0.0
github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d
Expand Down
3 changes: 2 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy86
github.com/Masterminds/semver v1.4.2/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y=
github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww=
github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y=
github.com/Masterminds/semver/v3 v3.1.0 h1:Y2lUDsFKVRSYGojLJ1yLxSXdMmMYTYls0rCvoqmMUQk=
github.com/Masterminds/semver/v3 v3.1.0/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs=
github.com/Masterminds/semver/v3 v3.1.1 h1:hLg3sBzpNErnxhQtUy/mmLR2I9foDujNK030IGemrRc=
github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs=
github.com/Masterminds/sprig v2.20.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuNhlNS5hqE0NB0E6fgfo2Br3o=
github.com/Masterminds/sprig v2.22.0+incompatible h1:z4yfnGrZ7netVz+0EDJ0Wi+5VZCSYp4Z0m2dk6cEM60=
github.com/Masterminds/sprig v2.22.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuNhlNS5hqE0NB0E6fgfo2Br3o=
Expand Down
4 changes: 2 additions & 2 deletions pkg/api-server/versions_ws.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ var Versions = []byte(`{
"1.0.8": {
"envoy": "1.16.2"
},
"^1.1.0": {
"~1.1.0": {
"envoy": "~1.17.0"
},
"^1.2.0": {
"~1.2.0": {
"envoy": "~1.18.0"
}
}
Expand Down
170 changes: 118 additions & 52 deletions pkg/api-server/versions_ws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,90 @@ import (
"encoding/json"
"fmt"
"net/http"
"strings"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/pkg/errors"

"github.com/Masterminds/semver/v3"

config "github.com/kumahq/kuma/pkg/config/api-server"
"github.com/kumahq/kuma/pkg/metrics"
"github.com/kumahq/kuma/pkg/plugins/resources/memory"
)

func buildConstraints(versions map[string]envoyVersion) ([]*semver.Constraints, error) {
var errs []string
var constraints []*semver.Constraints

for c := range versions {
constraint, err := semver.NewConstraint(c)
if err != nil {
errs = append(errs, err.Error())
continue
}

constraints = append(constraints, constraint)
}

if len(errs) > 0 {
return nil, errors.Errorf("couldn't build constraints:\n%s", strings.Join(errs, "\n"))
}

return constraints, nil
}

func getConstraint(constraints []*semver.Constraints, version string) (*semver.Constraints, error) {
v, err := semver.NewVersion(version)
if err != nil {
return nil, err
}

var matchedConstrain []*semver.Constraints

for _, constraint := range constraints {
if constraint.Check(v) {
matchedConstrain = append(matchedConstrain, constraint)
}
}

if len(matchedConstrain) == 0 {
return nil, errors.Errorf("no constraints for version: %s found", version)
}

if len(matchedConstrain) > 1 {
var matched []string
for _, c := range matchedConstrain {
matched = append(matched, c.String())
}

return nil, errors.Errorf(
"more than one constraint for version: %s\n%s",
version,
strings.Join(matched, "\n"),
)
}

return matchedConstrain[0], nil
}

func validateConstrainForEnvoy(constrain string, version string) error {
if constrain != version {
return errors.Errorf("envoy version in constrain: %s doesn't equal expected one: %s", constrain, version)
}

return nil
}

type envoyVersion struct {
Envoy string
}

type versions struct {
KumaDp map[string]envoyVersion
}

var _ = Describe("Versions WS", func() {
It("should return the supported versions", func() {
// setup
Expand All @@ -39,78 +114,69 @@ var _ = Describe("Versions WS", func() {
Expect(err).ToNot(HaveOccurred())

// then
var data struct {
KumaDp map[string]struct {
Envoy string
}
}
var data versions

Expect(json.NewDecoder(resp.Body).Decode(&data)).ToNot(HaveOccurred())

// 1.0.0
constraints, err := buildConstraints(data.KumaDp)
Expect(err).ToNot(HaveOccurred())

Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["1.0.0"]).ToNot(BeNil())
Expect(data.KumaDp["1.0.0"].Envoy).To(Equal("1.16.0"))

// 1.0.0
constrain, err := getConstraint(constraints, "1.0.0")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "1.16.0")).To(Succeed())

// 1.0.1
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["1.0.1"]).ToNot(BeNil())
Expect(data.KumaDp["1.0.1"].Envoy).To(Equal("1.16.0"))
constrain, err = getConstraint(constraints, "1.0.1")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "1.16.0")).To(Succeed())

// 1.0.2
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["1.0.2"]).ToNot(BeNil())
Expect(data.KumaDp["1.0.2"].Envoy).To(Equal("1.16.1"))
constrain, err = getConstraint(constraints, "1.0.2")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "1.16.1")).To(Succeed())

// 1.0.3
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["1.0.3"]).ToNot(BeNil())
Expect(data.KumaDp["1.0.3"].Envoy).To(Equal("1.16.1"))
constrain, err = getConstraint(constraints, "1.0.3")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "1.16.1")).To(Succeed())

// 1.0.4
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["1.0.4"]).ToNot(BeNil())
Expect(data.KumaDp["1.0.4"].Envoy).To(Equal("1.16.1"))
constrain, err = getConstraint(constraints, "1.0.4")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "1.16.1")).To(Succeed())

// 1.0.5
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["1.0.5"]).ToNot(BeNil())
Expect(data.KumaDp["1.0.5"].Envoy).To(Equal("1.16.2"))
constrain, err = getConstraint(constraints, "1.0.5")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "1.16.2")).To(Succeed())

// 1.0.6
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["1.0.6"]).ToNot(BeNil())
Expect(data.KumaDp["1.0.6"].Envoy).To(Equal("1.16.2"))
constrain, err = getConstraint(constraints, "1.0.6")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "1.16.2")).To(Succeed())

// 1.0.7
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["1.0.7"]).ToNot(BeNil())
Expect(data.KumaDp["1.0.7"].Envoy).To(Equal("1.16.2"))
constrain, err = getConstraint(constraints, "1.0.7")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "1.16.2")).To(Succeed())

// 1.0.8
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["1.0.8"]).ToNot(BeNil())
Expect(data.KumaDp["1.0.8"].Envoy).To(Equal("1.16.2"))

// ^1.1.0
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["^1.1.0"]).ToNot(BeNil())
Expect(data.KumaDp["^1.1.0"].Envoy).To(Equal("~1.17.0"))

// ^1.2.0
Expect(data).ToNot(BeNil())
Expect(data.KumaDp).ToNot(BeNil())
Expect(data.KumaDp["^1.2.0"]).ToNot(BeNil())
Expect(data.KumaDp["^1.2.0"].Envoy).To(Equal("~1.18.0"))
constrain, err = getConstraint(constraints, "1.0.8")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "1.16.2")).To(Succeed())

// ~1.1.0
constrain, err = getConstraint(constraints, "1.1.0")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "~1.17.0")).To(Succeed())

// ~1.2.0
constrain, err = getConstraint(constraints, "1.2.0")
Expect(err).NotTo(HaveOccurred())
Expect(validateConstrainForEnvoy(data.KumaDp[constrain.String()].Envoy, "~1.18.0")).To(Succeed())
})
})

0 comments on commit e2bd183

Please sign in to comment.