-
Notifications
You must be signed in to change notification settings - Fork 50
Add AWS validator for core resources #438
Add AWS validator for core resources #438
Conversation
...ollers/provider-aws/charts/validator-aws/templates/validatingwebhook-controller-manager.yaml
Outdated
Show resolved
Hide resolved
controllers/provider-aws/charts/validator-aws/templates/deployment.yaml
Outdated
Show resolved
Hide resolved
controllers/provider-aws/charts/validator-aws/templates/deployment.yaml
Outdated
Show resolved
Hide resolved
fe1b66d
to
b333930
Compare
Please do not merge since I'm also changing the |
The validation logic which allows to add availability zones has been added. I dropped the restriction that the zone order must be retained because in the new API group you cannot mix the networks and the names anymore. Furthermore I noticed that in the old API it's possible to configure the exact same zone multiple times with different networks which doesn't make any sense, right @zanetworker @rfranzke ? |
d0ef532
to
81d31ba
Compare
81d31ba
to
9c306fa
Compare
9c306fa
to
ee3beb0
Compare
@rfranzke, @zanetworker, @tim-ebert thanks for your reviews. I addressed your comments, PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good :) just a couple of small comments.
ee3beb0
to
707d01b
Compare
.../charts/validator-aws/charts/application/templates/validatingwebhook-controller-manager.yaml
Outdated
Show resolved
Hide resolved
.../charts/validator-aws/charts/application/templates/validatingwebhook-controller-manager.yaml
Outdated
Show resolved
Hide resolved
Is it delegated to g/garden-setup to install the validator charts? |
The responsible party that creates the |
controllers/provider-aws/cmd/gardener-validator-aws/app/options.go
Outdated
Show resolved
Hide resolved
707d01b
to
869f313
Compare
@rfranzke could you please have another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm (just a few minor suggestions :))
controllers/provider-aws/charts/validator-aws/charts/runtime/templates/deployment.yaml
Outdated
Show resolved
Hide resolved
controllers/provider-aws/charts/validator-aws/templates/_helpers.tpl
Outdated
Show resolved
Hide resolved
controllers/provider-aws/cmd/gardener-extension-validator-aws/app/app.go
Outdated
Show resolved
Hide resolved
869f313
to
1975c6c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm, nice work!
What this PR does / why we need it:
This PR adds a validating webhook that is responsible for checking resources of the group
core.gardener.cloud
(at the moment onlyshoot
resources).With GEP-4 cloud specific information was externalized and as a consequence cannot be validated by the Gardener anymore. On the other hand, it should be prevented that invalid
shoot
objects are created or updated.Example:
The above example is supposed to be denied because a worker group refers to an availability zone (eu-west-1c) which is not part of the InfrastructureConfig.
Since the
gardener-extensions
repository is already a home for cloud specific logic, this PR adds cloud specific validations for core resources to this repo.Part of #407
Special notes for your reviewer:
Validations for other cloud providers will follow.
Release note: