Skip to content

Commit

Permalink
Adds sanity checks for values of --traffic flags
Browse files Browse the repository at this point in the history
- percent value should range between 0-100
- repetition of revision reference in --traffic flag is not allowed
  • Loading branch information
Gong Zhang authored and navidshaikh committed Aug 14, 2019
1 parent 6e86ddf commit f6c94d7
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 13 deletions.
33 changes: 20 additions & 13 deletions pkg/kn/traffic/compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,14 @@ func errorOverWritingTag(tag string) error {
"add flag '--untag %s' in command to untag it", tag)
}

func errorRepeatingLatestRevision(forFlag string) error {
return fmt.Errorf("repetition of identifier %s "+
"is not allowed, use only once with %s flag", latestRevisionRef, forFlag)
func errorRepeatingRevision(forFlag string, name string) error {
if name == latestRevisionRef {
name = "identifier " + latestRevisionRef
} else {
name = "revision reference " + name
}
return fmt.Errorf("repetition of %s "+
"is not allowed, use only once with %s flag", name, forFlag)
}

// verifies if user has repeated @latest field in --tag or --traffic flags
Expand All @@ -208,7 +213,6 @@ func errorRepeatingLatestRevision(forFlag string) error {
// - if provided traffic portion are integers
func verifyInputSanity(trafficFlags *flags.Traffic) error {
var latestRevisionTag = false
var latestRevisionTraffic = false
var sum = 0

for _, each := range trafficFlags.RevisionsTags {
Expand All @@ -218,32 +222,35 @@ func verifyInputSanity(trafficFlags *flags.Traffic) error {
}

if latestRevisionTag && revision == latestRevisionRef {
return errorRepeatingLatestRevision("--tag")

return errorRepeatingRevision("--tag", latestRevisionRef)
}

if revision == latestRevisionRef {
latestRevisionTag = true
}
}

for _, each := range trafficFlags.RevisionsPercentages {
revisionRefMap := make(map[string]int)
for i, each := range trafficFlags.RevisionsPercentages {
revisionRef, percent, err := splitByEqualSign(each)
if err != nil {
return err
}

// To check if there are duplicate revision names in traffic flags
if _, exist := revisionRefMap[revisionRef]; exist {
return errorRepeatingRevision("--traffic", revisionRef)
} else {
revisionRefMap[revisionRef] = i
}

percentInt, err := strconv.Atoi(percent)
if err != nil {
return fmt.Errorf("error converting given %s to integer value for traffic distribution", percent)
}

if latestRevisionTraffic && revisionRef == latestRevisionRef {
return errorRepeatingLatestRevision("--traffic")
}

if revisionRef == latestRevisionRef {
latestRevisionTraffic = true
if percentInt < 0 || percentInt > 100 {
return fmt.Errorf("invalid value for traffic percent %d, expected 0 <= percent <= 100", percentInt)
}

sum += percentInt
Expand Down
12 changes: 12 additions & 0 deletions pkg/kn/traffic/compute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,18 @@ func TestComputeErrMsg(t *testing.T) {
[]string{"--traffic", "@latest=19,echo-v1=71"},
"given traffic percents sum to 90, want 100",
},
{
"verify error for values out of range given to percent",
append(newServiceTraffic([]v1alpha1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--traffic", "@latest=-100"},
"invalid value for traffic percent -100, expected 0 <= percent <= 100",
},
{
"repeatedly spliting traffic to the same revision",
append(newServiceTraffic([]v1alpha1.TrafficTarget{}), newTarget("", "", 100, true)),
[]string{"--traffic", "echo-v1=40", "--traffic", "echo-v1=60"},
"repetition of revision reference echo-v1 is not allowed, use only once with --traffic flag",
},
} {
t.Run(testCase.name, func(t *testing.T) {
testCmd, tFlags := newTestTrafficCommand()
Expand Down

0 comments on commit f6c94d7

Please sign in to comment.