Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial Bindings Delete API Implementation #1305

Merged
merged 11 commits into from
Oct 14, 2024
2 changes: 1 addition & 1 deletion cmd/broker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,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, gardenerClient),
UnbindEndpoint: broker.NewUnbind(logs),
UnbindEndpoint: broker.NewUnbind(logs, db.Bindings()),
GetBindingEndpoint: broker.NewGetBinding(logs, db.Bindings()),
LastBindingOperationEndpoint: broker.NewLastBindingOperation(logs),
}
Expand Down
42 changes: 33 additions & 9 deletions internal/broker/bind_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ func TestCreateBindingEndpoint(t *testing.T) {
//// api handler
bindEndpoint := NewBind(*bindingCfg, db.Instances(), db.Bindings(), logs, skrK8sClientProvider, skrK8sClientProvider, gardenerClient)
getBindingEndpoint := NewGetBinding(logs, db.Bindings())
unbindEndpoint := NewUnbind(logs, db.Bindings())
apiHandler := handlers.NewApiHandler(KymaEnvironmentBroker{
nil,
nil,
Expand All @@ -172,7 +173,7 @@ func TestCreateBindingEndpoint(t *testing.T) {
nil,
nil,
bindEndpoint,
nil,
unbindEndpoint,
getBindingEndpoint,
nil,
}, brokerLogger)
Expand All @@ -181,6 +182,7 @@ func TestCreateBindingEndpoint(t *testing.T) {
router := mux.NewRouter()
router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.Bind).Methods(http.MethodPut)
router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.GetBinding).Methods(http.MethodGet)
router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.Unbind).Methods(http.MethodDelete)
httpServer := httptest.NewServer(router)
defer httpServer.Close()

Expand Down Expand Up @@ -208,8 +210,8 @@ func TestCreateBindingEndpoint(t *testing.T) {
assert.Equal(t, expirationSeconds*time.Second, duration)

//// verify connectivity using kubeconfig from the generated binding
assertClusterAccess(t, response, "secret-to-check-first", binding)
assertRolesExistence(t, response, "kyma-binding-binding-id", binding)
assertClusterAccess(t, "secret-to-check-first", binding)
assertRolesExistence(t, "kyma-binding-binding-id", binding)
})

t.Run("should create a new service binding with custom token expiration time", func(t *testing.T) {
Expand Down Expand Up @@ -322,7 +324,7 @@ func TestCreateBindingEndpoint(t *testing.T) {
assert.Equal(t, expirationSeconds*time.Second, duration)

//// verify connectivity using kubeconfig from the generated binding
assertClusterAccess(t, response, "secret-to-check-first", binding)
assertClusterAccess(t, "secret-to-check-first", binding)
})

t.Run("should return created bindings when multiple bindings created", func(t *testing.T) {
Expand Down Expand Up @@ -352,7 +354,7 @@ func TestCreateBindingEndpoint(t *testing.T) {
binding := unmarshal(t, response)
assert.Equal(t, firstInstancefirstBinding, binding)
assert.Equal(t, firstInstanceFirstBindingDB.Kubeconfig, binding.Credentials.(map[string]interface{})["kubeconfig"])
assertClusterAccess(t, response, "secret-to-check-first", binding)
assertClusterAccess(t, "secret-to-check-first", binding)

// when - binding to the second instance
path = fmt.Sprintf("v2/service_instances/%s/service_bindings/%s?accepts_incomplete=false", instanceIDSecond, secondInstanceBindingID)
Expand All @@ -364,7 +366,7 @@ func TestCreateBindingEndpoint(t *testing.T) {
binding = unmarshal(t, response)
assert.Equal(t, secondInstanceFirstBinding, binding)
assert.Equal(t, secondInstanceFirstBindingDB.Kubeconfig, binding.Credentials.(map[string]interface{})["kubeconfig"])
assertClusterAccess(t, response, "secret-to-check-second", binding)
assertClusterAccess(t, "secret-to-check-second", binding)

// when - second binding to the first instance
path = fmt.Sprintf("v2/service_instances/%s/service_bindings/%s?accepts_incomplete=false", instanceIDFirst, firstInstanceSecondBindingID)
Expand All @@ -376,11 +378,33 @@ func TestCreateBindingEndpoint(t *testing.T) {
binding = unmarshal(t, response)
assert.Equal(t, firstInstanceSecondBinding, binding)
assert.Equal(t, firstInstanceSecondBindingDB.Kubeconfig, binding.Credentials.(map[string]interface{})["kubeconfig"])
assertClusterAccess(t, response, "secret-to-check-first", binding)
assertClusterAccess(t, "secret-to-check-first", binding)
})

t.Run("should delete created binding", func(t *testing.T) {
jaroslaw-pieszka marked this conversation as resolved.
Show resolved Hide resolved
// given
instanceID := "1"
createdBindingID, createdBinding := createBindingForInstance(instanceID, httpServer, t)
jaroslaw-pieszka marked this conversation as resolved.
Show resolved Hide resolved
createdBindingIDDB, err := db.Bindings().Get(instanceID, createdBindingID)
assert.NoError(t, err)
assert.Equal(t, createdBinding.Credentials.(map[string]interface{})["kubeconfig"], createdBindingIDDB.Kubeconfig)

// when
path := fmt.Sprintf("v2/service_instances/%s/service_bindings/%s?accepts_incomplete=false&service_id=%s&plan_id=%s", instanceID, createdBindingID, "123", fixture.PlanId)
jaroslaw-pieszka marked this conversation as resolved.
Show resolved Hide resolved

response := CallAPI(httpServer, http.MethodDelete, path, "", t)
defer response.Body.Close()

// then
assert.Equal(t, http.StatusOK, response.StatusCode)
createdBindingIDDB, err = db.Bindings().Get(instanceID, createdBindingID)
assert.Error(t, err)
jaroslaw-pieszka marked this conversation as resolved.
Show resolved Hide resolved
assert.Nil(t, createdBindingIDDB)
})

}

func assertClusterAccess(t *testing.T, response *http.Response, controlSecretName string, binding domain.Binding) {
func assertClusterAccess(t *testing.T, controlSecretName string, binding domain.Binding) {

credentials, ok := binding.Credentials.(map[string]interface{})
require.True(t, ok)
Expand All @@ -392,7 +416,7 @@ func assertClusterAccess(t *testing.T, response *http.Response, controlSecretNam
assert.NoError(t, err)
}

func assertRolesExistence(t *testing.T, response *http.Response, bindingID string, binding domain.Binding) {
func assertRolesExistence(t *testing.T, bindingID string, binding domain.Binding) {

credentials, ok := binding.Credentials.(map[string]interface{})
require.True(t, ok)
Expand Down
15 changes: 12 additions & 3 deletions internal/broker/bind_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,18 @@ package broker
import (
"context"

"github.com/kyma-project/kyma-environment-broker/internal/storage"
"github.com/pivotal-cf/brokerapi/v8/domain"
"github.com/sirupsen/logrus"
)

type UnbindEndpoint struct {
log logrus.FieldLogger
log logrus.FieldLogger
bindingsStorage storage.Bindings
}

func NewUnbind(log logrus.FieldLogger) *UnbindEndpoint {
return &UnbindEndpoint{log: log.WithField("service", "UnbindEndpoint")}
func NewUnbind(log logrus.FieldLogger, bindingsStorage storage.Bindings) *UnbindEndpoint {
return &UnbindEndpoint{log: log.WithField("service", "UnbindEndpoint"), bindingsStorage: bindingsStorage}
}

// Unbind deletes an existing service binding
Expand All @@ -23,6 +25,13 @@ 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)

if err != nil {
b.log.Errorf("Unbind error: %s", err)
return domain.UnbindSpec{}, err
}

return domain.UnbindSpec{
IsAsync: false,
}, nil
Expand Down
2 changes: 1 addition & 1 deletion internal/storage/driver/memory/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (s *Binding) Insert(binding *internal.Binding) error {
return nil
}

func (s *Binding) DeleteByBindingID(bindingID string) error {
func (s *Binding) Delete(instanceID, bindingID string) error {
s.mu.Lock()
defer s.mu.Unlock()

Expand Down
4 changes: 2 additions & 2 deletions internal/storage/driver/postsql/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ func (s *Binding) Insert(binding *internal.Binding) error {
return nil
}

func (s *Binding) DeleteByBindingID(ID string) error {
func (s *Binding) Delete(instanceID, bindingID string) error {
sess := s.NewWriteSession()
return sess.DeleteBinding(ID)
return sess.DeleteBinding(instanceID, bindingID)
}

func (s *Binding) ListByInstanceID(instanceID string) ([]internal.Binding, error) {
Expand Down
9 changes: 5 additions & 4 deletions internal/storage/driver/postsql/binding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ func TestBinding(t *testing.T) {
assert.NoError(t, err)

// when
createdBinding, err := brokerStorage.Bindings().Get("instance-"+testBindingId, testBindingId)
testInstanceID := "instance-" + testBindingId
createdBinding, err := brokerStorage.Bindings().Get(testInstanceID, testBindingId)

// then
assert.NoError(t, err)
Expand All @@ -42,7 +43,7 @@ func TestBinding(t *testing.T) {
assert.Equal(t, fixedBinding.Kubeconfig, createdBinding.Kubeconfig)

// when
err = brokerStorage.Bindings().DeleteByBindingID(testBindingId)
err = brokerStorage.Bindings().Delete(testInstanceID, testBindingId)

// then
nonExisting, err := brokerStorage.Bindings().Get("instance-"+testBindingId, testBindingId)
Expand Down Expand Up @@ -89,11 +90,11 @@ func TestBinding(t *testing.T) {
err = brokerStorage.Bindings().Insert(&fixedBinding)
assert.NoError(t, err)

err = brokerStorage.Bindings().DeleteByBindingID(fixedBinding.ID)
err = brokerStorage.Bindings().Delete(fixedBinding.InstanceID, fixedBinding.ID)
assert.NoError(t, err)

// then
err = brokerStorage.Bindings().DeleteByBindingID(fixedBinding.ID)
err = brokerStorage.Bindings().Delete(fixedBinding.InstanceID, fixedBinding.ID)
assert.NoError(t, err)
})

Expand Down
2 changes: 1 addition & 1 deletion internal/storage/ext.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,5 @@ type Bindings interface {
Insert(binding *internal.Binding) error
Get(instanceID string, bindingID string) (*internal.Binding, error)
ListByInstanceID(instanceID string) ([]internal.Binding, error)
DeleteByBindingID(bindingID string) error
Delete(instanceID, bindingID string) error
}
2 changes: 1 addition & 1 deletion internal/storage/postsql/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ type WriteSession interface {
DeleteOperationByID(operationID string) dberr.Error
InsertInstanceArchived(instance dbmodel.InstanceArchivedDTO) dberr.Error
InsertBinding(binding dbmodel.BindingDTO) dberr.Error
DeleteBinding(ID string) dberr.Error
DeleteBinding(instanceID, bindingID string) dberr.Error
}

type Transaction interface {
Expand Down
5 changes: 3 additions & 2 deletions internal/storage/postsql/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ type writeSession struct {
transaction *dbr.Tx
}

func (ws writeSession) DeleteBinding(ID string) dberr.Error {
func (ws writeSession) DeleteBinding(instanceID, bindingID string) dberr.Error {
_, err := ws.deleteFrom(BindingsTableName).
Where(dbr.Eq("id", ID)).
Where(dbr.Eq("id", bindingID)).
Where(dbr.Eq("instance_id", instanceID)).
Exec()

if err != nil {
Expand Down
Loading