Allow multiple webhooks to be used #882

mmirecki · 2020-10-12T08:37:27Z

The PR prepares gatekeeper to be able to create multiple
webhooks. Common webhook code was extracted to a separate
file, and an update to cert rotator is used that allows
to update multiple webhooks.

Tested:
Gatekeeper with the changes was run. An incoming resource
was successfuly denied request due to gatekeeper policy.

An example of adding a new webhook with these changes can be seen in PR 881. This PR adds a mutating webhook .

codecov-io · 2020-10-23T15:38:08Z

Codecov Report

Merging #882 into master will increase coverage by 0.25%.
The diff coverage is 41.17%.

@@            Coverage Diff             @@
##           master     #882      +/-   ##
==========================================
+ Coverage   43.46%   43.71%   +0.25%     
==========================================
  Files          47       48       +1     
  Lines        3173     3175       +2     
==========================================
+ Hits         1379     1388       +9     
+ Misses       1598     1595       -3     
+ Partials      196      192       -4

Flag	Coverage Δ
unittests	`43.71% <41.17%> (+0.25%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/webhook/policy.go	`24.19% <0.00%> (-4.45%)`	⬇️
pkg/webhook/stats_reporter.go	`80.00% <ø> (ø)`
pkg/webhook/common.go	`63.63% <63.63%> (ø)`
pkg/readiness/ready_tracker.go	`69.81% <0.00%> (+0.72%)`	⬆️
...onstrainttemplate/constrainttemplate_controller.go	`56.67% <0.00%> (+0.97%)`	⬆️
pkg/readiness/object_tracker.go	`79.14% <0.00%> (+1.22%)`	⬆️
pkg/readiness/list.go	`91.30% <0.00%> (+8.69%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ca4499...d7bf67f. Read the comment docs.

maxsmythe

1 note, 1 nit :)

pkg/webhook/common.go

pkg/webhook/policy.go

mmirecki · 2020-10-28T21:56:42Z

On Wed, Oct 28, 2020 at 10:33 PM Sertaç Özercan ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In vendor/github.com/open-policy-agent/cert-controller/pkg/rotator/rotator.go <#882 (comment)> : > @@ -45,18 +45,42 @@ const ( var crLog = logf.Log.WithName("cert-rotation") -var vwhGVK = schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Version: "v1beta1", Kind: "ValidatingWebhookConfiguration"} is there a corresponding cert-controller pr for this? how do we align this?

open-policy-agent/cert-controller#11 An update to go.mod is part of the PR

…

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#882 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD3ZIKCGDOJBHIGX27EUUUDSNCE3PANCNFSM4SMP5FHA> .

pkg/webhook/common.go

maxsmythe · 2020-10-29T01:50:08Z

pkg/webhook/policy.go

@@ -243,33 +210,25 @@ func (h *validationHandler) getDenyMessages(res []*rtypes.Result, req admission.
 					logging.Process, "admission",
 					logging.EventType, "violation",
 					logging.ConstraintName, r.Constraint.GetName(),
-					logging.ConstraintGroup, r.Constraint.GroupVersionKind().Group,


Why are we removing group/version from the log lines and events?

My bad (late night commits should be forbidden)

LOL no worries, thought I missed something

It looks like group/version are still missing?

maxsmythe

1 comment about group/version

maxsmythe · 2020-10-31T02:39:46Z

pkg/webhook/policy.go

@@ -243,33 +210,25 @@ func (h *validationHandler) getDenyMessages(res []*rtypes.Result, req admission.
 					logging.Process, "admission",
 					logging.EventType, "violation",
 					logging.ConstraintName, r.Constraint.GetName(),
-					logging.ConstraintGroup, r.Constraint.GroupVersionKind().Group,


It looks like group/version are still missing?

maxsmythe

This looks like all of my comments have been addressed. Is there any other outstanding work here?

mmirecki · 2020-11-03T07:33:22Z

This looks like all of my comments have been addressed. Is there any other outstanding work here?

No, all done in this one.

maxsmythe · 2020-11-03T23:02:41Z

@ritazh @shomron @sozercan LGTY?

ritazh · 2020-11-10T06:52:27Z

PR looks great! Please resolve conflicts in go.mod.

The PR prepares gatekeeper to be able to create multiple webhooks. Common webhook code was extracted to a separate file, and an update to cert rotator is used that allows to update multiple webhooks. Tested: Gatekeeper with the changes was run. An incoming resource was successfuly denied request due to gatekeeper policy. Signed-off-by: mmirecki <mmirecki@redhat.com>

mmirecki · 2020-11-10T09:29:01Z

PR looks great! Please resolve conflicts in go.mod.

Done

The PR prepares gatekeeper to be able to create multiple webhooks. Common webhook code was extracted to a separate file, and an update to cert rotator is used that allows to update multiple webhooks. Tested: Gatekeeper with the changes was run. An incoming resource was successfuly denied request due to gatekeeper policy. Signed-off-by: mmirecki <mmirecki@redhat.com> Co-authored-by: Rita Zhang <rita.z.zhang@gmail.com> Signed-off-by: juliankatz <juliankatz@google.com>

The PR prepares gatekeeper to be able to create multiple webhooks. Common webhook code was extracted to a separate file, and an update to cert rotator is used that allows to update multiple webhooks. Tested: Gatekeeper with the changes was run. An incoming resource was successfuly denied request due to gatekeeper policy. Signed-off-by: mmirecki <mmirecki@redhat.com> Co-authored-by: Rita Zhang <rita.z.zhang@gmail.com>

The PR prepares gatekeeper to be able to create multiple webhooks. Common webhook code was extracted to a separate file, and an update to cert rotator is used that allows to update multiple webhooks. Tested: Gatekeeper with the changes was run. An incoming resource was successfuly denied request due to gatekeeper policy. Signed-off-by: mmirecki <mmirecki@redhat.com> Signed-off-by: Oren Shomron <shomron@gmail.com> Co-authored-by: Rita Zhang <rita.z.zhang@gmail.com>

#982) * Allow multiple webhooks to be created (#882) The PR prepares gatekeeper to be able to create multiple webhooks. Common webhook code was extracted to a separate file, and an update to cert rotator is used that allows to update multiple webhooks. Tested: Gatekeeper with the changes was run. An incoming resource was successfuly denied request due to gatekeeper policy. Signed-off-by: mmirecki <mmirecki@redhat.com> Signed-off-by: Oren Shomron <shomron@gmail.com> Co-authored-by: Rita Zhang <rita.z.zhang@gmail.com> * Scope secrets cache to single namespace (#972) Incorporate open-policy-agent/cert-controller#16 to avoid caching cluster-wide secrets in certificate rotation controller. This will reduce the memory consumption of Gatekeeper on clusters with a large number of secrets defined. Fixes: #831 Signed-off-by: Oren Shomron <shomron@gmail.com> Co-authored-by: Max Smythe <smythe@google.com> * Fix e2e test flakiness (#964) * Fix e2e test flakiness Signed-off-by: Max Smythe <smythe@google.com> Signed-off-by: Oren Shomron <shomron@gmail.com> Co-authored-by: Marcin Mirecki <mmirecki@redhat.com> Co-authored-by: Rita Zhang <rita.z.zhang@gmail.com> Co-authored-by: Max Smythe <smythe@google.com>

mmirecki force-pushed the multipleWebhooks branch 2 times, most recently from 95992fc to 14bd93c Compare October 22, 2020 10:37

mmirecki changed the title ~~WIP: Allow cert rotator to work on multiple webhooks~~ Allow multiple webhooks to be used Oct 22, 2020

mmirecki mentioned this pull request Oct 22, 2020

Allow cert rotator to work on multiple webhooks open-policy-agent/cert-controller#11

Merged

mmirecki force-pushed the multipleWebhooks branch 2 times, most recently from a13e671 to 788ce4e Compare October 23, 2020 15:35

mmirecki force-pushed the multipleWebhooks branch from 788ce4e to 2b667a0 Compare October 23, 2020 17:59

maxsmythe requested changes Oct 28, 2020

View reviewed changes

pkg/webhook/common.go Show resolved Hide resolved

pkg/webhook/policy.go Outdated Show resolved Hide resolved

mmirecki force-pushed the multipleWebhooks branch 4 times, most recently from 87233fc to 3674cb7 Compare October 28, 2020 20:53

mmirecki force-pushed the multipleWebhooks branch 2 times, most recently from 10a9dc4 to e1d240e Compare October 28, 2020 22:10

maxsmythe reviewed Oct 29, 2020

View reviewed changes

mmirecki force-pushed the multipleWebhooks branch 3 times, most recently from 0000426 to 572dd3b Compare October 29, 2020 08:46

mmirecki requested a review from maxsmythe October 30, 2020 08:29

maxsmythe requested changes Oct 31, 2020

View reviewed changes

mmirecki force-pushed the multipleWebhooks branch 2 times, most recently from cb67b23 to d7bf67f Compare November 2, 2020 12:50

maxsmythe approved these changes Nov 3, 2020

View reviewed changes

mmirecki force-pushed the multipleWebhooks branch from d7bf67f to 8886edb Compare November 10, 2020 09:04

Merge branch 'master' into multipleWebhooks

5d05d05

ritazh approved these changes Nov 11, 2020

View reviewed changes

ritazh merged commit f3ec64a into open-policy-agent:master Nov 11, 2020

mmirecki deleted the multipleWebhooks branch November 16, 2020 11:49

shomron mentioned this pull request Nov 25, 2020

Cherry pick "Scope secrets cache to single namespace" (#972) to v3.2.x #982

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow multiple webhooks to be used #882

Allow multiple webhooks to be used #882

mmirecki commented Oct 12, 2020 •

edited

codecov-io commented Oct 23, 2020 •

edited

maxsmythe left a comment

mmirecki commented Oct 28, 2020 via email

maxsmythe Oct 29, 2020

mmirecki Oct 29, 2020

maxsmythe Oct 30, 2020

maxsmythe Oct 31, 2020

maxsmythe left a comment

maxsmythe Oct 31, 2020

maxsmythe left a comment

mmirecki commented Nov 3, 2020

maxsmythe commented Nov 3, 2020

ritazh commented Nov 10, 2020

mmirecki commented Nov 10, 2020

Allow multiple webhooks to be used #882

Allow multiple webhooks to be used #882

Conversation

mmirecki commented Oct 12, 2020 • edited

codecov-io commented Oct 23, 2020 • edited

Codecov Report

maxsmythe left a comment

Choose a reason for hiding this comment

mmirecki commented Oct 28, 2020 via email

maxsmythe Oct 29, 2020

Choose a reason for hiding this comment

mmirecki Oct 29, 2020

Choose a reason for hiding this comment

maxsmythe Oct 30, 2020

Choose a reason for hiding this comment

maxsmythe Oct 31, 2020

Choose a reason for hiding this comment

maxsmythe left a comment

Choose a reason for hiding this comment

maxsmythe Oct 31, 2020

Choose a reason for hiding this comment

maxsmythe left a comment

Choose a reason for hiding this comment

mmirecki commented Nov 3, 2020

maxsmythe commented Nov 3, 2020

ritazh commented Nov 10, 2020

mmirecki commented Nov 10, 2020

mmirecki commented Oct 12, 2020 •

edited

codecov-io commented Oct 23, 2020 •

edited