Graduate server side validation to beta #110178

kevindelgado · 2022-05-23T20:54:34Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Graduate server side field validation to beta and enable it by default.

All kubectl requests that set --validate=true by default will do so via strict server-side rather than client-side (for all requests targeting clusters with this feature enabled).

The apiserver will default to warning on unknown/invalid fields unless strict or ignore validation is requested.

Which issue(s) this PR fixes:

Refers to: kubernetes/enhancements#2885

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Graduated ServerSideFieldValidation to `beta`. Schema validation is performed server-side and requests will receive warnings for any invalid/unknown fields by default.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

KEP: https://github.com/kubernetes/enhancements/issues/2885

leilajal · 2022-05-24T20:11:38Z

/triage accepted

kevindelgado · 2022-07-13T23:05:16Z

/retest

kevindelgado · 2022-07-18T15:07:58Z

/assign @liggitt

pkg/features/kube_features.go

liggitt · 2022-07-18T15:09:41Z

test/e2e/kubectl/kubectl.go

 	{
 		"apiVersion": "apps/v1",
 		"kind": "Deployment",
 		"metadata": {
 			"name": "my-dep",
+			"namespace": "%s",


was this change required? I don't remember this bit

I added it because of #110509

oh, huh... should it have been added in #110509 then? I just wanted to make sure it wasn't newly required because of the server-side field validation feature defaulting on

if this test passes without this change, let's hoist this namespace addition to a separate PR to avoid entangling an unrelated fix in the feature enablement PR

Yea, I've removed it.

I'm trying to remember if it was actually needed or not. I went through the PR CI history, and it doesn't seem like the test was actually breaking because of this.

Also, looking at #110509, the error there was from hardcoding the namespace not omitting it. If this PR passes without it, I don't think we need it.

I think maybe I just got nervous seeing a kubectl call in an e2e test I was working on without the namespace set and just added it to be safe, but I don't think it's actually necessary

kevindelgado

PTAL @liggitt

kevindelgado · 2022-07-18T15:38:17Z

test/e2e/kubectl/kubectl.go

 	{
 		"apiVersion": "apps/v1",
 		"kind": "Deployment",
 		"metadata": {
 			"name": "my-dep",
+			"namespace": "%s",


I added it because of #110509

pkg/features/kube_features.go

liggitt · 2022-07-18T16:52:19Z

last two commits (feature enablement and test addition) this PR adds on top of #109494 lgtm

/lgtm
/approve

k8s-ci-robot · 2022-07-18T16:52:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kevindelgado, liggitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [liggitt]
~~staging/src/k8s.io/api/OWNERS~~ [liggitt]
~~staging/src/k8s.io/legacy-cloud-providers/OWNERS~~ [liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

liggitt · 2022-07-18T18:01:07Z

/retest

liggitt · 2022-07-18T18:49:57Z

/retest

kubectl wait unit test flake is an unrelated known issue

kevindelgado · 2022-07-18T22:09:32Z

/retest

kevindelgado · 2022-07-18T23:56:55Z

/retest

liggitt · 2022-07-19T02:11:50Z

/kind feature

liggitt · 2022-07-19T03:36:26Z

/retest

k8s-triage-robot · 2022-07-19T06:37:45Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-triage-robot · 2022-07-19T10:37:45Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

k8s-ci-robot requested a review from dchen1107 May 23, 2022 20:54

k8s-ci-robot added the area/test label May 23, 2022

k8s-ci-robot requested a review from KnVerey May 23, 2022 20:54

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 24, 2022

kevindelgado force-pushed the validation-beta-1-25 branch 3 times, most recently from cd37a4a to 8db54c1 Compare May 24, 2022 23:52

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 26, 2022

kevindelgado force-pushed the validation-beta-1-25 branch from 8716ff1 to 54d1a55 Compare May 26, 2022 06:55

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 26, 2022

kevindelgado force-pushed the validation-beta-1-25 branch from 54d1a55 to eaa42a1 Compare May 26, 2022 06:59

kevindelgado mentioned this pull request May 26, 2022

Server Side Unknown Field Validation kubernetes/enhancements#2885

Closed

26 tasks

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 11, 2022

kevindelgado added 2 commits July 18, 2022 14:55

Add server-side metadata unknown field validation

d983fd8

update kjson

c210483

kevindelgado force-pushed the validation-beta-1-25 branch from 43a3a8b to a8ab2dd Compare July 18, 2022 15:01

k8s-ci-robot assigned liggitt Jul 18, 2022

liggitt reviewed Jul 18, 2022

View reviewed changes

pkg/features/kube_features.go Show resolved Hide resolved

liggitt reviewed Jul 18, 2022

View reviewed changes

Enable ServerSideFieldValidation feature gate on by default in beta

36dbecd

kevindelgado force-pushed the validation-beta-1-25 branch from a8ab2dd to 6e6e60b Compare July 18, 2022 15:35

kevindelgado commented Jul 18, 2022

View reviewed changes

kevindelgado requested a review from liggitt July 18, 2022 15:38

Adjust testing for server-side validation as default

d815449

kevindelgado force-pushed the validation-beta-1-25 branch from 6e6e60b to d815449 Compare July 18, 2022 16:17

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 18, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 18, 2022

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jul 19, 2022

k8s-ci-robot merged commit eeb12bb into kubernetes:master Jul 19, 2022

k8s-ci-robot added this to the v1.25 milestone Jul 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Graduate server side validation to beta #110178

Graduate server side validation to beta #110178

kevindelgado commented May 23, 2022 •

edited

Loading

leilajal commented May 24, 2022

kevindelgado commented Jul 13, 2022

kevindelgado commented Jul 18, 2022

liggitt Jul 18, 2022

kevindelgado Jul 18, 2022

liggitt Jul 18, 2022

kevindelgado Jul 18, 2022 •

edited

Loading

kevindelgado left a comment

kevindelgado Jul 18, 2022

liggitt commented Jul 18, 2022

k8s-ci-robot commented Jul 18, 2022

liggitt commented Jul 18, 2022

liggitt commented Jul 18, 2022

kevindelgado commented Jul 18, 2022

kevindelgado commented Jul 18, 2022

liggitt commented Jul 19, 2022

liggitt commented Jul 19, 2022

k8s-triage-robot commented Jul 19, 2022

k8s-triage-robot commented Jul 19, 2022

Graduate server side validation to beta #110178

Graduate server side validation to beta #110178

Conversation

kevindelgado commented May 23, 2022 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

leilajal commented May 24, 2022

kevindelgado commented Jul 13, 2022

kevindelgado commented Jul 18, 2022

liggitt Jul 18, 2022

Choose a reason for hiding this comment

kevindelgado Jul 18, 2022

Choose a reason for hiding this comment

liggitt Jul 18, 2022

Choose a reason for hiding this comment

kevindelgado Jul 18, 2022 • edited Loading

Choose a reason for hiding this comment

kevindelgado left a comment

Choose a reason for hiding this comment

kevindelgado Jul 18, 2022

Choose a reason for hiding this comment

liggitt commented Jul 18, 2022

k8s-ci-robot commented Jul 18, 2022

liggitt commented Jul 18, 2022

liggitt commented Jul 18, 2022

kevindelgado commented Jul 18, 2022

kevindelgado commented Jul 18, 2022

liggitt commented Jul 19, 2022

liggitt commented Jul 19, 2022

k8s-triage-robot commented Jul 19, 2022

k8s-triage-robot commented Jul 19, 2022

kevindelgado commented May 23, 2022 •

edited

Loading

kevindelgado Jul 18, 2022 •

edited

Loading