Repository: knative/serving
Pull request #10532 (2021-01-12 23:49:43)

Ensure step size is validated and add tests
Cover one more thing that users can break by manually editing the annotation.

/assign @tcnghia 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/serving/pull/10532#" title="Author self-approved">vagababov</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fserving).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[pkg/reconciler/route/OWNERS](https://github.com/knative/serving/blob/master/pkg/reconciler/route/OWNERS)~~ [vagababov]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/serving/pull/10532?src=pr&el=h1) Report
> Merging [#10532](https://codecov.io/gh/knative/serving/pull/10532?src=pr&el=desc) (f66cce5) into [master](https://codecov.io/gh/knative/serving/commit/3ec271c55beb2099086966bb3035011894a4c2b7?el=desc) (3ec271c) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/serving/pull/10532/graphs/tree.svg?width=650&height=150&src=pr&token=MPpDfAj4Ui)](https://codecov.io/gh/knative/serving/pull/10532?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #10532   +/-   ##
=======================================
  Coverage   88.08%   88.08%           
=======================================
  Files         187      187           
  Lines        8868     8870    +2     
=======================================
+ Hits         7811     7813    +2     
  Misses        816      816           
  Partials      241      241           
```


| [Impacted Files](https://codecov.io/gh/knative/serving/pull/10532?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/reconciler/route/traffic/rollout.go](https://codecov.io/gh/knative/serving/pull/10532/diff?src=pr&el=tree#diff-cGtnL3JlY29uY2lsZXIvcm91dGUvdHJhZmZpYy9yb2xsb3V0Lmdv) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/serving/pull/10532?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/serving/pull/10532?src=pr&el=footer). Last update [3ec271c...f66cce5](https://codecov.io/gh/knative/serving/pull/10532?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/reconciler/route/traffic/rollout.go b/pkg/reconciler/route/traffic/rollout.go
index a7fa4d924..8ead05262 100644
--- a/pkg/reconciler/route/traffic/rollout.go
+++ b/pkg/reconciler/route/traffic/rollout.go
@@ -137,6 +137,10 @@ func (cur *Rollout) Validate() bool {
 		if c.Percent > 100 {
 			return false
 		}
+		// Ensure step size is valid.
+		if c.StepParams.StepSize < 0 || c.StepParams.StepSize > c.Percent {
+			return false
+		}
 		// If total % values in the revision do not add up — discard.
 		tot := 0
 		for _, r := range c.Revisions {


Test patch:
diff --git a/pkg/reconciler/route/traffic/rollout_test.go b/pkg/reconciler/route/traffic/rollout_test.go
index 948099806..6a83205a2 100644
--- a/pkg/reconciler/route/traffic/rollout_test.go
+++ b/pkg/reconciler/route/traffic/rollout_test.go
@@ -1273,6 +1273,36 @@ func TestValidateFailures(t *testing.T) {
 				}},
 			}},
 		},
+	}, {
+		name: "step larger than total",
+		r: &Rollout{
+			Configurations: []*ConfigurationRollout{{
+				ConfigurationName: "keith",
+				Percent:           71,
+				StepParams: RolloutParams{
+					StepSize: 75,
+				},
+				Revisions: []RevisionRollout{{
+					RevisionName: "black-on-blue",
+					Percent:      71,
+				}},
+			}},
+		},
+	}, {
+		name: "step negative",
+		r: &Rollout{
+			Configurations: []*ConfigurationRollout{{
+				ConfigurationName: "keith",
+				Percent:           71,
+				StepParams: RolloutParams{
+					StepSize: -1,
+				},
+				Revisions: []RevisionRollout{{
+					RevisionName: "black-on-blue",
+					Percent:      71,
+				}},
+			}},
+		},
 	}, {
 		name: "rev more than config",
 		r: &Rollout{
diff --git a/third_party/istio-latest/net-istio.yaml b/third_party/istio-latest/net-istio.yaml
index fc44fb321..41d3cf81e 100644
--- a/third_party/istio-latest/net-istio.yaml
+++ b/third_party/istio-latest/net-istio.yaml
@@ -20,7 +20,7 @@ metadata:
   # These are the permissions needed by the Istio Ingress implementation.
   name: knative-serving-istio
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     serving.knative.dev/controller: "true"
     networking.knative.dev/ingress-provider: istio
 rules:
@@ -50,7 +50,7 @@ metadata:
   name: knative-ingress-gateway
   namespace: knative-serving
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 spec:
   selector:
@@ -87,7 +87,7 @@ metadata:
   name: knative-local-gateway
   namespace: knative-serving
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 spec:
   selector:
@@ -106,7 +106,7 @@ metadata:
   name: knative-local-gateway
   namespace: istio-system
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 spec:
   type: ClusterIP
@@ -126,7 +126,7 @@ metadata:
   name: "webhook"
   namespace: "knative-serving"
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 spec:
   selector:
@@ -141,7 +141,7 @@ metadata:
   name: "istio-webhook"
   namespace: "knative-serving"
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 spec:
   selector:
@@ -170,7 +170,7 @@ kind: MutatingWebhookConfiguration
 metadata:
   name: webhook.istio.networking.internal.knative.dev
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 webhooks:
   - admissionReviewVersions:
@@ -206,7 +206,7 @@ kind: ValidatingWebhookConfiguration
 metadata:
   name: config.webhook.istio.networking.internal.knative.dev
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 webhooks:
   - admissionReviewVersions:
@@ -244,7 +244,7 @@ metadata:
   name: istio-webhook-certs
   namespace: knative-serving
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 
 ---
@@ -268,7 +268,7 @@ metadata:
   name: config-istio
   namespace: knative-serving
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 data:
   _example: |
@@ -342,7 +342,7 @@ metadata:
   name: networking-istio
   namespace: knative-serving
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 spec:
   selector:
@@ -358,7 +358,7 @@ spec:
         sidecar.istio.io/inject: "false"
       labels:
         app: networking-istio
-        serving.knative.dev/release: "v20210111-9f0302e4"
+        serving.knative.dev/release: "v20210112-9f0302e4"
     spec:
       serviceAccountName: controller
       containers:
@@ -422,7 +422,7 @@ metadata:
   name: istio-webhook
   namespace: knative-serving
   labels:
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 spec:
   selector:
@@ -436,7 +436,7 @@ spec:
       labels:
         app: istio-webhook
         role: istio-webhook
-        serving.knative.dev/release: "v20210111-9f0302e4"
+        serving.knative.dev/release: "v20210112-9f0302e4"
     spec:
       serviceAccountName: controller
       containers:
@@ -497,7 +497,7 @@ metadata:
   namespace: knative-serving
   labels:
     role: istio-webhook
-    serving.knative.dev/release: "v20210111-9f0302e4"
+    serving.knative.dev/release: "v20210112-9f0302e4"
     networking.knative.dev/ingress-provider: istio
 spec:
   ports: