From f6a0f7fc24ccee414079e85018ac1e8ad9c192a6 Mon Sep 17 00:00:00 2001
From: Utku Ozdemir <utku.ozdemir@siderolabs.com>
Date: Thu, 19 Dec 2024 13:39:39 +0100
Subject: [PATCH] fix: run destroy validations on teardown

If some operation violates destroy validations, we probably want it to not switch to TearingDown phase either. So we now run destroy validations as well as update validations when a resource is attempted to be torn down.

Signed-off-by: Utku Ozdemir <utku.ozdemir@siderolabs.com>
---
 .../backend/runtime/omni/validated/state.go   | 19 ++++++++-
 .../runtime/omni/validated/state_test.go      | 40 +++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/internal/backend/runtime/omni/validated/state.go b/internal/backend/runtime/omni/validated/state.go
index e3c9762cb..7a154b20a 100644
--- a/internal/backend/runtime/omni/validated/state.go
+++ b/internal/backend/runtime/omni/validated/state.go
@@ -105,17 +105,32 @@ func (v *State) Update(ctx context.Context, newResource resource.Resource, opts
 		return err
 	}
 
+	var validationErrs error
+
 	// if the existing resource was not found, instead of returning the not found error, run the validations first
 	// only if the validations pass, return the not found error
 
-	var validationErrs error
-
 	for _, validation := range v.updateValidations {
 		if validationErr := validation(ctx, existing, newResource, opts...); validationErr != nil {
 			validationErrs = multierror.Append(validationErrs, validationErr)
 		}
 	}
 
+	// If the resource is tearing down, run the destroy validations as well
+	if newResource.Metadata().Phase() == resource.PhaseTearingDown {
+		updateOpts := state.UpdateOptions{}
+
+		for _, opt := range opts {
+			opt(&updateOpts)
+		}
+
+		for _, validation := range v.destroyValidations {
+			if validationErr := validation(ctx, newResource.Metadata(), existing, state.WithDestroyOwner(updateOpts.Owner)); validationErr != nil {
+				validationErrs = multierror.Append(validationErrs, validationErr)
+			}
+		}
+	}
+
 	if validationErrs != nil {
 		return ValidationError(validationErrs)
 	}
diff --git a/internal/backend/runtime/omni/validated/state_test.go b/internal/backend/runtime/omni/validated/state_test.go
index 3c54a5556..cbbba0a40 100644
--- a/internal/backend/runtime/omni/validated/state_test.go
+++ b/internal/backend/runtime/omni/validated/state_test.go
@@ -13,11 +13,13 @@ import (
 	"time"
 
 	"github.com/cosi-project/runtime/pkg/resource"
+	"github.com/cosi-project/runtime/pkg/safe"
 	"github.com/cosi-project/runtime/pkg/state"
 	"github.com/cosi-project/runtime/pkg/state/impl/inmem"
 	"github.com/cosi-project/runtime/pkg/state/impl/namespaced"
 	"github.com/hashicorp/go-multierror"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/siderolabs/omni/client/pkg/omni/resources"
 	"github.com/siderolabs/omni/client/pkg/omni/resources/omni"
@@ -205,3 +207,41 @@ func TestValidations(t *testing.T) {
 	_, err = innerSt.Get(ctx, machine.Metadata())
 	assert.True(t, state.IsNotFoundError(err))
 }
+
+func TestTeardownDestroyValidations(t *testing.T) {
+	innerSt := state.WrapCore(namespaced.NewState(inmem.Build))
+	st := state.WrapCore(
+		validated.NewState(innerSt,
+			validated.WithUpdateValidations(func(context.Context, resource.Resource, resource.Resource, ...state.UpdateOption) error {
+				return errors.New("update")
+			}), validated.WithDestroyValidations(func(_ context.Context, _ resource.Pointer, _ resource.Resource, option ...state.DestroyOption) error {
+				opts := state.DestroyOptions{}
+
+				for _, opt := range option {
+					opt(&opts)
+				}
+
+				return errors.New("destroy by " + opts.Owner)
+			}),
+		),
+	)
+
+	res := omni.NewCluster(resources.DefaultNamespace, "something")
+
+	require.NoError(t, st.Create(context.Background(), res))
+
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	t.Cleanup(cancel)
+
+	_, err := safe.StateUpdateWithConflicts(ctx, st, res.Metadata(), func(res *omni.Cluster) error {
+		res.TypedSpec().Value.TalosVersion = "1234"
+
+		return nil
+	})
+	require.EqualError(t, err, "failed to validate: 1 error occurred:\n\t* update\n\n")
+
+	const teardownOwner = "foobar-controller"
+
+	_, err = st.Teardown(ctx, res.Metadata(), state.WithTeardownOwner(teardownOwner))
+	require.EqualError(t, err, fmt.Sprintf("failed to validate: 2 errors occurred:\n\t* update\n\t* destroy by %s\n\n", teardownOwner))
+}