Add validation for cloud provider Secrets #112

ialidzhikov · 2020-06-09T15:10:07Z

How to categorize this PR?

/area robustness
/area ops-productivity
/kind enhancement
/priority normal
/platform gcp

What this PR does / why we need it:
Add validation for cloud provider Secrets.

Which issue(s) this PR fixes:
Ref gardener/gardener#2293

Special notes for your reviewer:

Release note:

The existing ValidatingWebhookConfiguration of `admission-gcp` for Shoot validation does now validate also the Shoot secret. `admission-gcp` does now feature also a new webhook that prevents Shoot secret to be updated with invalid keys.

gardener-robot · 2020-06-09T15:10:15Z

@ialidzhikov Labels area/todo, kind/todo do not exist.

ialidzhikov · 2020-06-22T13:47:54Z

Something interesting that I figure out today trying to use client.MatchingFields/client.MatchingFieldsSelector with the controller-runtime client.
When the client is a non-cached one, the above options are handled as field selector sent to the API server.
When the client is a cached one, the above options work on index and you have to explicitly add the index fields. For example:

			if err := mgr.GetFieldIndexer().IndexField(&gardencorev1beta1.Shoot{}, "spec.seedName", func(rawObj runtime.Object) []string {
				shoot := rawObj.(*gardencorev1beta1.Shoot)
				if shoot.Spec.SeedName == nil {
					return nil
				}

				return []string{*shoot.Spec.SeedName}
			}); err != nil {
				return err
			}

Otherwise the calls always fail with

Index with name field:spec.seedName does not exist

Ahh, that was quite unintuitive for me, as I added the required changes for gardener-apiserver for new field-selectors, kubectl and a non-cached client were working well, but I was wondering why the cached client continues to return index does not exist.

timuthy · 2020-06-23T13:48:11Z

/assign

vpnachev

Thanks, looks very nice!

I have mostly comments and several change requests.

...rdener-extension-admission-gcp/charts/application/templates/validatingwebhook-validator.yaml

docs/local-setup.md

example/40-validatingwebhookconfiguration.yaml

pkg/admission/validator/secret.go

rfranzke

Nice PR!

pkg/admission/cache/informer_cache.go

pkg/apis/gcp/validation/secret.go

timuthy

Nice PR, I really like it. Just a few comments and questions.

docs/local-setup.md

example/40-validatingwebhookconfiguration.yaml

pkg/admission/validator/secret.go

pkg/admission/validator/shoot.go

pkg/admission/validator/webhook.go

pkg/admission/validator/secret.go

ialidzhikov · 2020-06-26T21:08:23Z

pkg/admission/cache/informer_cache.go

+// SecretRefNamespaceField is the field name for the index function that extracts the corresponding field from SecretBinding.
+const SecretRefNamespaceField string = "secretRef.namespace"
+
+func SecretRefNamespaceIndexerFunc(rawObj runtime.Object) []string {


These funcs are not admission-gcp specific, they would also need to go under g/g extensions pkgs to make them usable also from other provider extensions.

pkg/admission/validator/secret.go

timuthy

Nice. Can you update the g/g dependency to include gardener/gardener#2537 and adapt this PR to it?

docs/local-setup.md

Signed-off-by: ialidzhikov <i.alidjikov@gmail.com>

ialidzhikov · 2020-07-10T16:36:28Z

Nice. Can you update the g/g dependency to include gardener/gardener#2537 and adapt this PR to it?

Sorry for the long delay, now I updated the PR.

vpnachev

/lgtm
/reviewed ok-to-test

sorry, wrong tab

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 9, 2020

gardener-robot added platform/gcp Google cloud platform/infrastructure priority/normal labels Jun 9, 2020

gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jun 9, 2020

ialidzhikov mentioned this pull request Jun 22, 2020

Use cached controller-runtime client gardener/gardener#2449

Merged

ialidzhikov force-pushed the enh/credentials-validation branch from 3839ae7 to 823d93b Compare June 22, 2020 20:29

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 22, 2020

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 22, 2020

ialidzhikov force-pushed the enh/credentials-validation branch from 823d93b to c4fe9e7 Compare June 23, 2020 07:36

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 23, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 23, 2020

gardener-robot added area/ops-productivity Operator productivity related (how to improve operations) area/robustness Robustness, reliability, resilience related kind/enhancement Enhancement, improvement, extension labels Jun 23, 2020

ialidzhikov marked this pull request as ready for review June 23, 2020 07:47

ialidzhikov requested review from a team as code owners June 23, 2020 07:47

gardener-robot assigned timuthy Jun 23, 2020

vpnachev suggested changes Jun 23, 2020

View reviewed changes

rfranzke reviewed Jun 24, 2020

View reviewed changes

pkg/admission/cache/informer_cache.go Outdated Show resolved Hide resolved

pkg/admission/cache/informer_cache.go Outdated Show resolved Hide resolved

pkg/apis/gcp/validation/secret.go Show resolved Hide resolved

timuthy requested changes Jun 24, 2020

View reviewed changes

timuthy reviewed Jun 24, 2020

View reviewed changes

pkg/admission/validator/secret.go Outdated Show resolved Hide resolved

ialidzhikov force-pushed the enh/credentials-validation branch from c4fe9e7 to 66450e4 Compare June 26, 2020 21:19

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 26, 2020

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 26, 2020

ialidzhikov commented Jun 26, 2020

View reviewed changes

ialidzhikov force-pushed the enh/credentials-validation branch from 327e589 to 245f296 Compare June 30, 2020 14:05

gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 30, 2020

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 30, 2020

ialidzhikov mentioned this pull request Jul 1, 2020

Add common funcs to the extensions library gardener/gardener#2537

Merged

timuthy reviewed Jul 7, 2020

View reviewed changes

docs/local-setup.md Outdated Show resolved Hide resolved

Add validation for cloud provider Secrets

800ee3e

Signed-off-by: ialidzhikov <i.alidjikov@gmail.com>

ialidzhikov force-pushed the enh/credentials-validation branch from 245f296 to 800ee3e Compare July 10, 2020 16:18

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jul 10, 2020

ialidzhikov requested review from vpnachev, timuthy and rfranzke July 10, 2020 16:36

vpnachev previously approved these changes Jul 13, 2020

View reviewed changes

gardener-robot added reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jul 13, 2020

timuthy approved these changes Jul 13, 2020

View reviewed changes

timuthy merged commit f560921 into gardener:master Jul 13, 2020

ialidzhikov mentioned this pull request Jul 14, 2020

Fix whitespace issue in webhook manifest #130

Merged

ialidzhikov deleted the enh/credentials-validation branch July 25, 2020 17:23

ialidzhikov mentioned this pull request Jul 25, 2020

Add VPA for admission-gcp deployment #141

Closed

timuthy mentioned this pull request Jul 28, 2020

Reduce memory footprint for GCP admission component #143

Closed

gardener-robot added the priority/3 Priority (lower number equals higher priority) label Mar 8, 2021

ialidzhikov mentioned this pull request Apr 7, 2021

Use client.Reader to prevent starting Informer for Secrets #253

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add validation for cloud provider Secrets #112

Add validation for cloud provider Secrets #112

ialidzhikov commented Jun 9, 2020 •

edited

Loading

gardener-robot commented Jun 9, 2020

ialidzhikov commented Jun 22, 2020

timuthy commented Jun 23, 2020

vpnachev left a comment

rfranzke left a comment

timuthy left a comment

ialidzhikov Jun 26, 2020

timuthy left a comment

ialidzhikov commented Jul 10, 2020

vpnachev left a comment

Add validation for cloud provider Secrets #112

Add validation for cloud provider Secrets #112

Conversation

ialidzhikov commented Jun 9, 2020 • edited Loading

gardener-robot commented Jun 9, 2020

ialidzhikov commented Jun 22, 2020

timuthy commented Jun 23, 2020

vpnachev left a comment

Choose a reason for hiding this comment

rfranzke left a comment

Choose a reason for hiding this comment

timuthy left a comment

Choose a reason for hiding this comment

ialidzhikov Jun 26, 2020

Choose a reason for hiding this comment

timuthy left a comment

Choose a reason for hiding this comment

ialidzhikov commented Jul 10, 2020

vpnachev left a comment

Choose a reason for hiding this comment

ialidzhikov commented Jun 9, 2020 •

edited

Loading