diff --git a/cmd/shipperctl/configurator/cluster.go b/cmd/shipperctl/configurator/cluster.go index d50ae7e08..4625827c8 100644 --- a/cmd/shipperctl/configurator/cluster.go +++ b/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}, diff --git a/cmd/shipperctl/configurator/cluster_test.go b/cmd/shipperctl/configurator/cluster_test.go index 0c5a0c34e..84f9c6d70 100644 --- a/cmd/shipperctl/configurator/cluster_test.go +++ b/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) diff --git a/pkg/controller/release/release_controller.go b/pkg/controller/release/release_controller.go index 90edb1f19..3a501d65f 100644 --- a/pkg/controller/release/release_controller.go +++ b/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 diff --git a/pkg/controller/release/release_controller_test.go b/pkg/controller/release/release_controller_test.go index 0f63281ae..3178896c8 100644 --- a/pkg/controller/release/release_controller_test.go +++ b/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{}, diff --git a/pkg/util/rolloutblock/blocks.go b/pkg/util/rolloutblock/blocks.go index 70cae1c13..8858d728f 100644 --- a/pkg/util/rolloutblock/blocks.go +++ b/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() +} diff --git a/pkg/util/rolloutblock/object_name_list.go b/pkg/util/rolloutblock/object_name_list.go index ebe3e61ed..e5e07f9be 100644 --- a/pkg/util/rolloutblock/object_name_list.go +++ b/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 { diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index a23089c12..db16def13 100644 --- a/pkg/webhook/webhook.go +++ b/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 +} diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index fe6a08a5c..a62dfcca5 100644 --- a/test/e2e/e2e_test.go +++ b/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")