Skip to content

Commit

Permalink
add a deletion approval flow with a validation webhook (#3678)
Browse files Browse the repository at this point in the history
  • Loading branch information
natasha41575 committed Jan 17, 2023
1 parent 9b05920 commit bb2725f
Show file tree
Hide file tree
Showing 37 changed files with 883 additions and 71 deletions.
2 changes: 1 addition & 1 deletion commands/alpha/repo/reg/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ type runner struct {

func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
const op errors.Op = command + ".preRunE"
client, err := porch.CreateClient(r.cfg)
client, err := porch.CreateClientWithFlags(r.cfg)
if err != nil {
return errors.E(op, err)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/alpha/repo/unreg/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type runner struct {

func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
const op errors.Op = command + ".preRunE"
client, err := porch.CreateClient(r.cfg)
client, err := porch.CreateClientWithFlags(r.cfg)
if err != nil {
return errors.E(op, err)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/alpha/rpkg/clone/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ type runner struct {

func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
const op errors.Op = command + ".preRunE"
client, err := porch.CreateClient(r.cfg)
client, err := porch.CreateClientWithFlags(r.cfg)
if err != nil {
return errors.E(op, err)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/alpha/rpkg/copy/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type runner struct {

func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
const op errors.Op = command + ".preRunE"
client, err := porch.CreateClient(r.cfg)
client, err := porch.CreateClientWithFlags(r.cfg)
if err != nil {
return errors.E(op, err)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/alpha/rpkg/del/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type runner struct {
func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
const op errors.Op = command + ".preRunE"

client, err := porch.CreateClient(r.cfg)
client, err := porch.CreateClientWithFlags(r.cfg)
if err != nil {
return errors.E(op, err)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/alpha/rpkg/init/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ type runner struct {
func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
const op errors.Op = command + ".preRunE"

client, err := porch.CreateClient(r.cfg)
client, err := porch.CreateClientWithFlags(r.cfg)
if err != nil {
return errors.E(op, err)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/alpha/rpkg/propose/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type runner struct {
func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
const op errors.Op = command + ".preRunE"

client, err := porch.CreateClient(r.cfg)
client, err := porch.CreateClientWithFlags(r.cfg)
if err != nil {
return errors.E(op, err)
}
Expand Down
2 changes: 1 addition & 1 deletion commands/alpha/rpkg/update/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ type runner struct {

func (r *runner) preRunE(cmd *cobra.Command, args []string) error {
const op errors.Op = command + ".preRunE"
c, err := porch.CreateClient(r.cfg)
c, err := porch.CreateClientWithFlags(r.cfg)
if err != nil {
return errors.E(op, err)
}
Expand Down
13 changes: 4 additions & 9 deletions e2e/testdata/porch/rpkg-lifecycle/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,8 @@ commands:
- delete
- git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034
- --namespace=rpkg-lifecycle
stderr: |
git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034 deleted
- args:
- alpha
- rpkg
- get
- git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034
- --namespace=rpkg-lifecycle
stderr: "Error: the server could not find the requested resource (get packagerevisions.porch.kpt.dev git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034) \n"
exitCode: 1
stderr: |
git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034 failed (admission webhook "packagerevdeletion.google.com" denied the request: failed to delete package revision "git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034": published PackageRevisions must be proposed for deletion by setting spec.lifecycle to 'DeletionProposed' prior to deletion)
Error: errors:
admission webhook "packagerevdeletion.google.com" denied the request: failed to delete package revision "git-017a8366a5e0d9b35ae6dc489d4d3f68046d6034": published PackageRevisions must be proposed for deletion by setting spec.lifecycle to 'DeletionProposed' prior to deletion
16 changes: 10 additions & 6 deletions internal/util/porch/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

func CreateClient(flags *genericclioptions.ConfigFlags) (client.Client, error) {
config, err := flags.ToRESTConfig()
if err != nil {
return nil, err
}

func CreateClient(config *rest.Config) (client.Client, error) {
scheme, err := createScheme()
if err != nil {
return nil, err
Expand All @@ -52,6 +47,15 @@ func CreateClient(flags *genericclioptions.ConfigFlags) (client.Client, error) {
return c, nil
}

func CreateClientWithFlags(flags *genericclioptions.ConfigFlags) (client.Client, error) {
config, err := flags.ToRESTConfig()
if err != nil {
return nil, err
}

return CreateClient(config)
}

func CreateDynamicClient(flags *genericclioptions.ConfigFlags) (client.WithWatch, error) {
config, err := flags.ToRESTConfig()
if err != nil {
Expand Down
7 changes: 4 additions & 3 deletions porch/api/porch/types_packagerevisions.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ type PackageRevisionList struct {
type PackageRevisionLifecycle string

const (
PackageRevisionLifecycleDraft PackageRevisionLifecycle = "Draft"
PackageRevisionLifecycleProposed PackageRevisionLifecycle = "Proposed"
PackageRevisionLifecyclePublished PackageRevisionLifecycle = "Published"
PackageRevisionLifecycleDraft PackageRevisionLifecycle = "Draft"
PackageRevisionLifecycleProposed PackageRevisionLifecycle = "Proposed"
PackageRevisionLifecyclePublished PackageRevisionLifecycle = "Published"
PackageRevisionLifecycleDeletionProposed PackageRevisionLifecycle = "DeletionProposed"
)

type WorkspaceName string
Expand Down
7 changes: 4 additions & 3 deletions porch/api/porch/v1alpha1/types_packagerevisions.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,10 @@ type PackageRevisionList struct {
type PackageRevisionLifecycle string

const (
PackageRevisionLifecycleDraft PackageRevisionLifecycle = "Draft"
PackageRevisionLifecycleProposed PackageRevisionLifecycle = "Proposed"
PackageRevisionLifecyclePublished PackageRevisionLifecycle = "Published"
PackageRevisionLifecycleDraft PackageRevisionLifecycle = "Draft"
PackageRevisionLifecycleProposed PackageRevisionLifecycle = "Proposed"
PackageRevisionLifecyclePublished PackageRevisionLifecycle = "Published"
PackageRevisionLifecycleDeletionProposed PackageRevisionLifecycle = "DeletionProposed"
)

type WorkspaceName string
Expand Down
19 changes: 19 additions & 0 deletions porch/api/porch/v1alpha1/util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2022 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License

package v1alpha1

func LifecycleIsPublished(lifecycle PackageRevisionLifecycle) bool {
return lifecycle == PackageRevisionLifecyclePublished || lifecycle == PackageRevisionLifecycleDeletionProposed
}
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (r *DownstreamPackageReconciler) findAndUpdateExistingRevision(ctx context.
return downstream, nil
}

if downstream.Spec.Lifecycle == porchapi.PackageRevisionLifecyclePublished {
if porchapi.LifecycleIsPublished(downstream.Spec.Lifecycle) {
var err error
downstream, err = r.copyPublished(ctx, downstream, dp)
if err != nil {
Expand Down Expand Up @@ -231,7 +231,7 @@ func (r *DownstreamPackageReconciler) getDownstreamPR(dp *api.DownstreamPackage,

// Check if this PR is a draft. We should only have one draft created by this controller at a time,
// so we can just return it.
if pr.Spec.Lifecycle != porchapi.PackageRevisionLifecyclePublished {
if !porchapi.LifecycleIsPublished(pr.Spec.Lifecycle) {
return &pr
} else {
latestPublished, latestVersion = compare(&pr, latestPublished, latestVersion)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func (r *RootSyncRolloutReconciler) getPackages(ctx context.Context, rollout *v1
packageRevision := packageRevisionList.Items[i]
packageName := packageRevision.Spec.PackageName
// TODO: See if we can handle this with a FieldSelector on packagerevisions.
if packageRevision.Spec.Lifecycle != porchapi.PackageRevisionLifecyclePublished {
if !porchapi.LifecycleIsPublished(packageRevision.Spec.Lifecycle) {
continue
}
if _, found := packageMap[packageName]; !found {
Expand Down
11 changes: 11 additions & 0 deletions porch/deployments/porch/3-porch-server.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ spec:
volumes:
- name: cache-volume
emptyDir: {}
- name: webhook-certs
emptyDir: {}
containers:
- name: porch-server
# Update image to the image of your porch apiserver build.
Expand All @@ -52,12 +54,16 @@ spec:
volumeMounts:
- mountPath: /cache
name: cache-volume
- mountPath: /etc/webhook/certs
name: webhook-certs
env:
# Uncomment to enable trace-reporting to jaeger
#- name: OTEL
# value: otel://jaeger-oltp:4317
- name: OTEL_SERVICE_NAME
value: porch-server
- name: CERT_STORAGE_DIR
value: "/etc/webhook/certs"
args:
- --function-runner=function-runner:9445
- --cache-directory=/cache
Expand All @@ -73,5 +79,10 @@ spec:
- port: 443
protocol: TCP
targetPort: 443
name: api
- port: 8443
protocol: TCP
targetPort: 8443
name: webhooks
selector:
app: porch-server
5 changes: 4 additions & 1 deletion porch/deployments/porch/5-rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@ rules:
- apiGroups: ["admissionregistration.k8s.io"]
resources:
["mutatingwebhookconfigurations", "validatingwebhookconfigurations"]
verbs: ["get", "watch", "list"]
verbs: ["get", "watch", "list", "create", "patch", "delete"]
- apiGroups: ["porch.kpt.dev"]
resources: ["functions"]
verbs: ["get", "list", "watch", "create", "update", "patch"]
- apiGroups: ["config.porch.kpt.dev"]
resources: ["repositories", "repositories/status"]
verbs: ["get", "list", "watch", "create", "update", "patch"]
- apiGroups: ["porch.kpt.dev"]
resources: ["packagerevisions", "packagerevisions/status"]
verbs: ["get", "list"]
- apiGroups: ["config.porch.kpt.dev"]
resources: ["packagerevs", "packagerevs/status"]
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
Expand Down
11 changes: 11 additions & 0 deletions porch/pkg/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package apiserver
import (
"context"
"fmt"
"os"

"github.com/GoogleContainerTools/kpt/internal/fnruntime"
"github.com/GoogleContainerTools/kpt/porch/api/porch/install"
Expand All @@ -39,6 +40,7 @@ import (
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand Down Expand Up @@ -277,5 +279,14 @@ func (c completedConfig) New() (*PorchServer, error) {

func (s *PorchServer) Run(ctx context.Context) error {
porch.RunBackground(ctx, s.coreClient, s.cache)
certStorageDir, found := os.LookupEnv("CERT_STORAGE_DIR")
if found && certStorageDir != "" {
if err := setupWebhooks(ctx, certStorageDir); err != nil {
klog.Errorf("%v\n", err)
return err
}
} else {
klog.Infoln("Cert storage dir not provided, skipping webhook setup")
}
return s.GenericAPIServer.PrepareRun().Run(ctx.Done())
}

0 comments on commit bb2725f

Please sign in to comment.