add a deletion approval flow with a validation webhook #3678

natasha41575 · 2022-11-30T21:35:31Z

Addresses the second part of #3264

This adds a deletion approval process via a self-signing webhook started up in the porch-server pod, exposed on port 8443. It stores the deletionProposed field as a separate branch in git.

This works on GKE and running in kind. But it doesn't seem to work if you try to run the webhook locally, so it's gated by an env var (meaning it will be skipped when you run make run-local).

This PR also only covers the git use-case, and leaves OCI implementation as a TODO.

porch/pkg/apiserver/webhooks.go

porch/api/porch/v1alpha1/types_packagerevisions.go

natasha41575 · 2022-12-13T22:32:45Z

e2e/testdata/porch/rpkg-lifecycle/config.yaml

-      - --namespace=rpkg-lifecycle
-    stderr: "Error: the server could not find the requested resource (get packagerevisions.porch.kpt.dev git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034) \n"
-    exitCode: 1
+      git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034 failed (admission webhook "packagerevdeletion.google.com" denied the request: failed to delete package revision "git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034")


I plan to make a close followup PR to update the CLI respectively.

porch/api/porch/v1alpha1/types_packagerevisions.go

porch/pkg/engine/engine.go

porch/pkg/registry/porch/strategy.go

porch/test/e2e/e2e_test.go

porch/webhooks/Dockerfile

porch/webhooks/main.go

porch/internal/api/porchinternal/v1alpha1/types.go

natasha41575 · 2023-01-10T02:26:11Z

@mortent ready for review! As a note, I left a couple of TODOs in the oci implementation because my impression was that that is not super important right now.

mortent

Mostly looks good. Just a few questions and clarifications.

porch/pkg/git/git.go

porch/pkg/git/annotation.go

porch/pkg/git/git.go

porch/pkg/git/package.go

porch/pkg/registry/porch/strategy.go

porch/pkg/cache/repository.go

* porch: don't save empty patches (#3695) * docs: fixes for some minor documentation typos (#3699) * docs: Update the kpt book with more details about namespaces and RBAC for porch (#3692) * Log enabled controllers and warn if no controllers are enabled (#3710) Because the default is to enable no controllers, it is easy to mistakenly start a no-op controller. * Extract out common parse-package logic (#3711) We had this code duplicated in a few places also. * refactor pod warmup to avoid vet warning (#3713) By refactoring the parallel operation into a separate function, it should be easier to read and we avoid a loop-closure go-tcha. * Bump json5 from 2.2.0 to 2.2.3 in /site (#3717) Bumps [json5](https://github.com/json5/json5) from 2.2.0 to 2.2.3. - [Release notes](https://github.com/json5/json5/releases) - [Changelog](https://github.com/json5/json5/blob/main/CHANGELOG.md) - [Commits](json5/json5@v2.2.0...v2.2.3) --- updated-dependencies: - dependency-name: json5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * tests: add more logging around problematic test timeout (#3718) Trying to figure out why this test keeps timing out. * Refactor controller logic for getting RESTConfig to a remote cluster (#3712) We had two copies, rationalize and take the best of each. Also remove the HACK_ENABLE_LOOPBACK hack now that we can target remote clusters. * add a deletion approval flow with a validation webhook (#3678) * PackageVariant controller: implement pruning, deletionPolicy, and adoptionPolicy (#3701) * e2e: add delay after registering Repository (#3741) I believe this will help avoid the "failed to list resources" error immediately after registering a repository. * licensescan: fix ignore handling (#3740) The previous logic did not work correctly. * licensescan: Add licenses for more libraries. (#3736) Updating our database with the latest libraries, as needed by some other projects sharing this DB! * Docs: Updating 'Developing in Go' (#3715) * licensescan: Fix missing pipe character in README (#3739) The command is not correct without it. * RemoteRootSyncSet: able to specify a packageRef to a package (#3734) This makes it easy to apply packages we create. * chore: Upgrade cli-utils to v0.34.0 (#3746) Upgrades cli-utils to v0.34.0 which contains an upgrade to Go v1.18 and Kubernetes v1.25 resources. This PR was origininally authored by rquitales #3642 * rollouts: added top level directory * rollouts: scaffolded the project using kubebuilder (#3689) * rollouts: added cluster discovery and selection (#3696) * Rollouts package discovery (#3697) * rollouts: added remoterootsync API (#3698) * rollouts: add package cluster matcher (#3700) * rollouts: add AllAtOnce strategy (#3703) * rollouts: allow packages to be discovered from multiple repositories (#3702) * rollouts: rename packages git source to github (#3708) * rollouts: allow the root directory of a repository to be synced (#3709) * rollouts: add caching for discovered packages (#3706) * rollouts: add rolling update strategy (#3714) * rollouts: added API for ProgressiveRolloutStrategy (#3716) * rollouts: refine package to cluster matcher (#3720) * rollouts: implement progressive strategy (#3719) * rollouts: update progressive strategy to pause after wave (#3721) * rollouts: added skeleton CLI (#3724) * rollouts: added skeleton CLI * added table display * rollouts: add rollout summary status (#3725) * rollouts: tidy up go.mod/sum (#3726) * rollouts: duplicate target fix (#3727) * rollouts: sort cluster status list (#3728) * rollouts: conditionally show wave status (#3729) * rollouts: CLI now supports displaying waves and progress counts (#3730) * rollouts: cli can now advance waves on progressive rollouts (#3731) * rollouts: enable server side throttling for cli (#3732) * rollouts: add container cluster watch (#3738) * rollouts: delete remote root sync when no longer needed (#3742) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Natasha Sarkar <[email protected]> Co-authored-by: James Brook <[email protected]> Co-authored-by: Morten Torkildsen <[email protected]> Co-authored-by: Justin Santa Barbara <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: abangser <[email protected]> Co-authored-by: Christopher Fry <[email protected]>

* porch: don't save empty patches (kptdev#3695) * docs: fixes for some minor documentation typos (kptdev#3699) * docs: Update the kpt book with more details about namespaces and RBAC for porch (kptdev#3692) * Log enabled controllers and warn if no controllers are enabled (kptdev#3710) Because the default is to enable no controllers, it is easy to mistakenly start a no-op controller. * Extract out common parse-package logic (kptdev#3711) We had this code duplicated in a few places also. * refactor pod warmup to avoid vet warning (kptdev#3713) By refactoring the parallel operation into a separate function, it should be easier to read and we avoid a loop-closure go-tcha. * Bump json5 from 2.2.0 to 2.2.3 in /site (kptdev#3717) Bumps [json5](https://github.com/json5/json5) from 2.2.0 to 2.2.3. - [Release notes](https://github.com/json5/json5/releases) - [Changelog](https://github.com/json5/json5/blob/main/CHANGELOG.md) - [Commits](json5/json5@v2.2.0...v2.2.3) --- updated-dependencies: - dependency-name: json5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * tests: add more logging around problematic test timeout (kptdev#3718) Trying to figure out why this test keeps timing out. * Refactor controller logic for getting RESTConfig to a remote cluster (kptdev#3712) We had two copies, rationalize and take the best of each. Also remove the HACK_ENABLE_LOOPBACK hack now that we can target remote clusters. * add a deletion approval flow with a validation webhook (kptdev#3678) * PackageVariant controller: implement pruning, deletionPolicy, and adoptionPolicy (kptdev#3701) * e2e: add delay after registering Repository (kptdev#3741) I believe this will help avoid the "failed to list resources" error immediately after registering a repository. * licensescan: fix ignore handling (kptdev#3740) The previous logic did not work correctly. * licensescan: Add licenses for more libraries. (kptdev#3736) Updating our database with the latest libraries, as needed by some other projects sharing this DB! * Docs: Updating 'Developing in Go' (kptdev#3715) * licensescan: Fix missing pipe character in README (kptdev#3739) The command is not correct without it. * RemoteRootSyncSet: able to specify a packageRef to a package (kptdev#3734) This makes it easy to apply packages we create. * chore: Upgrade cli-utils to v0.34.0 (kptdev#3746) Upgrades cli-utils to v0.34.0 which contains an upgrade to Go v1.18 and Kubernetes v1.25 resources. This PR was origininally authored by rquitales kptdev#3642 * rollouts: added top level directory * rollouts: scaffolded the project using kubebuilder (kptdev#3689) * rollouts: added cluster discovery and selection (kptdev#3696) * Rollouts package discovery (kptdev#3697) * rollouts: added remoterootsync API (kptdev#3698) * rollouts: add package cluster matcher (kptdev#3700) * rollouts: add AllAtOnce strategy (kptdev#3703) * rollouts: allow packages to be discovered from multiple repositories (kptdev#3702) * rollouts: rename packages git source to github (kptdev#3708) * rollouts: allow the root directory of a repository to be synced (kptdev#3709) * rollouts: add caching for discovered packages (kptdev#3706) * rollouts: add rolling update strategy (kptdev#3714) * rollouts: added API for ProgressiveRolloutStrategy (kptdev#3716) * rollouts: refine package to cluster matcher (kptdev#3720) * rollouts: implement progressive strategy (kptdev#3719) * rollouts: update progressive strategy to pause after wave (kptdev#3721) * rollouts: added skeleton CLI (kptdev#3724) * rollouts: added skeleton CLI * added table display * rollouts: add rollout summary status (kptdev#3725) * rollouts: tidy up go.mod/sum (kptdev#3726) * rollouts: duplicate target fix (kptdev#3727) * rollouts: sort cluster status list (kptdev#3728) * rollouts: conditionally show wave status (kptdev#3729) * rollouts: CLI now supports displaying waves and progress counts (kptdev#3730) * rollouts: cli can now advance waves on progressive rollouts (kptdev#3731) * rollouts: enable server side throttling for cli (kptdev#3732) * rollouts: add container cluster watch (kptdev#3738) * rollouts: delete remote root sync when no longer needed (kptdev#3742) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Natasha Sarkar <[email protected]> Co-authored-by: James Brook <[email protected]> Co-authored-by: Morten Torkildsen <[email protected]> Co-authored-by: Justin Santa Barbara <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: abangser <[email protected]> Co-authored-by: Christopher Fry <[email protected]>

mortent reviewed Dec 2, 2022

View reviewed changes

porch/pkg/apiserver/webhooks.go Outdated Show resolved Hide resolved

natasha41575 force-pushed the deletionvalidationwebhook branch 4 times, most recently from 264445a to ff23928 Compare December 10, 2022 00:14

natasha41575 commented Dec 13, 2022

View reviewed changes

porch/api/porch/v1alpha1/types_packagerevisions.go Outdated Show resolved Hide resolved

natasha41575 force-pushed the deletionvalidationwebhook branch 6 times, most recently from 507b88b to 113af5e Compare December 13, 2022 22:09

natasha41575 changed the title ~~WIP: webhook for deletion validation~~ add a deletion approval flow with a validation webhook Dec 13, 2022

natasha41575 marked this pull request as ready for review December 13, 2022 22:12

natasha41575 requested review from mortent and justinsb December 13, 2022 22:13

natasha41575 force-pushed the deletionvalidationwebhook branch 2 times, most recently from b5918c4 to 05ad299 Compare December 13, 2022 22:31

natasha41575 commented Dec 13, 2022

View reviewed changes

natasha41575 force-pushed the deletionvalidationwebhook branch from 20b1eab to bcd815d Compare December 14, 2022 17:43

natasha41575 requested a review from droot December 14, 2022 17:47

natasha41575 force-pushed the deletionvalidationwebhook branch 2 times, most recently from 49cecde to 6c53440 Compare December 14, 2022 19:21

mortent reviewed Dec 14, 2022

View reviewed changes

natasha41575 force-pushed the deletionvalidationwebhook branch 6 times, most recently from db9f9bb to b0e5e4a Compare December 20, 2022 22:45

natasha41575 requested a review from mortent December 21, 2022 23:06

natasha41575 force-pushed the deletionvalidationwebhook branch 4 times, most recently from 9ee20b5 to f738738 Compare December 22, 2022 18:41

natasha41575 mentioned this pull request Dec 22, 2022

PackageVariant controller: implement pruning, deletionPolicy, and adoptionPolicy #3701

Merged

natasha41575 mentioned this pull request Dec 29, 2022

Add PackageVariantSet controller #3707

Merged

mortent reviewed Jan 1, 2023

View reviewed changes

porch/internal/api/porchinternal/v1alpha1/types.go Outdated Show resolved Hide resolved

natasha41575 force-pushed the deletionvalidationwebhook branch 3 times, most recently from 2ecb5bd to 08789f8 Compare January 10, 2023 01:39

natasha41575 requested a review from mortent January 10, 2023 01:40

add webhook for deletion validation

67eb0ee

natasha41575 force-pushed the deletionvalidationwebhook branch from 08789f8 to 67eb0ee Compare January 10, 2023 02:29

mortent reviewed Jan 10, 2023

View reviewed changes

address review comments

abfc76e

natasha41575 requested a review from mortent January 13, 2023 21:57

mortent approved these changes Jan 14, 2023

View reviewed changes

porch/pkg/cache/repository.go Show resolved Hide resolved

add todo for cache layer knowing about repo type

7eeb378

natasha41575 merged commit bb2725f into kptdev:main Jan 17, 2023

natasha41575 deleted the deletionvalidationwebhook branch January 17, 2023 17:56

natasha41575 mentioned this pull request Jan 23, 2023

add command for deletion proposal of porch package revisions #3745

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add a deletion approval flow with a validation webhook #3678

add a deletion approval flow with a validation webhook #3678

natasha41575 commented Nov 30, 2022 •

edited

Loading

natasha41575 Dec 13, 2022

natasha41575 commented Jan 10, 2023

mortent left a comment

add a deletion approval flow with a validation webhook #3678

add a deletion approval flow with a validation webhook #3678

Conversation

natasha41575 commented Nov 30, 2022 • edited Loading

natasha41575 Dec 13, 2022

Choose a reason for hiding this comment

natasha41575 commented Jan 10, 2023

mortent left a comment

Choose a reason for hiding this comment

natasha41575 commented Nov 30, 2022 •

edited

Loading