atc: static and dynamic airway modes support resource deletions

Author: davidmdm

cmd/atc-installer/installer/run.go

@@ -375,6 +375,7 @@ func Run(cfg Config) (flight.Stages, error) {
 					{
 						Operations: []admissionregistrationv1.OperationType{
 							admissionregistrationv1.Update,
+							admissionregistrationv1.Delete,
 						},
 						Rule: admissionregistrationv1.Rule{
 							APIGroups:   []string{"*"},

cmd/atc/handler.go

@@ -249,38 +249,26 @@ func Handler(client *k8s.Client, cache *wasm.ModuleCache, controllers *atc.Contr
 
 		addRequestAttrs(r.Context(), slog.String("user", review.Request.UserInfo.Username))
 
-		review.Response = &admissionv1.AdmissionResponse{
-			UID:     review.Request.UID,
-			Allowed: true,
-			Result: &metav1.Status{
-				Status:  metav1.StatusSuccess,
-				Message: "validation passed",
-			},
-		}
-
-		var prev unstructured.Unstructured
-		if err := json.Unmarshal(review.Request.OldObject.Raw, &prev); err != nil {
+		prev, err := UnstructuredFromRawExt(review.Request.OldObject)
+		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
 
-		var next unstructured.Unstructured
-		if err := json.Unmarshal(review.Request.Object.Raw, &next); err != nil {
+		next, err := UnstructuredFromRawExt(review.Request.Object)
+		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
 		}
 
-		addRequestAttrs(r.Context(), slog.String("resourceId", internal.Canonical(&next)))
-
-		if internal.GetOwner(&prev) != internal.GetOwner(&next) {
-			review.Response.Allowed = false
-			review.Response.Result = &metav1.Status{
-				Message: "cannot modify yoke labels",
-				Status:  metav1.StatusFailure,
-				Reason:  metav1.StatusReasonBadRequest,
-			}
+		review.Response = &admissionv1.AdmissionResponse{
+			UID:     review.Request.UID,
+			Allowed: true,
+			Result: &metav1.Status{
+				Status:  metav1.StatusSuccess,
+				Message: "validation passed",
+			},
 		}
-
 		defer func() {
 			review.Request = nil
 			addRequestAttrs(r.Context(), slog.Group(
@@ -291,16 +279,24 @@ func Handler(client *k8s.Client, cache *wasm.ModuleCache, controllers *atc.Contr
 			json.NewEncoder(w).Encode(review)
 		}()
 
-		if !review.Response.Allowed {
+		addRequestAttrs(r.Context(), slog.String("resourceId", internal.Canonical(prev)))
+
+		if next != nil && internal.GetOwner(prev) != internal.GetOwner(next) {
+			review.Response.Allowed = false
+			review.Response.Result = &metav1.Status{
+				Message: "cannot modify yoke labels",
+				Status:  metav1.StatusFailure,
+				Reason:  metav1.StatusReasonBadRequest,
+			}
 			return
 		}
 
-		if ctrl.ResourcesAreEqual(&prev, &next) {
+		if next != nil && ctrl.ResourcesAreEqual(prev, next) {
 			addRequestAttrs(r.Context(), slog.String("skipReason", "resources are equal"))
 			return
 		}
 
-		owners := next.GetOwnerReferences()
+		owners := prev.GetOwnerReferences()
 		if len(owners) == 0 {
 			addRequestAttrs(r.Context(), slog.String("skipReason", "no owner references"))
 			return
@@ -328,30 +324,43 @@ func Handler(client *k8s.Client, cache *wasm.ModuleCache, controllers *atc.Contr
 		)
 
 		if !ok {
+			addRequestAttrs(r.Context(), slog.String("skipReason", "no registered flight controller"))
 			return
 		}
 
-		labels := next.GetLabels()
+		labels := prev.GetLabels()
 		release := labels[internal.LabelYokeRelease]
 		namespace := labels[internal.LabelYokeReleaseNS]
 
-		mode := controller.FlightMode(next.GetName(), namespace)
+		mode := controller.FlightMode(prev.GetName(), namespace)
 
 		addRequestAttrs(r.Context(), slog.String("airwayMode", string(mode)))
 
 		switch mode {
 		case v1alpha1.AirwayModeStatic:
+			if next == nil || !next.GetDeletionTimestamp().IsZero() {
+				review.Response.Allowed = false
+				review.Response.Result = &metav1.Status{
+					Message: "cannot delete resources managed by Air-Traffic-Controller",
+					Status:  metav1.StatusFailure,
+					Reason:  metav1.StatusReasonBadRequest,
+				}
+				return
+			}
+
 			release, err := client.GetRelease(r.Context(), release, namespace)
 			if err != nil {
-				// Handle?
+				addRequestAttrs(r.Context(), slog.String("skipReason", fmt.Sprintf("failed to get release: %v", err)))
 				return
 			}
 			if len(release.History) == 0 {
+				addRequestAttrs(r.Context(), slog.String("skipReason", "no release history found"))
 				return
 			}
 
 			stages, err := client.GetRevisionResources(r.Context(), release.ActiveRevision())
 			if err != nil {
+				addRequestAttrs(r.Context(), slog.String("skipReason", fmt.Sprintf("failed to get release resources: %v", err)))
 				return
 			}
 
@@ -363,12 +372,13 @@ func Handler(client *k8s.Client, cache *wasm.ModuleCache, controllers *atc.Contr
 					resource.GetName() == next.GetName()
 			})
 			if !ok {
+				addRequestAttrs(r.Context(), slog.String("skipReason", "could not find desired resource in release"))
 				return
 			}
 
-			internal.RemoveAdditions(desired, &next)
+			internal.RemoveAdditions(desired, next)
 
-			if !ctrl.ResourcesAreEqual(desired, &next) {
+			if !ctrl.ResourcesAreEqual(desired, next) {
 				review.Response.Allowed = false
 				review.Response.Result = &metav1.Status{
 					Message: "cannot modify flight sub-resources",
@@ -511,3 +521,16 @@ func addRequestAttrs(ctx context.Context, attrs ...slog.Attr) {
 	}
 	*reqAttrs = append(*reqAttrs, attrs...)
 }
+
+func UnstructuredFromRawExt(ext runtime.RawExtension) (*unstructured.Unstructured, error) {
+	if len(ext.Raw) == 0 {
+		return nil, nil
+	}
+
+	var resource unstructured.Unstructured
+	if err := json.Unmarshal(ext.Raw, &resource); err != nil {
+		return nil, err
+	}
+
+	return &resource, nil
+}

cmd/atc/main_test.go

@@ -1244,6 +1244,12 @@ func TestAirwayModes(t *testing.T) {
 	deployment, err = deploymentIntf.Update(ctx, deployment, metav1.UpdateOptions{FieldManager: "test"})
 	require.EqualError(t, err, `admission webhook "resources.yoke.cd" denied the request: cannot modify flight sub-resources`)
 
+	require.EqualError(
+		t,
+		deploymentIntf.Delete(ctx, "test", metav1.DeleteOptions{}),
+		`admission webhook "resources.yoke.cd" denied the request: cannot delete resources managed by Air-Traffic-Controller`,
+	)
+
 	backendIntf := client.Dynamic.
 		Resource(schema.GroupVersionResource{
 			Group:    "examples.com",
@@ -1303,7 +1309,26 @@ func TestAirwayModes(t *testing.T) {
 		},
 		time.Second,
 		30*time.Second,
-		"deployment failed to self-heal",
+		"deployment failed to self-heal from bad value",
+	)
+
+	require.NoError(t, deploymentIntf.Delete(ctx, "test", metav1.DeleteOptions{}))
+
+	testutils.EventuallyNoErrorf(
+		t,
+		func() error {
+			deployment, err = deploymentIntf.Get(ctx, "test", metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+			if actual := *deployment.Spec.Replicas; actual != 2 {
+				return fmt.Errorf("expected replicas to be 2 but got %d", actual)
+			}
+			return nil
+		},
+		time.Second,
+		30*time.Second,
+		"deployment failed to self-heal after deletion",
 	)
 
 	configmapIntf := client.Clientset.CoreV1().ConfigMaps("default")

internal/k8s/k8s.go

@@ -248,13 +248,13 @@ func (client Client) RemoveOrphans(ctx context.Context, previous, current intern
 
 				defer internal.DebugTimer(ctx, "delete resource "+name)()
 
-				resourceInterface, err := client.GetDynamicResourceInterface(resource)
+				intf, err := client.GetDynamicResourceInterface(resource)
 				if err != nil {
 					errs = append(errs, fmt.Errorf("failed to resolve resource %s: %w", name, err))
 					return
 				}
 
-				if err := resourceInterface.Delete(ctx, resource.GetName(), metav1.DeleteOptions{}); err != nil {
+				if err := intf.Delete(ctx, resource.GetName(), metav1.DeleteOptions{}); err != nil && !kerrors.IsNotFound(err) {
 					errs = append(errs, fmt.Errorf("failed to delete %s: %w", name, err))
 					return
 				}