Add finalizer to link resource (#20321)

* Add finalizer to link resource

* Update internal/hcp/internal/controllers/link/controller.go

Co-authored-by: Semir Patel <semir.patel@hashicorp.com>

* Address PR style feedback

---------

Co-authored-by: Semir Patel <semir.patel@hashicorp.com>

internal/hcp/internal/controllers/link/controller.go

@@ -7,10 +7,9 @@ import (
 	"context"
 	"strings"
 
+	gnmmod "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/models"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
-
-	gnmmod "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/models"
 	"google.golang.org/protobuf/types/known/anypb"
 
 	"github.com/hashicorp/consul/agent/hcp/bootstrap"
@@ -18,6 +17,7 @@ import (
 	"github.com/hashicorp/consul/agent/hcp/config"
 	"github.com/hashicorp/consul/internal/controller"
 	"github.com/hashicorp/consul/internal/hcp/internal/types"
+	"github.com/hashicorp/consul/internal/resource"
 	"github.com/hashicorp/consul/internal/storage"
 	pbhcp "github.com/hashicorp/consul/proto-public/pbhcp/v2"
 	"github.com/hashicorp/consul/proto-public/pbresource"
@@ -108,6 +108,19 @@ func (r *linkReconciler) Reconcile(ctx context.Context, rt controller.Runtime, r
 		return err
 	}
 
+	if err = addFinalizer(ctx, rt, res); err != nil {
+		rt.Logger.Error("error adding finalizer to link resource", "error", err)
+		return err
+	}
+
+	if resource.IsMarkedForDeletion(res) {
+		if err = cleanup(ctx, rt, res, r.dataDir); err != nil {
+			rt.Logger.Error("error cleaning up link resource", "error", err)
+			return err
+		}
+		return nil
+	}
+
 	// Validation - Ensure V2 Resource APIs are not enabled unless hcpAllowV2ResourceApis flag is set
 	var newStatus *pbresource.Status
 	if r.resourceApisEnabled && !r.hcpAllowV2ResourceApis {

internal/hcp/internal/controllers/link/controller_test.go

@@ -6,24 +6,28 @@ package link
 import (
 	"context"
 	"fmt"
+	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/hashicorp/go-uuid"
+	gnmmod "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/models"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
-	"github.com/hashicorp/go-uuid"
-	gnmmod "github.com/hashicorp/hcp-sdk-go/clients/cloud-global-network-manager-service/preview/2022-02-15/models"
-
 	svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing"
+	"github.com/hashicorp/consul/agent/hcp/bootstrap"
 	hcpclient "github.com/hashicorp/consul/agent/hcp/client"
 	"github.com/hashicorp/consul/agent/hcp/config"
 	"github.com/hashicorp/consul/internal/controller"
 	"github.com/hashicorp/consul/internal/hcp/internal/types"
+	"github.com/hashicorp/consul/internal/resource"
 	rtest "github.com/hashicorp/consul/internal/resource/resourcetest"
 	pbhcp "github.com/hashicorp/consul/proto-public/pbhcp/v2"
 	"github.com/hashicorp/consul/proto-public/pbresource"
 	"github.com/hashicorp/consul/sdk/testutil"
+	"github.com/hashicorp/consul/sdk/testutil/retry"
 )
 
 type controllerSuite struct {
@@ -34,6 +38,7 @@ type controllerSuite struct {
 	rt     controller.Runtime
 
 	tenancies []*pbresource.Tenancy
+	dataDir   string
 }
 
 func mockHcpClientFn(t *testing.T) (*hcpclient.MockClient, HCPClientFn) {
@@ -68,6 +73,18 @@ func TestLinkController(t *testing.T) {
 func (suite *controllerSuite) deleteResourceFunc(id *pbresource.ID) func() {
 	return func() {
 		suite.client.MustDelete(suite.T(), id)
+
+		// Ensure hcp-config directory is removed
+		retry.Run(suite.T(), func(r *retry.R) {
+			if suite.dataDir != "" {
+				file := filepath.Join(suite.dataDir, bootstrap.SubDir)
+				if _, err := os.Stat(file); !os.IsNotExist(err) {
+					r.Fatalf("should have removed hcp-config directory")
+				}
+			}
+		})
+
+		suite.client.WaitForDeletion(suite.T(), id)
 	}
 }
 
@@ -90,6 +107,7 @@ func (suite *controllerSuite) TestController_Ok() {
 		}, nil).Once()
 
 	dataDir := testutil.TempDir(suite.T(), "test-link-controller")
+	suite.dataDir = dataDir
 	mgr.Register(LinkController(
 		false,
 		false,
@@ -112,6 +130,11 @@ func (suite *controllerSuite) TestController_Ok() {
 
 	suite.T().Cleanup(suite.deleteResourceFunc(link.Id))
 
+	// Ensure finalizer was added
+	suite.client.WaitForResourceState(suite.T(), link.Id, func(t rtest.T, res *pbresource.Resource) {
+		require.True(t, resource.HasFinalizer(res, StatusKey), "link resource does not have finalizer")
+	})
+
 	suite.client.WaitForStatusCondition(suite.T(), link.Id, StatusKey, ConditionLinked(linkData.ResourceId))
 	var updatedLink pbhcp.Link
 	updatedLinkResource := suite.client.WaitForNewVersion(suite.T(), link.Id, link.Version)
@@ -138,6 +161,7 @@ func (suite *controllerSuite) TestController_Initialize() {
 	}
 
 	dataDir := testutil.TempDir(suite.T(), "test-link-controller")
+	suite.dataDir = dataDir
 
 	mgr.Register(LinkController(
 		false,
@@ -176,6 +200,7 @@ func (suite *controllerSuite) TestControllerResourceApisEnabled_LinkDisabled() {
 	mgr := controller.NewManager(suite.client, suite.rt.Logger)
 	_, mockClientFunc := mockHcpClientFn(suite.T())
 	dataDir := testutil.TempDir(suite.T(), "test-link-controller")
+	suite.dataDir = dataDir
 	mgr.Register(LinkController(
 		true,
 		false,
@@ -217,6 +242,7 @@ func (suite *controllerSuite) TestControllerResourceApisEnabledWithOverride_Link
 		}, nil).Once()
 
 	dataDir := testutil.TempDir(suite.T(), "test-link-controller")
+	suite.dataDir = dataDir
 
 	mgr.Register(LinkController(
 		true,
@@ -271,6 +297,7 @@ func (suite *controllerSuite) TestController_GetClusterError() {
 			mockClient.EXPECT().GetCluster(mock.Anything).Return(nil, tc.expectErr)
 
 			dataDir := testutil.TempDir(suite.T(), "test-link-controller")
+			suite.dataDir = dataDir
 			mgr.Register(LinkController(
 				true,
 				true,

internal/hcp/internal/controllers/link/finalizer.go

@@ -0,0 +1,74 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package link
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+
+	"github.com/hashicorp/consul/agent/hcp/bootstrap"
+	"github.com/hashicorp/consul/internal/controller"
+	"github.com/hashicorp/consul/internal/resource"
+	"github.com/hashicorp/consul/proto-public/pbresource"
+)
+
+func cleanup(ctx context.Context, rt controller.Runtime, res *pbresource.Resource, dataDir string) error {
+	rt.Logger.Trace("cleaning up link resource")
+	if dataDir != "" {
+		hcpConfigDir := filepath.Join(dataDir, bootstrap.SubDir)
+		rt.Logger.Debug("deleting hcp-config dir", "dir", hcpConfigDir)
+		err := os.RemoveAll(hcpConfigDir)
+		if err != nil {
+			return err
+		}
+	}
+
+	err := ensureDeleted(ctx, rt, res)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func addFinalizer(ctx context.Context, rt controller.Runtime, res *pbresource.Resource) error {
+	// The statusKey doubles as the finalizer name for the link resource
+	if resource.HasFinalizer(res, StatusKey) {
+		rt.Logger.Trace("already has finalizer")
+		return nil
+	}
+
+	// Finalizer hasn't been written, so add it.
+	resource.AddFinalizer(res, StatusKey)
+	_, err := rt.Client.Write(ctx, &pbresource.WriteRequest{Resource: res})
+	if err != nil {
+		return err
+	}
+	rt.Logger.Trace("added finalizer")
+	return err
+}
+
+// ensureDeleted makes sure a link is finally deleted
+func ensureDeleted(ctx context.Context, rt controller.Runtime, res *pbresource.Resource) error {
+	// Remove finalizer if present
+	if resource.HasFinalizer(res, StatusKey) {
+		resource.RemoveFinalizer(res, StatusKey)
+		_, err := rt.Client.Write(ctx, &pbresource.WriteRequest{Resource: res})
+		if err != nil {
+			return err
+		}
+		rt.Logger.Trace("removed finalizer")
+	}
+
+	// Finally, delete the link
+	_, err := rt.Client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id})
+	if err != nil {
+		return err
+	}
+
+	// Success
+	rt.Logger.Trace("finally deleted")
+	return nil
+}