fix: unregister-member deletes secret & add needed permissions (#99)

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,28 @@ 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
+		}
+		ctx.Printlnf("\nThe referenced ToolchainCluster secret %s, is not present.", toolchainCluster.Spec.SecretRef.Name)
+	} 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,18 +36,50 @@ 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 TestUnregisterMemberWhenRestartError(t *testing.T) {
+func TestUnregisterMemberWhenSecretIsMissing(t *testing.T) {
 	// given
 	toolchainCluster := NewToolchainCluster(ToolchainClusterName("member-cool-server.com"))
+	newClient, fakeClient := NewFakeClients(t, toolchainCluster)
 
-	newClient, _ := 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.Contains(t, term.Output(), "The referenced ToolchainCluster secret member-cool-server.com-secret, is not present.")
+	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"))
+	secret := newSecret(toolchainCluster)
+	newClient, _ := NewFakeClients(t, toolchainCluster, secret)
 
 	SetFileConfig(t, Host(), Member())
 	term := NewFakeTerminalWithResponse("y")
@@ -57,8 +97,8 @@ func TestUnregisterMemberWhenRestartError(t *testing.T) {
 func TestUnregisterMemberCallsRestart(t *testing.T) {
 	// given
 	toolchainCluster := NewToolchainCluster(ToolchainClusterName("member-cool-server.com"))
-
-	newClient, _ := NewFakeClients(t, toolchainCluster)
+	secret := newSecret(toolchainCluster)
+	newClient, _ := NewFakeClients(t, toolchainCluster, secret)
 
 	SetFileConfig(t, Host(), Member())
 	term := NewFakeTerminalWithResponse("y")
@@ -78,7 +118,8 @@ func TestUnregisterMemberCallsRestart(t *testing.T) {
 func TestUnregisterMemberWhenAnswerIsN(t *testing.T) {
 	// given
 	toolchainCluster := NewToolchainCluster(ToolchainClusterName("member-cool-server.com"))
-	newClient, fakeClient := NewFakeClients(t, toolchainCluster)
+	secret := newSecret(toolchainCluster)
+	newClient, fakeClient := NewFakeClients(t, toolchainCluster, secret)
 	SetFileConfig(t, Host(), Member())
 	term := NewFakeTerminalWithResponse("n")
 	ctx := clicontext.NewCommandContext(term, newClient)
@@ -91,17 +132,19 @@ func TestUnregisterMemberWhenAnswerIsN(t *testing.T) {
 	// then
 	require.NoError(t, err)
 	AssertToolchainClusterSpec(t, fakeClient, toolchainCluster)
+	AssertObjectExists(t, fakeClient, client.ObjectKeyFromObject(secret), &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.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")
 }
 
 func TestUnregisterMemberWhenNotFound(t *testing.T) {
 	// given
 	toolchainCluster := NewToolchainCluster(ToolchainClusterName("another-cool-server.com"))
-	newClient, fakeClient := NewFakeClients(t, toolchainCluster)
+	secret := newSecret(toolchainCluster)
+	newClient, fakeClient := NewFakeClients(t, toolchainCluster, secret)
 	SetFileConfig(t, Host(), Member())
 	term := NewFakeTerminalWithResponse("n")
 	ctx := clicontext.NewCommandContext(term, newClient)
@@ -114,17 +157,19 @@ func TestUnregisterMemberWhenNotFound(t *testing.T) {
 	// then
 	require.EqualError(t, err, "toolchainclusters.toolchain.dev.openshift.com \"member-cool-server.com\" not found")
 	AssertToolchainClusterSpec(t, fakeClient, toolchainCluster)
+	AssertObjectExists(t, fakeClient, client.ObjectKeyFromObject(secret), &v1.Secret{})
 	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")
 }
 
 func TestUnregisterMemberWhenUnknownClusterName(t *testing.T) {
 	// given
 	toolchainCluster := NewToolchainCluster(ToolchainClusterName("member-cool-server.com"))
-	newClient, fakeClient := NewFakeClients(t, toolchainCluster)
+	secret := newSecret(toolchainCluster)
+	newClient, fakeClient := NewFakeClients(t, toolchainCluster, secret)
 	SetFileConfig(t, Host(), Member())
 	term := NewFakeTerminalWithResponse("n")
 	ctx := clicontext.NewCommandContext(term, newClient)
@@ -138,10 +183,11 @@ func TestUnregisterMemberWhenUnknownClusterName(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "the provided cluster-name 'some' is not present in your ksctl.yaml file.")
 	AssertToolchainClusterSpec(t, fakeClient, toolchainCluster)
+	AssertObjectExists(t, fakeClient, client.ObjectKeyFromObject(secret), &v1.Secret{})
 	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")
 }
 
@@ -150,7 +196,8 @@ func TestUnregisterMemberLacksPermissions(t *testing.T) {
 	SetFileConfig(t, Host(NoToken()), Member(NoToken()))
 
 	toolchainCluster := NewToolchainCluster(ToolchainClusterName("member-cool-server.com"))
-	newClient, fakeClient := NewFakeClients(t, toolchainCluster)
+	secret := newSecret(toolchainCluster)
+	newClient, fakeClient := NewFakeClients(t, toolchainCluster, secret)
 	term := NewFakeTerminal()
 	ctx := clicontext.NewCommandContext(term, newClient)
 
@@ -162,6 +209,7 @@ func TestUnregisterMemberLacksPermissions(t *testing.T) {
 	// then
 	require.EqualError(t, err, "ksctl command failed: the token in your ksctl.yaml file is missing")
 	AssertToolchainClusterSpec(t, fakeClient, toolchainCluster)
+	AssertObjectExists(t, fakeClient, client.ObjectKeyFromObject(secret), &v1.Secret{})
 }
 
 func mockRestart(ctx *clicontext.CommandContext, clusterName string, cfc ConfigFlagsAndClientGetterFunc) error {

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"