Webhook validates deletion

cmd/shipperctl/configurator/cluster.go

@@ -428,6 +428,7 @@ func (c *Cluster) CreateOrUpdateValidatingWebhookConfiguration(caBundle []byte,
 						Operations: []admissionregistrationv1beta1.OperationType{
 							admissionregistrationv1beta1.Create,
 							admissionregistrationv1beta1.Update,
+							admissionregistrationv1beta1.Delete,
 						},
 						Rule: admissionregistrationv1beta1.Rule{
 							APIGroups:   []string{shipper.SchemeGroupVersion.Group},

cmd/shipperctl/configurator/cluster_test.go

@@ -28,6 +28,7 @@ func TestCreateValidatingWebhookConfiguration(t *testing.T) {
 	operations := []admissionregistrationv1beta1.OperationType{
 		admissionregistrationv1beta1.Create,
 		admissionregistrationv1beta1.Update,
+		admissionregistrationv1beta1.Delete,
 	}
 	expectedConfiguration := f.newValidatingWebhookConfiguration(caBundle, shipperSystemNamespace, operations)
 	gvr := admissionregistrationv1beta1.SchemeGroupVersion.WithResource("validatingwebhookconfigurations")
@@ -65,6 +66,7 @@ func TestUpdateValidatingWebhookConfiguration(t *testing.T) {
 	operations = []admissionregistrationv1beta1.OperationType{
 		admissionregistrationv1beta1.Create,
 		admissionregistrationv1beta1.Update,
+		admissionregistrationv1beta1.Delete,
 	}
 	expectedConfiguration := f.newValidatingWebhookConfiguration(caBundle, shipperSystemNamespace, operations)
 	getAction := kubetesting.NewGetAction(gvr, "", expectedConfiguration.Name)

pkg/controller/release/release_controller.go

@@ -1,7 +1,9 @@
 package release
 
 import (
+	"encoding/json"
 	"fmt"
+	"strings"
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -301,6 +303,10 @@ func (c *Controller) syncOneReleaseHandler(key string) error {
 		c.recorder,
 	)
 
+	if err := c.updateApplicationOverrides(rel); err != nil {
+		klog.Errorf("failed to update application overrides: %v", err)
+	}
+
 	rolloutBlocked, events, err := rolloutblock.BlocksRollout(c.rolloutBlockLister, rel)
 	for _, ev := range events {
 		c.recorder.Event(rel, ev.Type, ev.Reason, ev.Message)
@@ -388,6 +394,31 @@ ApplyChanges:
 	return err
 }
 
+func (c *Controller) updateApplicationOverrides(rel *shipper.Release) error {
+	namespace := rel.GetNamespace()
+	// update application with same override annotations as release
+	appName, err := releaseutil.ApplicationNameForRelease(rel)
+	if err != nil {
+		return err
+	}
+	application, err := c.applicationLister.Applications(namespace).Get(appName)
+	if err != nil {
+		return err
+	}
+	applicationOverrides := application.GetAnnotations()[shipper.RolloutBlocksOverrideAnnotation]
+	newOverrides := rolloutblock.ApplicationOverrides(applicationOverrides, rel.GetAnnotations()[shipper.RolloutBlocksOverrideAnnotation])
+	if strings.EqualFold(applicationOverrides, newOverrides) {
+		return nil
+	}
+	patch := make(map[string]interface{})
+	patch["annotations"] = newOverrides
+	b, _ := json.Marshal(patch)
+	if _, err = c.clientset.ShipperV1alpha1().Applications(namespace).Patch(appName, types.MergePatchType, b); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (c *Controller) applicationReleases(rel *shipper.Release) ([]*shipper.Release, error) {
 	appName, err := releaseutil.ApplicationNameForRelease(rel)
 	if err != nil {
@@ -444,7 +475,6 @@ func updateConditions(rel *shipper.Release, diff *diffutil.MultiDiff, targetStep
 	isLastStep := int(targetStep) == len(strategy.Steps)-1
 	prevStep := rel.Status.AchievedStep
 
-
 	var achievedStep int32
 	if complete {
 		var achievedStepName string

pkg/controller/release/release_controller_test.go

@@ -249,9 +249,10 @@ func newRolloutBlock(name string, namespace string) *shipper.RolloutBlock {
 func buildApplication(namespace string, appName string) *shipper.Application {
 	return &shipper.Application{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      appName,
-			Namespace: namespace,
-			UID:       "foobarbaz",
+			Name:        appName,
+			Namespace:   namespace,
+			UID:         "foobarbaz",
+			Annotations: make(map[string]string),
 		},
 		Status: shipper.ApplicationStatus{
 			History: []string{},

pkg/util/rolloutblock/blocks.go

@@ -90,3 +90,11 @@ func ValidateAnnotations(existing, overrides ObjectNameList) error {
 	}
 	return nil
 }
+
+func ApplicationOverrides(applicationOverrides, releaseOverrides string) string {
+	appOverrides := NewObjectNameList(applicationOverrides)
+	relOverrides := NewObjectNameList(releaseOverrides)
+	diff := relOverrides.Diff(appOverrides)
+	appOverrides.AddMultiple(diff)
+	return appOverrides.String()
+}

pkg/util/rolloutblock/object_name_list.go

@@ -61,6 +61,12 @@ func (o ObjectNameList) Add(statement string) {
 	o[statement] = struct{}{}
 }
 
+func (o ObjectNameList) AddMultiple(o2 ObjectNameList) {
+	for s := range o2 {
+		o.Add(s)
+	}
+}
+
 func (o ObjectNameList) Diff(o2 ObjectNameList) ObjectNameList {
 	res := make(ObjectNameList)
 	for s := range o {

pkg/webhook/webhook.go

@@ -220,18 +220,39 @@ func (c *Webhook) validateHandlerFunc(review *admission.AdmissionReview) *admiss
 	request := review.Request
 	var err error
 
+	if request.Operation == kubeclient.Delete {
+		err = c.validateDelete(request)
+	} else {
+		err = c.validateCreateUpdate(request)
+	}
+
+	if err != nil {
+		return &admission.AdmissionResponse{
+			Result: &metav1.Status{
+				Message: err.Error(),
+			},
+		}
+	}
+
+	return &admission.AdmissionResponse{
+		Allowed: true,
+	}
+}
+
+func (c *Webhook) validateCreateUpdate(request *kubeclient.AdmissionRequest) error {
+	var err error
 	switch request.Kind.Kind {
 	case "Application":
 		var application shipper.Application
 		err = json.Unmarshal(request.Object.Raw, &application)
 		if err == nil {
-			err = c.validateApplication(request, application)
+			err = c.validateBlocksForApplication(request, application)
 		}
 	case "Release":
 		var release shipper.Release
 		err = json.Unmarshal(request.Object.Raw, &release)
 		if err == nil {
-			err = c.validateRelease(request, release)
+			err = c.validateBlocksForRelease(request, release)
 		}
 	case "Cluster":
 		var cluster shipper.Cluster
@@ -249,21 +270,31 @@ func (c *Webhook) validateHandlerFunc(review *admission.AdmissionReview) *admiss
 		var rolloutBlock shipper.RolloutBlock
 		err = json.Unmarshal(request.Object.Raw, &rolloutBlock)
 	}
+	return err
+}
 
-	if err != nil {
-		return &admission.AdmissionResponse{
-			Result: &metav1.Status{
-				Message: err.Error(),
-			},
-		}
+func (c *Webhook) validateDelete(request *kubeclient.AdmissionRequest) error {
+	var err error
+	var obj metav1.Object
+	switch request.Kind.Kind {
+	case "Application":
+		var application shipper.Application
+		err = json.Unmarshal(request.OldObject.Raw, &application)
+		obj = &application
+	case "Release":
+		var release shipper.Release
+		err = json.Unmarshal(request.OldObject.Raw, &release)
+		obj = &release
+	default:
+		return nil
 	}
-
-	return &admission.AdmissionResponse{
-		Allowed: true,
+	if err != nil {
+		return err
 	}
+	return c.validateBlocksForObject(obj)
 }
 
-func (c *Webhook) validateRelease(request *admission.AdmissionRequest, release shipper.Release) error {
+func (c *Webhook) validateBlocksForRelease(request *admission.AdmissionRequest, release shipper.Release) error {
 	var err error
 	overrides, existingBlocks, err := rolloutblock.GetAllBlocks(c.rolloutBlocksLister, &release)
 	if err != nil {
@@ -296,7 +327,7 @@ func (c *Webhook) validateRelease(request *admission.AdmissionRequest, release s
 	return err
 }
 
-func (c *Webhook) validateApplication(request *admission.AdmissionRequest, application shipper.Application) error {
+func (c *Webhook) validateBlocksForApplication(request *admission.AdmissionRequest, application shipper.Application) error {
 	var err error
 	overrides, existingBlocks, err := rolloutblock.GetAllBlocks(c.rolloutBlocksLister, &application)
 	if err != nil {
@@ -322,3 +353,18 @@ func (c *Webhook) validateApplication(request *admission.AdmissionRequest, appli
 
 	return err
 }
+
+func (c *Webhook) validateBlocksForObject(obj metav1.Object) error {
+	overrides, existingBlocks, err := rolloutblock.GetAllBlocks(c.rolloutBlocksLister, obj)
+	if err != nil {
+		return err
+	}
+	if err := rolloutblock.ValidateAnnotations(existingBlocks, overrides); err != nil {
+		return err
+	}
+	if err := rolloutblock.ValidateBlocks(existingBlocks, overrides); err != nil {
+		return err
+	}
+
+	return nil
+}

test/e2e/e2e_test.go

@@ -198,6 +198,102 @@ func TestRejectModificationToReleaseEnv(t *testing.T) {
 	t.Logf("successfully failed to update release environment %q", relName)
 }
 
+func TestRejectDeleteReleaseRolloutBlock(t *testing.T) {
+	if !*runEndToEnd {
+		t.Skip("skipping end-to-end tests: --e2e is false")
+	}
+	t.Parallel()
+
+	targetReplicas := 4
+	ns, err := setupNamespace(t.Name())
+	if err != nil {
+		t.Fatalf("could not create namespace %s: %q", t.Name(), err)
+	}
+	f := newFixture(ns.GetName(), t)
+	defer func() {
+		if *inspectFailed && t.Failed() {
+			return
+		}
+		teardownNamespace(ns.GetName())
+	}()
+
+	newApp := newApplication(ns.GetName(), appName, &allIn)
+	newApp.Spec.Template.Values = &shipper.ChartValues{"replicaCount": targetReplicas}
+	newApp.Spec.Template.Chart.Name = "test-nginx"
+	newApp.Spec.Template.Chart.Version = "0.0.1"
+
+	_, err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Create(newApp)
+	if err != nil {
+		t.Fatalf("could not create application %q: %q", appName, err)
+	}
+
+	t.Logf("waiting for a new release for new application %q", appName)
+	rel := f.waitForRelease(appName, 0)
+	relName := rel.GetName()
+	t.Logf("waiting for release %q to complete", relName)
+	f.waitForComplete(rel.GetName())
+	t.Logf("checking that release %q has %d pods (strategy step 0 -- finished)", relName, targetReplicas)
+	f.checkReadyPods(relName, targetReplicas)
+
+	_, err = createRolloutBlock(ns.GetName(), rolloutBlockName)
+	if err != nil {
+		t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err)
+	}
+
+	if err = shipperClient.ShipperV1alpha1().Releases(ns.GetName()).Delete(relName, &metav1.DeleteOptions{}); err == nil {
+		t.Fatalf("deleted release %q: %q", relName, err)
+	}
+	t.Logf("successfully failed to delete release %q", relName)
+}
+
+func TestRejectDeleteApplicationRolloutBlock(t *testing.T) {
+	if !*runEndToEnd {
+		t.Skip("skipping end-to-end tests: --e2e is false")
+	}
+	t.Parallel()
+
+	targetReplicas := 4
+	ns, err := setupNamespace(t.Name())
+	if err != nil {
+		t.Fatalf("could not create namespace %s: %q", t.Name(), err)
+	}
+	f := newFixture(ns.GetName(), t)
+	defer func() {
+		if *inspectFailed && t.Failed() {
+			return
+		}
+		teardownNamespace(ns.GetName())
+	}()
+
+	newApp := newApplication(ns.GetName(), appName, &allIn)
+	newApp.Spec.Template.Values = &shipper.ChartValues{"replicaCount": targetReplicas}
+	newApp.Spec.Template.Chart.Name = "test-nginx"
+	newApp.Spec.Template.Chart.Version = "0.0.1"
+
+	_, err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Create(newApp)
+	if err != nil {
+		t.Fatalf("could not create application %q: %q", appName, err)
+	}
+
+	t.Logf("waiting for a new release for new application %q", appName)
+	rel := f.waitForRelease(appName, 0)
+	relName := rel.GetName()
+	t.Logf("waiting for release %q to complete", relName)
+	f.waitForComplete(rel.GetName())
+	t.Logf("checking that release %q has %d pods (strategy step 0 -- finished)", relName, targetReplicas)
+	f.checkReadyPods(relName, targetReplicas)
+
+	_, err = createRolloutBlock(ns.GetName(), rolloutBlockName)
+	if err != nil {
+		t.Fatalf("could not create rollout block %q: %q", rolloutBlockName, err)
+	}
+
+	if err = shipperClient.ShipperV1alpha1().Applications(ns.GetName()).Delete(appName, &metav1.DeleteOptions{}); err == nil {
+		t.Fatalf("deleted application %q: %q", appName, err)
+	}
+	t.Logf("successfully failed to delete application %q", relName)
+}
+
 func TestNewAppAllInWithRolloutBlockOverride(t *testing.T) {
 	if !*runEndToEnd {
 		t.Skip("skipping end-to-end tests: --e2e is false")