Split webhook validation #10284

sheidkamp · 2024-11-06T16:56:14Z

Description

Split the ValidatingWebhookConfiguration into 2 webhooks to allow different FailurePolicies for gloo and non-gloo resources.

Code changes

None - changes are Helm, tests, and docs

CI changes

Added ValidationSplitWebhook tests to TestValidationStrict tests

Docs changes

Helm values documentation updated
https://github.com/solo-io/gloo/blob/75961241cc1982e62eace5b539aa8e5ed7d3837d/projects/gateway/pkg/services/k8sadmission/README.md updated

Context

Having changes to kubernetes core objects such as secrets and namespaces go through the Gloo Service's validating webhook can prevent changes unrelated to any gloo configuration from being applied if the FailurePolicy is Fail. By splitting the webhooks and allowing different FailurePolicies to be set, this can be avoided.

Testing steps

Create cluster and build images:

./ci/kind/setup-kind.sh

Install with the gloo FailurePolicy of Fail and kubes FailurePolicy of Ignore

 helm upgrade --install gloo _test/gloo-${VERSION}.tgz --create-namespace --namespace gloo-system --set-string gloo.deployment.image.tag=${VERSION} --set discovery.deployment.enablePodSecurityContext=true --set discovery.enabled=true --set gateway.validation.failurePolicy="Fail" --set gateway.validation.kubeCoreFailurePolicy="Ignore" --set gateway.validation.alwaysAcceptResources=false --set gateway.validation.allowWarnings=false --set gateway.validation.alwaysAcceptResources=false --set gateway.validation.allowWarnings=false

Create an upstream (Gloo resource) and a secret (Kube resource)

glooctl create upstream static --static-hosts jsonplaceholder.typicode.com:80 --name json-upstream -n gloo-system
glooctl create secret apikey infra-apikey --apikey doesntmatter

Scale down the Gloo deployment:

kubectl scale --replicas=0 deployment/gloo -n gloo-system

Try to delete the secret (should succeed)

 kubectl delete secret infra-apikey -n gloo-system

Try to delete the upstream (should fail)

delete upstream -n gloo-system json-upstream

Scale back up

kubectl scale --replicas=1 deployment/gloo -n gloo-system

Update the configuration to set the gloo webhook failurePolicy to Ignore and the kube webhook failurePolicy to Fail

 helm upgrade --install gloo _test/gloo-${VERSION}.tgz --namespace gloo-system --set gateway.validation.failurePolicy="Ignore" --set gateway.validation.kubeCoreFailurePolicy="Fail"

Recreate the deleted secret:

glooctl create secret apikey infra-apikey --apikey doesntmatter

Scale down the Gloo deployment:

kubectl scale --replicas=0 deployment/gloo -n gloo-system

Try to delete the secret (should fail)

 kubectl delete secret infra-apikey -n gloo-system

Try to delete the upstream (should succeed)

delete upstream -n gloo-system json-upstream

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works

solo-changelog-bot · 2024-11-06T16:56:25Z

Issues linked to changelog:
solo-io#10247

github-actions · 2024-11-06T17:10:47Z

Visit the preview URL for this PR (updated for commit 9c28910):

https://gloo-edge--pr10284-sheidkamp-split-vali-f1w7xqqf.web.app

_{(expires Wed, 20 Nov 2024 00:09:17 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 77c2b86e287749579b7ff9cadb81e099042ef677}

…/solo-io/gloo into sheidkamp/split-validating-webhook

install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml

sam-heilbron

I think that it could be valuable to capture the background for the split configs in https://github.com/solo-io/gloo/tree/main/projects/gateway/pkg/services/k8sadmission. That way we have a public source of truth for that decision.

The changes look good overall. I'd like to see unit tests for the new field, and i imagine we want to rely on the default value (fail open) for our e2e tests (so no changes would be needed)

install/helm/gloo/generate/values.go

install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml

install/helm/gloo/values-template.yaml

install/test/helm_test.go

sam-heilbron

These changes LGTM!

I know there are some small documentation changes that we intend to make, I just wanted to 👍 the general direction.

…/solo-io/gloo into sheidkamp/split-validating-webhook

jenshu · 2024-11-12T19:29:31Z

install/helm/gloo/generate/values.go

+	FailurePolicy                    *string  `json:"failurePolicy,omitempty" desc:"failurePolicy defines how unrecognized errors for Gloo resources from the Gateway validation endpoint are handled - allowed values are 'Ignore' or 'Fail'. Defaults to Ignore "`
+	KubeCoreFailurePolicy            *string  `json:"kubeCoreFailurePolicy,omitempty" desc:"kubeCoreFailurePolicy defines how unrecognized errors for core resources from the Gateway validation endpoint are handled - allowed values are 'Ignore' or 'Fail'. If this is set to 'Fail' modifications to core resources such as secrets and namespace that are defined in the validating webhook will be blocked if the Gloo Service is not available. Defaults to Ignore "`


nit: we don't need to mention the default values in the description since the docs will extract that from values-template.yaml

jenshu · 2024-11-12T19:32:09Z

test/kubernetes/e2e/features/validation/split_webhook/suite.go

+// var _ e2e.NewSuiteFunc = NewKubeFailTestingSuite
+// var _ e2e.NewSuiteFunc = NewGlooFailTestingSuite


jenshu · 2024-11-12T19:32:55Z

test/kubernetes/e2e/features/validation/split_webhook/suite.go

+func (s *testingSuite) TearDownSuite() {
+	// nothing at the moment
+}
+func (s *testingSuite) SetupDownSuite() {


jenshu · 2024-11-12T19:37:47Z

test/kubernetes/e2e/features/validation/split_webhook/suite.go

+	manifest := manifests[testName]
+	output, err := s.testInstallation.Actions.Kubectl().DeleteFileWithOutput(s.ctx, manifest.filename, "-n", s.testInstallation.Metadata.InstallNamespace)
+	// May have already been deleted
+	if err == nil {


what about the case where err != nil

updated to track successful deletion and only cleanup if neccessary.

install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml

jenshu · 2024-11-12T19:43:19Z

install/test/5-gateway-validation-webhook-configuration_test.go

+				structuredDeployment, ok := webhookObject.(*admissionregistrationv1.ValidatingWebhookConfiguration)
+				ExpectWithOffset(1, ok).To(BeTrue(), fmt.Sprintf("Webhook %+v should be able to cast to a structured deployment", webhook))
+
+				//ExpectWithOffset(1, structuredDeployment.Spec.Template.ObjectMeta.Annotations).To(BeEmpty(), fmt.Sprintf("No annotations should be present on deployment %+v", structuredDeployment))


still need this line?

nope, deleted

Co-authored-by: Jenny Shu <[email protected]>

naive split

4dad0c3

github-actions bot added keep pr updated signals bulldozer to keep pr up to date with base branch work in progress signals bulldozer to keep pr open (don't auto-merge) labels Nov 6, 2024

Merge main into sheidkamp/split-validating-webhook

fe34ac7

sheidkamp removed the keep pr updated signals bulldozer to keep pr up to date with base branch label Nov 6, 2024

create new helm value

0dd97b8

changelog-bot and others added 3 commits November 6, 2024 18:23

Adding changelog file to new location

21b533e

Deleting changelog file from old location

cd46675

Merge branch 'sheidkamp/split-validating-webhook' of ssh://github.com…

37d2f7e

…/solo-io/gloo into sheidkamp/split-validating-webhook

sheidkamp requested a review from a team as a code owner November 7, 2024 14:21

Update 5-gateway-validation-webhook-configuration.yaml

791aabd

jenshu reviewed Nov 7, 2024

View reviewed changes

install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml Outdated Show resolved Hide resolved

jenshu reviewed Nov 7, 2024

View reviewed changes

install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml Outdated Show resolved Hide resolved

tests pass

22a0d1d

sam-heilbron reviewed Nov 7, 2024

View reviewed changes

sheidkamp and others added 6 commits November 7, 2024 15:25

Fix up existing e2e tests

2baaaa4

renaming

ababa41

Fix renaming

c6b2de1

fix whitespace test

2fc33d6

Merge branch 'main' into sheidkamp/split-validating-webhook

f528618

Merge branch 'main' into sheidkamp/split-validating-webhook

6ff3019

sam-heilbron reviewed Nov 11, 2024

View reviewed changes

sheidkamp added 4 commits November 11, 2024 21:53

e2e tests

d1ad2a0

Update pr-kubernetes-tests.yaml

275560a

touch ups

da39106

Merge branch 'main' into sheidkamp/split-validating-webhook

7596124

sheidkamp added the keep pr updated signals bulldozer to keep pr up to date with base branch label Nov 12, 2024

Update split-validating-webhook.yaml

873c111

e2e updates

1e0dc8c

sheidkamp force-pushed the sheidkamp/split-validating-webhook branch from 7c91dac to 1e0dc8c Compare November 12, 2024 18:50

soloio-bulldozer bot and others added 3 commits November 12, 2024 18:51

Merge refs/heads/main into sheidkamp/split-validating-webhook

3807a3f

Unit test to check webhook not rendered when no need

e3c6edb

Merge branch 'sheidkamp/split-validating-webhook' of ssh://github.com…

52708e9

…/solo-io/gloo into sheidkamp/split-validating-webhook

sheidkamp removed the work in progress signals bulldozer to keep pr open (don't auto-merge) label Nov 12, 2024

sam-heilbron approved these changes Nov 12, 2024

View reviewed changes

jenshu reviewed Nov 12, 2024

View reviewed changes

sheidkamp added the work in progress signals bulldozer to keep pr open (don't auto-merge) label Nov 12, 2024

jenshu reviewed Nov 12, 2024

View reviewed changes

install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml Outdated Show resolved Hide resolved

jenshu reviewed Nov 12, 2024

View reviewed changes

install/helm/gloo/templates/5-gateway-validation-webhook-configuration.yaml Outdated Show resolved Hide resolved

jenshu reviewed Nov 12, 2024

View reviewed changes

jenshu approved these changes Nov 12, 2024

View reviewed changes

sheidkamp and others added 3 commits November 12, 2024 14:49

Apply suggestions from code review

f12af6a

Co-authored-by: Jenny Shu <[email protected]>

generate

37a77bb

PR feedback

16c4312

sheidkamp removed the work in progress signals bulldozer to keep pr open (don't auto-merge) label Nov 12, 2024

jenshu approved these changes Nov 12, 2024

View reviewed changes

Update pr-kubernetes-tests.yaml

e87c432

sam-heilbron approved these changes Nov 12, 2024

View reviewed changes

soloio-bulldozer bot and others added 3 commits November 12, 2024 21:15

Merge refs/heads/main into sheidkamp/split-validating-webhook

4fca811

Merge refs/heads/main into sheidkamp/split-validating-webhook

b6cfd33

Update suite.go

9c28910

sam-heilbron approved these changes Nov 13, 2024

View reviewed changes

soloio-bulldozer bot merged commit f40d935 into main Nov 13, 2024
19 checks passed

soloio-bulldozer bot deleted the sheidkamp/split-validating-webhook branch November 13, 2024 00:33

github-actions bot mentioned this pull request Nov 14, 2024

[Migrated] Split webhook validation solo-io/gloo#10284

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split webhook validation #10284

Split webhook validation #10284

sheidkamp commented Nov 6, 2024 •

edited

Loading

solo-changelog-bot bot commented Nov 6, 2024

github-actions bot commented Nov 6, 2024 •

edited

Loading

sam-heilbron left a comment

sam-heilbron left a comment

jenshu Nov 12, 2024

jenshu Nov 12, 2024

sheidkamp Nov 12, 2024

jenshu Nov 12, 2024

sheidkamp Nov 12, 2024

jenshu Nov 12, 2024

sheidkamp Nov 12, 2024

jenshu Nov 12, 2024

sheidkamp Nov 12, 2024

		FailurePolicy *string `json:"failurePolicy,omitempty" desc:"failurePolicy defines how unrecognized errors for Gloo resources from the Gateway validation endpoint are handled - allowed values are 'Ignore' or 'Fail'. Defaults to Ignore "`
		KubeCoreFailurePolicy *string `json:"kubeCoreFailurePolicy,omitempty" desc:"kubeCoreFailurePolicy defines how unrecognized errors for core resources from the Gateway validation endpoint are handled - allowed values are 'Ignore' or 'Fail'. If this is set to 'Fail' modifications to core resources such as secrets and namespace that are defined in the validating webhook will be blocked if the Gloo Service is not available. Defaults to Ignore "`

		// var _ e2e.NewSuiteFunc = NewKubeFailTestingSuite
		// var _ e2e.NewSuiteFunc = NewGlooFailTestingSuite

Split webhook validation #10284

Split webhook validation #10284

Conversation

sheidkamp commented Nov 6, 2024 • edited Loading

Description

Code changes

CI changes

Docs changes

Context

Testing steps

Checklist:

solo-changelog-bot bot commented Nov 6, 2024

github-actions bot commented Nov 6, 2024 • edited Loading

sam-heilbron left a comment

Choose a reason for hiding this comment

sam-heilbron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sheidkamp commented Nov 6, 2024 •

edited

Loading

github-actions bot commented Nov 6, 2024 •

edited

Loading