fix: unregister-member deletes secret & has needed permissions

pkg/cmd/adm/unregister_member.go

@@ -9,6 +9,8 @@ import (
 	"github.com/kubesaw/ksctl/pkg/configuration"
 	clicontext "github.com/kubesaw/ksctl/pkg/context"
 	"github.com/kubesaw/ksctl/pkg/ioutils"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 
 	"github.com/spf13/cobra"
 	"k8s.io/apimachinery/pkg/types"
@@ -59,10 +61,27 @@ func UnregisterMemberCluster(ctx *clicontext.CommandContext, clusterName string,
 		return nil
 	}
 
+	tcSecret := &v1.Secret{}
+	if err := hostClusterClient.Get(context.TODO(), types.NamespacedName{Namespace: hostClusterConfig.OperatorNamespace, Name: toolchainCluster.Spec.SecretRef.Name}, tcSecret); err != nil {
+		if !errors.IsNotFound(err) {
+			return err
+		}
+	} else {
+		ctx.Printlnf("\nDeleting the ToolchainCluster secret %s...", toolchainCluster.Spec.SecretRef.Name)
+		if err := hostClusterClient.Delete(context.TODO(), tcSecret); err != nil {
+			return err
+		}
+		ctx.Printlnf("The ToolchainCluster secret %s has been deleted", toolchainCluster.Spec.SecretRef.Name)
+	}
+
+	ctx.Printlnf("\nDeleting the ToolchainCluster CR %s...", toolchainCluster.Name)
 	if err := hostClusterClient.Delete(context.TODO(), toolchainCluster); err != nil {
 		return err
 	}
-	ctx.Printlnf("\nThe deletion of the Toolchain member cluster from the Host cluster has been triggered")
+	ctx.Printlnf("The ToolchainCluster CR %s has been deleted", toolchainCluster.Name)
+
+	ctx.Printlnf("\nThe deletion of the Member cluster from the Host cluster has been finished.")
 
+	ctx.Printlnf("\nRestarting the Host operator components to reload the current setup...")
 	return restart(ctx, "host", getConfigFlagsAndClient)
 }

pkg/cmd/adm/unregister_member_test.go

@@ -4,17 +4,25 @@ import (
 	"fmt"
 	"testing"
 
+	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
+	"github.com/codeready-toolchain/toolchain-common/pkg/test"
 	clicontext "github.com/kubesaw/ksctl/pkg/context"
 	. "github.com/kubesaw/ksctl/pkg/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func TestUnregisterMemberWhenAnswerIsY(t *testing.T) {
 	// given
 	toolchainCluster := NewToolchainCluster(ToolchainClusterName("member-cool-server.com"))
+	noiseToolchainCluster := NewToolchainCluster(ToolchainClusterName("noise"))
+	secret := newSecret(toolchainCluster)
+	noiseSecret := newSecret(noiseToolchainCluster)
 
-	newClient, fakeClient := NewFakeClients(t, toolchainCluster)
+	newClient, fakeClient := NewFakeClients(t, noiseToolchainCluster, toolchainCluster, secret, noiseSecret)
 
 	SetFileConfig(t, Host(), Member())
 	term := NewFakeTerminalWithResponse("y")
@@ -28,13 +36,44 @@ func TestUnregisterMemberWhenAnswerIsY(t *testing.T) {
 	// then
 	require.NoError(t, err)
 	AssertToolchainClusterDoesNotExist(t, fakeClient, toolchainCluster)
+	AssertObjectExists(t, fakeClient, client.ObjectKeyFromObject(noiseToolchainCluster), &toolchainv1alpha1.ToolchainCluster{})
+	AssertObjectExists(t, fakeClient, client.ObjectKeyFromObject(noiseSecret), &v1.Secret{})
 	assert.Contains(t, term.Output(), "!!!  DANGER ZONE  !!!")
 	assert.NotContains(t, term.Output(), "THIS COMMAND WILL CAUSE UNREGISTER MEMBER CLUSTER FORM HOST CLUSTER. MAKE SURE THERE IS NO USERS LEFT IN THE MEMBER CLUSTER BEFORE UNREGISTERING IT")
 	assert.Contains(t, term.Output(), "Delete Member cluster stated above from the Host cluster?")
-	assert.Contains(t, term.Output(), "The deletion of the Toolchain member cluster from the Host cluster has been triggered")
+	assert.Contains(t, term.Output(), "The deletion of the Member cluster from the Host cluster has been finished.")
 	assert.NotContains(t, term.Output(), "cool-token")
 }
 
+func TestUnregisterMemberWhenSecretIsMissing(t *testing.T) {
+	// given
+	toolchainCluster := NewToolchainCluster(ToolchainClusterName("member-cool-server.com"))
+	newClient, fakeClient := NewFakeClients(t, toolchainCluster)
+
+	SetFileConfig(t, Host(), Member())
+	term := NewFakeTerminalWithResponse("y")
+	ctx := clicontext.NewCommandContext(term, newClient)
+
+	// when
+	err := UnregisterMemberCluster(ctx, "member1", func(_ *clicontext.CommandContext, _ string, _ ConfigFlagsAndClientGetterFunc) error {
+		return nil
+	})
+
+	// then
+	require.NoError(t, err)
+	AssertToolchainClusterDoesNotExist(t, fakeClient, toolchainCluster)
+	assert.NotContains(t, term.Output(), "cool-token")
+}
+
+func newSecret(tc *toolchainv1alpha1.ToolchainCluster) *v1.Secret {
+	return &v1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      tc.Spec.SecretRef.Name,
+			Namespace: test.HostOperatorNs,
+		},
+	}
+}
+
 func TestUnregisterMemberWhenRestartError(t *testing.T) {
 	// given
 	toolchainCluster := NewToolchainCluster(ToolchainClusterName("member-cool-server.com"))
@@ -94,7 +133,7 @@ func TestUnregisterMemberWhenAnswerIsN(t *testing.T) {
 	assert.Contains(t, term.Output(), "!!!  DANGER ZONE  !!!")
 	assert.NotContains(t, term.Output(), "THIS COMMAND WILL CAUSE UNREGISTER MEMBER CLUSTER FORM HOST CLUSTER. MAKE SURE THERE IS NO USERS LEFT IN THE MEMBER CLUSTER BEFORE UNREGISTERING IT")
 	assert.Contains(t, term.Output(), "Delete Member cluster stated above from the Host cluster?")
-	assert.NotContains(t, term.Output(), "The deletion of the Toolchain member cluster from the Host cluster has been triggered")
+	assert.NotContains(t, term.Output(), "The deletion of the Member cluster from the Host cluster has been finished.")
 	assert.NotContains(t, term.Output(), "cool-token")
 }
 
@@ -117,7 +156,7 @@ func TestUnregisterMemberWhenNotFound(t *testing.T) {
 	assert.NotContains(t, term.Output(), "!!!  DANGER ZONE  !!!")
 	assert.NotContains(t, term.Output(), "THIS COMMAND WILL CAUSE UNREGISTER MEMBER CLUSTER FORM HOST CLUSTER. MAKE SURE THERE IS NO USERS LEFT IN THE MEMBER CLUSTER BEFORE UNREGISTERING IT")
 	assert.NotContains(t, term.Output(), "Delete Member cluster stated above from the Host cluster?")
-	assert.NotContains(t, term.Output(), "The deletion of the Toolchain member cluster from the Host cluster has been triggered")
+	assert.NotContains(t, term.Output(), "The deletion of the Member cluster from the Host cluster has been finished.")
 	assert.NotContains(t, term.Output(), "cool-token")
 }
 
@@ -141,7 +180,7 @@ func TestUnregisterMemberWhenUnknownClusterName(t *testing.T) {
 	assert.NotContains(t, term.Output(), "!!!  DANGER ZONE  !!!")
 	assert.NotContains(t, term.Output(), "THIS COMMAND WILL CAUSE UNREGISTER MEMBER CLUSTER FORM HOST CLUSTER. MAKE SURE THERE IS NO USERS LEFT IN THE MEMBER CLUSTER BEFORE UNREGISTERING IT")
 	assert.NotContains(t, term.Output(), "Delete Member cluster stated above from the Host cluster?")
-	assert.NotContains(t, term.Output(), "The deletion of the Toolchain member cluster from the Host cluster has been triggered")
+	assert.NotContains(t, term.Output(), "The deletion of the Member cluster from the Host cluster has been finished.")
 	assert.NotContains(t, term.Output(), "cool-token")
 }
 

pkg/test/toolchaincluster.go

@@ -6,6 +6,7 @@ import (
 
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
 	"github.com/codeready-toolchain/toolchain-common/pkg/test"
+	v1 "k8s.io/api/core/v1"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -19,6 +20,11 @@ func NewToolchainCluster(modifiers ...ToolchainClusterModifier) *toolchainv1alph
 			Name:      "member1",
 			Namespace: test.HostOperatorNs,
 		},
+		Spec: toolchainv1alpha1.ToolchainClusterSpec{
+			SecretRef: toolchainv1alpha1.LocalSecretReference{
+				Name: "member1-secret",
+			},
+		},
 		Status: toolchainv1alpha1.ToolchainClusterStatus{
 			APIEndpoint: "https://api.member.com:6443",
 		},
@@ -33,6 +39,9 @@ func AssertToolchainClusterDoesNotExist(t *testing.T, fakeClient *test.FakeClien
 	deletedCluster := &toolchainv1alpha1.ToolchainCluster{}
 	err := fakeClient.Get(context.TODO(), test.NamespacedName(toolchainCluster.Namespace, toolchainCluster.Name), deletedCluster)
 	require.True(t, apierrors.IsNotFound(err), "the ToolchainCluster should be deleted")
+
+	err = fakeClient.Get(context.TODO(), test.NamespacedName(toolchainCluster.Namespace, toolchainCluster.Spec.SecretRef.Name), &v1.Secret{})
+	require.True(t, apierrors.IsNotFound(err), "the ToolchainCluster secret should be deleted")
 }
 
 func AssertToolchainClusterSpec(t *testing.T, fakeClient *test.FakeClient, expectedToolchainCluster *toolchainv1alpha1.ToolchainCluster) {
@@ -47,5 +56,6 @@ type ToolchainClusterModifier func(toolchainCluster *toolchainv1alpha1.Toolchain
 func ToolchainClusterName(name string) ToolchainClusterModifier {
 	return func(toolchainCluster *toolchainv1alpha1.ToolchainCluster) {
 		toolchainCluster.Name = name
+		toolchainCluster.Spec.SecretRef.Name = name + "-secret"
 	}
 }

resources/roles/host.yaml

@@ -263,3 +263,27 @@ objects:
     - "list"
     - "patch"
     - "update"
+
+- kind: Role
+  apiVersion: rbac.authorization.k8s.io/v1
+  metadata:
+    name: unregister-member
+    labels:
+      provider: ksctl
+  rules:
+  - apiGroups:
+    - toolchain.dev.openshift.com
+    resources:
+    - "toolchainclusters"
+    verbs:
+    - "get"
+    - "list"
+    - "delete"
+  - apiGroups:
+    - ""
+    resources:
+    - "secrets"
+    verbs:
+    - "get"
+    - "list"
+    - "delete"