Vap for updates #4165

chris-kaiser-7 · 2025-10-12T23:01:37Z

What type of PR is this?

/kind feature

What this PR does / why we need it:
This is required for the release changes https://docs.google.com/document/d/1iG6RKVJFZUxG1mZ4NbUL5Tm09n_zDck-LukYmUXoQ7c/edit?tab=t.0

Which issue(s) this PR fixes:

Fixes #
VAP for Upgrades #4162

Does this PR introduce a user-facing change?:

Adds a VAP that prohibits the following:
- Installation of experimental CRDs on top of standard channel CRDs (within the same API group)
- Installation of monthly releases
- Installation of older releases

k8s-ci-robot · 2025-10-12T23:01:47Z

Hi @chris-kaiser-7. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

kflynn

Couple of very minor nits -- thanks very much for digging into this!

kflynn · 2025-10-16T19:31:28Z

config/crd/standard/gateway.networking.k8s.io_vap_safeupgrades.yaml

+    api-approved.kubernetes.io: https://github.com/kubernetes-sigs/gateway-api/pull/3328
+    gateway.networking.k8s.io/bundle-version: v1.4.0
+    gateway.networking.k8s.io/channel: standard
+  name: "gateway-api-safe-upgrades.gateway.networking.k8s.io"


🤔 Should we drop the domain here?

You think it should be named "safe-upgrades.gateway.networking.k8s.io"? Yea it is a little redundant to have gateway api twice.

You think it should be named "safe-upgrades.gateway.networking.k8s.io"? Yea it is a little redundant to have gateway api twice.

Yep, I think that would be a nice simplification here

config/crd/standard/gateway.networking.k8s.io_safeUpgradesVAP.yaml

kflynn · 2025-10-16T19:34:10Z

/approve

Looks good to me with a couple of minor comments, leaving the final LGTM for someone else -- thanks, @chris-kaiser-7! 🙂

k8s-ci-robot · 2025-10-16T19:34:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chris-kaiser-7, kflynn
Once this PR has been reviewed and has the lgtm label, please assign aojea for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

kflynn · 2025-10-16T19:34:23Z

/ok-to-test

…yaml Co-authored-by: Flynn <[email protected]>

howardjohn · 2025-10-16T20:23:09Z

config/crd/standard/gateway.networking.k8s.io_safeUpgradesVAP.yaml

+      operations:  ["CREATE", "UPDATE"]
+      resources:   ["*"]
+  validations:
+    - expression: "object.spec.group != 'gateway.networking.k8s.io' || 


this doesn't block XListenerSet, XMesh, etc. Is that intended?

No its not. I'll fix that.

robscott

Thanks @chris-kaiser-7, this is a really great change! Any chance that you could include some basic tests for this VAP?

I'm thinking that we'd want them to validate the following behavior when the VAP is installed:

Experimental CRDs that are part of x-k8s.io can be installed
Experimental CRDs that are part of k8s.io can NOT be installed
Standard CRDs with an old version can NOT be installed
Standard CRDs with the current version can be installed

It might actually be possible in https://github.com/kubernetes-sigs/gateway-api/blob/150554d3a55b731d3261525cf265bc930d2d5d85/pkg/test/crd/crd_test.go, but I'll defer to @rikatz on the best home for this.

config/crd/standard/gateway.networking.k8s.io_safeUpgradesVAP.yaml

robscott · 2025-10-16T22:29:18Z

config/crd/standard/gateway.networking.k8s.io_safeUpgradesVAP.yaml

+      message: "Installing experimental CRDs on top of standard channel CRDs is prohibited by default. Uninstall ValidatingAdmissionPolicy gateway-api-safe-upgrades.gateway.networking.k8s.io to install experimental CRDs on top of standard channel CRDs."
+      reason: Invalid
+    - expression: "(object.spec.group != 'gateway.networking.k8s.io' && object.spec.group != 'gateway.networking.x-k8s.io') || 
+        (!matches(object.metadata.annotations['gateway.networking.k8s.io/bundle-version'], 'v1.[0-3].\\\\d+') &&


Mind adding a note to RELEASE.md to include a step to update this value?

Oh yea good idea. The logic is a bit weird because CEL doesn't allow match groups in regex as far as I can tell so the regex gets a bit long.

I added a note in RELEASE.md. I was thinking I could add a script in hack to update this regex based on the bundle version. That would make the process easier.

config/crd/kustomization.yaml

config/crd/standard/gateway.networking.k8s.io_safeUpgradesVAP.yaml

robscott · 2025-10-16T22:32:29Z

config/crd/standard/gateway.networking.k8s.io_vap_safeupgrades.yaml

+  annotations:
+    api-approved.kubernetes.io: https://github.com/kubernetes-sigs/gateway-api/pull/3328
+    gateway.networking.k8s.io/bundle-version: v1.4.0
+    gateway.networking.k8s.io/channel: standard


I'm not 100% sure if this should be part of a release channel or not. Fine for now, but may want to adjust if we make a VAP that is meant to be included with both release channels.

I was wondering about this too.

config/crd/standard/gateway.networking.k8s.io_safeUpgradesVAP.yaml

robscott · 2025-10-16T22:32:53Z

config/crd/standard/gateway.networking.k8s.io_vap_safeupgrades.yaml

+  annotations:
+    api-approved.kubernetes.io: https://github.com/kubernetes-sigs/gateway-api/pull/3328
+    gateway.networking.k8s.io/bundle-version: v1.4.0
+    gateway.networking.k8s.io/channel: standard


Same comment as above - unsure if this is necessary

rikatz · 2025-10-16T22:51:42Z

/cc
/assign

chris-kaiser-7 · 2025-10-18T22:38:01Z

Thanks @chris-kaiser-7, this is a really great change! Any chance that you could include some basic tests for this VAP?

I'm thinking that we'd want them to validate the following behavior when the VAP is installed:

Experimental CRDs that are part of x-k8s.io can be installed

Experimental CRDs that are part of k8s.io can NOT be installed

Standard CRDs with an old version can NOT be installed

Standard CRDs with the current version can be installed

It might actually be possible in https://github.com/kubernetes-sigs/gateway-api/blob/150554d3a55b731d3261525cf265bc930d2d5d85/pkg/test/crd/crd_test.go, but I'll defer to @rikatz on the best home for this.

Thanks for the review. I just started working on the tests.

rikatz · 2025-10-22T16:41:51Z

config/crd/standard/gateway.networking.k8s.io_vap_safeupgrades.yaml

+  matchResources:
+    objectSelector: {}
+
+# !has(object.metadata.annotations['gateway.networking.k8s.io/channel']) ||


is this something required or something that was on file and was left behind?

Yea I didn't notice that. I cleaned it up.

rikatz · 2025-10-22T16:42:09Z

config/crd/standard/gateway.networking.k8s.io_vap_safeupgrades.yaml

+  policyName: gateway-api-safe-upgrades.gateway.networking.k8s.io
+  validationActions: [Deny]
+  matchResources:
+    objectSelector: {}


maybe add the object selector of CRDs here?

I would say something like:

spec.matchResources.resourceRules.apiGroups = apiextensions.k8s.io spec.matchResources...apiVersions=v1 spec.matchResources...resources=customresourcedefinitions

Not sure this would properly work, but if so the idea is at least to filter out this binding and not send every resource validation to our VAP

I did look into this but had issues selecting crds with this. I'll look into again tho.

OK never mind it seems to be working as you said. I must have done something wrong before when testing resourcerRules in binding.

chris-kaiser-7 · 2025-10-23T01:04:54Z

Thanks @chris-kaiser-7, this is a really great change! Any chance that you could include some basic tests for this VAP?

I'm thinking that we'd want them to validate the following behavior when the VAP is installed:

Experimental CRDs that are part of x-k8s.io can be installed

Experimental CRDs that are part of k8s.io can NOT be installed

Standard CRDs with an old version can NOT be installed

Standard CRDs with the current version can be installed

It might actually be possible in https://github.com/kubernetes-sigs/gateway-api/blob/150554d3a55b731d3261525cf265bc930d2d5d85/pkg/test/crd/crd_test.go, but I'll defer to @rikatz on the best home for this.

I pushed some tests in CRD_test. I can move them to a separate test file if that is better. I also made a few changes to that. I pulled some of the init tests up a level so you can run isolated tests like. "make test.crds-validation GO_TEST_FLAGS="-run TestCRDValidation/safeupgrades_VAP_should_validate_correctly"".

chris-kaiser-7 · 2025-10-23T01:06:12Z

I noticed a race condition and spent a while trying to fix it without time.sleep. I'm going to look at this some more but figured I should push what I got.

chris-kaiser-7 · 2025-11-03T14:52:26Z

@rikatz @robscott See anything else that needs fixing?

robscott

Thanks @chris-kaiser-7!

robscott · 2025-11-03T17:52:55Z

pkg/test/crd/standard-install-1.3.0.yaml

@@ -0,0 +1,10611 @@
+# Copyright 2025 The Kubernetes Authors.


Instead of including the entire release manifest from previous releases, let's just add some very lightweight manifests with a variety of annotations (different release channels, versions, etc).

+1! Or pick some existing manifest from https://github.com/kubernetes-sigs/gateway-api/tree/main/config/crd and mutate it accordingly.

good call! I work on this.

robscott · 2025-11-03T17:54:51Z

pkg/test/crd/crd_test.go

-	t.Run("should be able to set kubectl and kubeconfig and connect to the cluster", func(t *testing.T) {
-		kubectlLocation = testEnv.ControlPlane.KubectlPath
-		require.NotEmpty(t, kubectlLocation)


Any reason for removing this?

I put this logic in the parent test so that you could run targeted tests without issue. for example "make test.crds-validation GO_TEST_FLAGS="-run TestCRDValidation/safeupgrades_VAP_should_validate_correctly"". would not work as well as a few other tests because it depended on logic in this test.

rikatz · 2025-11-04T19:25:14Z

config/crd/standard/gateway.networking.k8s.io_vap_safeupgrades.yaml

+        (has(object.metadata.annotations) && object.metadata.annotations.exists(k, k == 'gateway.networking.k8s.io/bundle-version') && 
+        !matches(object.metadata.annotations['gateway.networking.k8s.io/bundle-version'], 'v1.[0-3].\\\\d+') &&
+        !matches(object.metadata.annotations['gateway.networking.k8s.io/bundle-version'], 'v0'))"
+      message: "Installing CRDs with version before v1.4.0 is prohibited by default. Uninstall ValidatingAdmissionPolicy gateway-api-safe-upgrades.gateway.networking.k8s.io to install older versions."


I think the goal here is to block the installation of older CRDs (avoid downgrade), right?

Can we add a note/comment that on Kubernetes 1.37 we should migrate this to something like https://kubernetes.io/docs/reference/using-api/cel/#kubernetes-semver-library instead?

This way, instead of having this bump as part of the release process, we can probably extract the version directly and check if it is older than the current existing.

Yea this is good. I didn't know this was in CEL for Kubernetes. I was just working off the base CEL documentation and that was kind of frustrating to say the least. I'll update it to this for 1.37. There is a lot of other good stuff in that doc too I didn't know about.

rikatz · 2025-11-04T19:25:51Z

config/crd/standard/gateway.networking.k8s.io_vap_safeupgrades.yaml

+kind: ValidatingAdmissionPolicyBinding
+metadata:
+  annotations:
+    gateway.networking.k8s.io/bundle-version: v1.4.0


As this will be released with main, should we make this v1.5.0?

Yea that makes sense. I'll fix that.

rikatz · 2025-11-04T19:28:53Z

overall lgtm, thanks.

I just want to be sure that:

We have a comment and an action item to use the semver library on vap once we can safely rely on it (eg.: Kubernetes 1.34 added it, once 1.37 is released we could rely on it as we would drop support for 1.33).
The CRD generation can be much much smaller as Rob pointed out. or even use the existing one, but do some "inplace replacement" just of the annotation (like some strings.Replace)

chris-kaiser-7 · 2025-11-04T20:09:01Z

overall lgtm, thanks.

I just want to be sure that:

We have a comment and an action item to use the semver library on vap once we can safely rely on it (eg.: Kubernetes 1.34 added it, once 1.37 is released we could rely on it as we would drop support for 1.33).

The CRD generation can be much much smaller as Rob pointed out. or even use the existing one, but do some "inplace replacement" just of the annotation (like some strings.Replace)

@rikatz @robscott
Thanks for the feedback. I was considering using logic like strings.Replace but I was a bit worried about the integrity of the test if it has logical dependencies like that but that might be a little to nit. Though if you're both ok with that I can definitely do that.

chris-kaiser-7 added 2 commits October 12, 2025 18:52

added vap for experimental crds on standard crds

70ea7d7

tidy

44d7b2c

k8s-ci-robot requested review from aojea and robscott October 12, 2025 23:01

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 12, 2025

chris-kaiser-7 changed the title ~~[WIP] Vap for updates 4162~~ [WIP] Vap for updates Oct 12, 2025

added validation for older versions

3f5a154

kflynn reviewed Oct 16, 2025

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 16, 2025

Update config/crd/standard/gateway.networking.k8s.io_safeUpgradesVAP.…

883bffa

…yaml Co-authored-by: Flynn <[email protected]>

howardjohn reviewed Oct 16, 2025

View reviewed changes

added x-k8s to validation

8c00ec9

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 16, 2025

chris-kaiser-7 added 2 commits October 16, 2025 17:35

tidy

a865143

tidy

a3b8c74

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 16, 2025

robscott reviewed Oct 16, 2025

View reviewed changes

k8s-ci-robot assigned rikatz Oct 16, 2025

k8s-ci-robot requested a review from rikatz October 16, 2025 22:51

chris-kaiser-7 added 2 commits October 18, 2025 18:16

added validation and tidy

deb203d

renamed vaps.

a923fc1

rikatz reviewed Oct 22, 2025

View reviewed changes

added tests for VAP and tidy

faac444

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 23, 2025

chris-kaiser-7 added 2 commits October 22, 2025 23:55

added resource rules to vap binding.

737de6b

added note in RELEASE.md

dda4086

chris-kaiser-7 changed the title ~~[WIP] Vap for updates~~ Vap for updates Nov 3, 2025

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 3, 2025

robscott reviewed Nov 3, 2025

View reviewed changes

rikatz reviewed Nov 4, 2025

View reviewed changes

chris-kaiser-7 closed this Nov 4, 2025

chris-kaiser-7 reopened this Nov 4, 2025

Vap for updates #4165

Are you sure you want to change the base?

Vap for updates #4165

Conversation

chris-kaiser-7 commented Oct 12, 2025

Uh oh!

k8s-ci-robot commented Oct 12, 2025

Uh oh!

kflynn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chris-kaiser-7 Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kflynn commented Oct 16, 2025

Uh oh!

k8s-ci-robot commented Oct 16, 2025

Uh oh!

kflynn commented Oct 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robscott left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rikatz commented Oct 16, 2025

Uh oh!

chris-kaiser-7 commented Oct 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chris-kaiser-7 commented Oct 23, 2025

Uh oh!

chris-kaiser-7 commented Oct 23, 2025

Uh oh!

chris-kaiser-7 commented Nov 3, 2025

Uh oh!

robscott left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chris-kaiser-7 Oct 16, 2025 •

edited

Loading

chris-kaiser-7 Nov 4, 2025 •

edited

Loading