Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: use hooks as a validating webhook handlers #223

Merged
merged 19 commits into from
Jan 12, 2021
Merged

feat: use hooks as a validating webhook handlers #223

merged 19 commits into from
Jan 12, 2021

Conversation

diafour
Copy link
Contributor

@diafour diafour commented Dec 9, 2020

Fix #216

@diafour diafour changed the title feat: use hooks as a validating webhook handlers WIP: feat: use hooks as a validating webhook handlers Dec 9, 2020
@diafour diafour requested review from zuzzas and nabokihms December 9, 2020 10:19
zuzzas
zuzzas reviewed Dec 9, 2020
View reviewed changes
Copy link
Contributor

@zuzzas zuzzas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update my review once more things come up.

VALIDATING.md Outdated Show resolved Hide resolved
VALIDATING.md Outdated Show resolved Hide resolved
diafour
diafour commented Dec 9, 2020
View reviewed changes
VALIDATING.md Outdated Show resolved Hide resolved
nabokihms
nabokihms previously requested changes Dec 10, 2020
View reviewed changes
VALIDATING.md Outdated Show resolved Hide resolved
@diafour diafour force-pushed the feat_validating_webhook branch from a07cd76 to 0f9c327 Compare December 23, 2020 20:09
@diafour diafour requested review from zuzzas and nabokihms December 23, 2020 20:09
zuzzas
zuzzas suggested changes Dec 24, 2020
View reviewed changes
BINDING_VALIDATING.md Outdated Show resolved Hide resolved
BINDING_VALIDATING.md Outdated Show resolved Hide resolved
BINDING_VALIDATING.md Show resolved Hide resolved
diafour and others added 4 commits December 24, 2020 12:27
@diafour @zuzzas
spell check
34d2ab0 
Co-authored-by: Andrey Klimentyev <andrey.klimentyev@flant.com>
@diafour
fixes for ci
54d2e21
@diafour
docs: group parameter
2087509
@diafour
++--
7745bc2
@diafour diafour added the publish/image/dev Build and push dev image using PR number as docker tag label Dec 24, 2020
@github-actions github-actions bot removed the publish/image/dev Build and push dev image using PR number as docker tag label Dec 24, 2020
nabokihms
nabokihms reviewed Dec 28, 2020
View reviewed changes
BINDING_VALIDATING.md
- `timeoutSeconds` — a seconds API server should wait for a hook to respond before treating the call as a failure. See [timeouts](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#timeouts). Default is 10 (seconds).

As you can see, it is the close copy of a [Webhook configuration](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#webhook-configuration). Differences are:
- `objectSelector` is a `labelSelector` as in the `kubernetes` binding.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so important, but this works only in Kubernetes 1.15+

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is even 1.16+ (https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#prerequisites). Add a note about required version: https://github.com/flant/shell-operator/pull/223/files#diff-d1081f0b2fcf5eccb9e4cd1e5a83e24fcdf5921db0742bc5a2d52ae160fa89daR5

@nabokihms nabokihms dismissed their stale review December 28, 2020 10:22

everything is ok now

@diafour diafour added the publish/image/dev Build and push dev image using PR number as docker tag label Jan 11, 2021
@github-actions github-actions bot removed the publish/image/dev Build and push dev image using PR number as docker tag label Jan 11, 2021
@diafour diafour added the publish/image/dev/amd64 Build and push dev image using PR number as docker tag label Jan 11, 2021
@diafour diafour added publish/image/dev/amd64 Build and push dev image using PR number as docker tag and removed publish/image/dev/amd64 Build and push dev image using PR number as docker tag labels Jan 11, 2021
@github-actions github-actions bot removed the publish/image/dev/amd64 Build and push dev image using PR number as docker tag label Jan 11, 2021
@diafour diafour added the publish/image/dev/amd64 Build and push dev image using PR number as docker tag label Jan 11, 2021
@github-actions github-actions bot removed the publish/image/dev/amd64 Build and push dev image using PR number as docker tag label Jan 11, 2021
@diafour diafour changed the title WIP: feat: use hooks as a validating webhook handlers feat: use hooks as a validating webhook handlers Jan 12, 2021
@diafour
Copy link
Contributor Author

diafour commented Jan 12, 2021

This PR was successfully tested with hooks from addon-operator installation by @apolovov. I think it is ready to merge.

@diafour diafour merged commit 51b75bf into master Jan 12, 2021
@diafour diafour added this to the 1.0.0-rc.1 milestone Jan 12, 2021
@nabokihms nabokihms mentioned this pull request Feb 24, 2021
Open
@diafour diafour deleted the feat_validating_webhook branch April 13, 2021 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nabokihms nabokihms nabokihms left review comments

@zuzzas zuzzas zuzzas requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0.0-rc.1
Development

Successfully merging this pull request may close these issues.

New binding type for validation
3 participants
@diafour @zuzzas @nabokihms