Missing Bindings' Resources Removal

cmd/broker/bind_create_test.go

@@ -15,7 +15,10 @@ import (
 
 	"github.com/golang-jwt/jwt/v4"
 	"github.com/google/uuid"
+	brokerBindings "github.com/kyma-project/kyma-environment-broker/internal/broker/bindings"
 	"gopkg.in/yaml.v2"
+	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 
 	"code.cloudfoundry.org/lager"
 	"github.com/gorilla/mux"
@@ -192,7 +195,7 @@ func TestCreateBindingEndpoint(t *testing.T) {
 	//// api handler
 	bindEndpoint := broker.NewBind(*bindingCfg, db.Instances(), db.Bindings(), logs, skrK8sClientProvider, skrK8sClientProvider)
 	getBindingEndpoint := broker.NewGetBinding(logs, db.Bindings())
-	unbindEndpoint := broker.NewUnbind(logs, db.Bindings())
+	unbindEndpoint := broker.NewUnbind(logs, db.Bindings(), db.Instances(), brokerBindings.NewServiceAccountBindingsManager(skrK8sClientProvider, skrK8sClientProvider))
 	apiHandler := handlers.NewApiHandler(broker.KymaEnvironmentBroker{
 		ServicesEndpoint:             nil,
 		ProvisionEndpoint:            nil,
@@ -366,26 +369,39 @@ func TestCreateBindingEndpoint(t *testing.T) {
 		assert.Nil(t, createdBindingIDDB)
 	})
 
-	t.Run("should selectively delete created binding", func(t *testing.T) {
+	t.Run("should selectively delete created binding and its service account resources", func(t *testing.T) {
 		// given
 		instanceFirst := "1"
+
+		//// first instance first binding
 		createdBindingIDInstanceFirstFirst, createdBindingInstanceFirstFirst := createBindingForInstanceWithRandomBindingID(instanceFirst, httpServer, t)
 
 		assertExistsAndKubeconfigCreated(t, createdBindingInstanceFirstFirst, createdBindingIDInstanceFirstFirst, instanceFirst, httpServer, db)
 
+		assertResourcesExistence(t, clientFirst, createdBindingIDInstanceFirstFirst)
+
+		//// first instance second binding
 		createdBindingIDInstanceFirstSecond, createdBindingInstanceFirstSecond := createBindingForInstanceWithRandomBindingID(instanceFirst, httpServer, t)
 
 		assertExistsAndKubeconfigCreated(t, createdBindingInstanceFirstSecond, createdBindingIDInstanceFirstSecond, instanceFirst, httpServer, db)
 
+		assertResourcesExistence(t, clientFirst, createdBindingIDInstanceFirstSecond)
+
+		//// second instance first binding
 		instanceSecond := "2"
 		createdBindingIDInstanceSecondFirst, createdBindingInstanceSecondFirst := createBindingForInstanceWithRandomBindingID(instanceSecond, httpServer, t)
 
 		assertExistsAndKubeconfigCreated(t, createdBindingInstanceSecondFirst, createdBindingIDInstanceSecondFirst, instanceSecond, httpServer, db)
 
+		assertResourcesExistence(t, clientSecond, createdBindingIDInstanceSecondFirst)
+
+		//// second instance second binding
 		createdBindingIDInstanceSecondSecond, createdBindingInstanceSecondSecond := createBindingForInstanceWithRandomBindingID(instanceSecond, httpServer, t)
 
 		assertExistsAndKubeconfigCreated(t, createdBindingInstanceSecondSecond, createdBindingIDInstanceSecondSecond, instanceSecond, httpServer, db)
 
+		assertResourcesExistence(t, clientSecond, createdBindingIDInstanceSecondSecond)
+
 		// when
 		path := fmt.Sprintf(bindingsPath+deleteParams, instanceFirst, createdBindingIDInstanceFirstFirst, "123", fixture.PlanId)
 
@@ -395,12 +411,20 @@ func TestCreateBindingEndpoint(t *testing.T) {
 		// then
 		assert.Equal(t, http.StatusOK, response.StatusCode)
 
+		assertServiceAccountsNotExists(t, clientFirst, createdBindingIDInstanceFirstFirst)
+
 		assertExistsAndKubeconfigCreated(t, createdBindingInstanceFirstSecond, createdBindingIDInstanceFirstSecond, instanceFirst, httpServer, db)
 
+		assertResourcesExistence(t, clientFirst, createdBindingIDInstanceFirstSecond)
+
 		assertExistsAndKubeconfigCreated(t, createdBindingInstanceSecondFirst, createdBindingIDInstanceSecondFirst, instanceSecond, httpServer, db)
 
+		assertResourcesExistence(t, clientSecond, createdBindingIDInstanceSecondFirst)
+
 		assertExistsAndKubeconfigCreated(t, createdBindingInstanceSecondSecond, createdBindingIDInstanceSecondSecond, instanceSecond, httpServer, db)
 
+		assertResourcesExistence(t, clientSecond, createdBindingIDInstanceSecondSecond)
+
 		removedBinding, err := db.Bindings().Get(instanceFirst, createdBindingIDInstanceFirstFirst)
 		assert.Error(t, err)
 		assert.Nil(t, removedBinding)
@@ -426,6 +450,43 @@ func TestCreateBindingEndpoint(t *testing.T) {
 	})
 }
 
+func assertResourcesExistence(t *testing.T, k8sClient client.Client, bindingID string) {
+	name := "kyma-binding-" + bindingID
+	namespace := "kyma-system"
+
+	serviceAccount := corev1.ServiceAccount{}
+	err := k8sClient.Get(context.Background(), client.ObjectKey{Name: name, Namespace: namespace}, &serviceAccount)
+	assert.NoError(t, err)
+	assert.NotNil(t, serviceAccount)
+
+	clusterRole := rbacv1.ClusterRole{}
+	err = k8sClient.Get(context.Background(), client.ObjectKey{Name: name, Namespace: namespace}, &clusterRole)
+	assert.NoError(t, err)
+	assert.NotNil(t, clusterRole)
+
+	clusterRoleBinding := rbacv1.ClusterRoleBinding{}
+	err = k8sClient.Get(context.Background(), client.ObjectKey{Name: name, Namespace: namespace}, &clusterRoleBinding)
+	assert.NoError(t, err)
+	assert.NotNil(t, clusterRoleBinding)
+}
+
+func assertServiceAccountsNotExists(t *testing.T, k8sClient client.Client, bindingID string) {
+	name := "kyma-binding-" + bindingID
+	namespace := "kyma-namespace"
+
+	serviceAccount := corev1.ServiceAccount{}
+	err := k8sClient.Get(context.Background(), client.ObjectKey{Name: name, Namespace: namespace}, &serviceAccount)
+	assert.True(t, apierrors.IsNotFound(err))
+
+	clusterRole := rbacv1.ClusterRole{}
+	err = k8sClient.Get(context.Background(), client.ObjectKey{Name: name, Namespace: namespace}, &clusterRole)
+	assert.True(t, apierrors.IsNotFound(err))
+
+	clusterRoleBinding := rbacv1.ClusterRoleBinding{}
+	err = k8sClient.Get(context.Background(), client.ObjectKey{Name: name, Namespace: namespace}, &clusterRoleBinding)
+	assert.True(t, apierrors.IsNotFound(err))
+}
+
 func TestCreatedBy(t *testing.T) {
 	emptyStr := ""
 	email := "[email protected]"
@@ -481,8 +542,8 @@ func TestCreatedBy(t *testing.T) {
 
 func assertExistsAndKubeconfigCreated(t *testing.T, actual domain.Binding, bindingID, instanceID string, httpServer *httptest.Server, db storage.BrokerStorage) {
 	expected, err := db.Bindings().Get(instanceID, bindingID)
-	assert.NoError(t, err)
-	assert.Equal(t, actual.Credentials.(map[string]interface{})["kubeconfig"], expected.Kubeconfig)
+	require.NoError(t, err)
+	require.Equal(t, actual.Credentials.(map[string]interface{})["kubeconfig"], expected.Kubeconfig)
 }
 
 func assertClusterAccess(t *testing.T, controlSecretName string, binding domain.Binding) {

cmd/broker/main.go

@@ -28,6 +28,7 @@ import (
 	"github.com/kyma-project/kyma-environment-broker/internal"
 	"github.com/kyma-project/kyma-environment-broker/internal/appinfo"
 	"github.com/kyma-project/kyma-environment-broker/internal/broker"
+	brokerBindings "github.com/kyma-project/kyma-environment-broker/internal/broker/bindings"
 	kebConfig "github.com/kyma-project/kyma-environment-broker/internal/config"
 	"github.com/kyma-project/kyma-environment-broker/internal/dashboard"
 	"github.com/kyma-project/kyma-environment-broker/internal/edp"
@@ -446,7 +447,7 @@ func createAPI(router *mux.Router, servicesConfig broker.ServicesConfig, planVal
 		GetInstanceEndpoint:          broker.NewGetInstance(cfg.Broker, db.Instances(), db.Operations(), kcBuilder, logs),
 		LastOperationEndpoint:        broker.NewLastOperation(db.Operations(), db.InstancesArchived(), logs),
 		BindEndpoint:                 broker.NewBind(cfg.Broker.Binding, db.Instances(), db.Bindings(), logs, clientProvider, kubeconfigProvider),
-		UnbindEndpoint:               broker.NewUnbind(logs, db.Bindings()),
+		UnbindEndpoint:               broker.NewUnbind(logs, db.Bindings(), db.Instances(), brokerBindings.NewServiceAccountBindingsManager(clientProvider, kubeconfigProvider)),
 		GetBindingEndpoint:           broker.NewGetBinding(logs, db.Bindings()),
 		LastBindingOperationEndpoint: broker.NewLastBindingOperation(logs),
 	}

internal/broker/bind_delete.go

@@ -2,19 +2,26 @@ package broker
 
 import (
 	"context"
+	"fmt"
+	"net/http"
 
+	broker "github.com/kyma-project/kyma-environment-broker/internal/broker/bindings"
 	"github.com/kyma-project/kyma-environment-broker/internal/storage"
+	"github.com/kyma-project/kyma-environment-broker/internal/storage/dberr"
 	"github.com/pivotal-cf/brokerapi/v8/domain"
+	"github.com/pivotal-cf/brokerapi/v8/domain/apiresponses"
 	"github.com/sirupsen/logrus"
 )
 
 type UnbindEndpoint struct {
-	log             logrus.FieldLogger
-	bindingsStorage storage.Bindings
+	log              logrus.FieldLogger
+	bindingsStorage  storage.Bindings
+	instancesStorage storage.Instances
+	bindingsManager  broker.BindingsManager
 }
 
-func NewUnbind(log logrus.FieldLogger, bindingsStorage storage.Bindings) *UnbindEndpoint {
-	return &UnbindEndpoint{log: log.WithField("service", "UnbindEndpoint"), bindingsStorage: bindingsStorage}
+func NewUnbind(log logrus.FieldLogger, bindingsStorage storage.Bindings, instancesStorage storage.Instances, bindingsManager broker.BindingsManager) *UnbindEndpoint {
+	return &UnbindEndpoint{log: log.WithField("service", "UnbindEndpoint"), bindingsStorage: bindingsStorage, instancesStorage: instancesStorage, bindingsManager: bindingsManager}
 }
 
 // Unbind deletes an existing service binding
@@ -25,10 +32,23 @@ func (b *UnbindEndpoint) Unbind(ctx context.Context, instanceID, bindingID strin
 	b.log.Infof("Unbind details: %+v", details)
 	b.log.Infof("Unbind asyncAllowed: %v", asyncAllowed)
 
-	err := b.bindingsStorage.Delete(instanceID, bindingID)
+	instance, err := b.instancesStorage.GetByID(instanceID)
+	switch {
+	case dberr.IsNotFound(err):
+		return domain.UnbindSpec{}, apiresponses.ErrInstanceDoesNotExist
+	case err != nil:
+		return domain.UnbindSpec{}, apiresponses.NewFailureResponse(fmt.Errorf("failed to get instance %s", instanceID), http.StatusInternalServerError, fmt.Sprintf("failed to get instance %s", instanceID))
+	}
+
+	err = b.bindingsManager.Delete(ctx, instance, bindingID)
+	if err != nil {
+		b.log.Errorf("Unbind error during removal of service account resources: %s", err)
+		return domain.UnbindSpec{}, err
+	}
 
+	err = b.bindingsStorage.Delete(instanceID, bindingID)
 	if err != nil {
-		b.log.Errorf("Unbind error: %s", err)
+		b.log.Errorf("Unbind error during removal of db entity: %s", err)
 		return domain.UnbindSpec{}, err
 	}
 

internal/broker/bindings/bindings_manager.go

@@ -22,6 +22,7 @@ type Credentials struct {
 
 type BindingsManager interface {
 	Create(ctx context.Context, instance *internal.Instance, bindingID string, expirationSeconds int) (string, time.Time, error)
+	Delete(ctx context.Context, instance *internal.Instance, bindingID string) error
 }
 
 type ClientProvider interface {
@@ -52,6 +53,7 @@ func (c *ServiceAccountBindingsManager) Create(ctx context.Context, instance *in
 	}
 
 	serviceBindingName := fmt.Sprintf("kyma-binding-%s", bindingID)
+	fmt.Printf("Creating a service account binding for runtime %s with name %s", instance.RuntimeID, serviceBindingName)
 
 	_, err = clientset.CoreV1().ServiceAccounts("kyma-system").Create(ctx,
 		&v1.ServiceAccount{
@@ -137,3 +139,36 @@ func (c *ServiceAccountBindingsManager) Create(ctx context.Context, instance *in
 
 	return string(kubeconfigContent), expiresAt, nil
 }
+
+func (c *ServiceAccountBindingsManager) Delete(ctx context.Context, instance *internal.Instance, bindingID string) error {
+	clientset, err := c.clientProvider.K8sClientSetForRuntimeID(instance.RuntimeID)
+
+	if err != nil {
+		return fmt.Errorf("while creating a runtime client for binding creation: %v", err)
+	}
+
+	serviceBindingName := fmt.Sprintf("kyma-binding-%s", bindingID)
+
+	// remove a binding
+	err = clientset.RbacV1().ClusterRoleBindings().Delete(ctx, serviceBindingName, mv1.DeleteOptions{})
+
+	if err != nil && !apierrors.IsNotFound(err) {
+		return fmt.Errorf("while removing a cluster role binding: %v", err)
+	}
+
+	// remove a role
+	err = clientset.RbacV1().ClusterRoles().Delete(ctx, serviceBindingName, mv1.DeleteOptions{})
+
+	if err != nil && !apierrors.IsNotFound(err) {
+		return fmt.Errorf("while removing a cluster role: %v", err)
+	}
+
+	// remove an account
+	err = clientset.CoreV1().ServiceAccounts("kyma-system").Delete(ctx, serviceBindingName, mv1.DeleteOptions{})
+
+	if err != nil && !apierrors.IsNotFound(err) {
+		return fmt.Errorf("while creating a service account: %v", err)
+	}
+
+	return nil
+}