Skip to content

Commit

Permalink
Merge branch 'main' into globalaccountstool-imprv1
Browse files Browse the repository at this point in the history
  • Loading branch information
ukff authored Oct 23, 2024
2 parents fa9b965 + cb4efd8 commit e5e2d6f
Show file tree
Hide file tree
Showing 15 changed files with 314 additions and 75 deletions.
17 changes: 13 additions & 4 deletions cmd/broker/bind_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,21 @@ func TestCreateBindingEndpoint(t *testing.T) {

err = db.Instances().Insert(fixture.FixInstance(instanceID1))
require.NoError(t, err)
operation := fixture.FixOperation("operation-001", instanceID1, internal.OperationTypeProvision)
err = db.Operations().InsertOperation(operation)
require.NoError(t, err)

err = db.Instances().Insert(fixture.FixInstance(instanceID2))
require.NoError(t, err)
operation = fixture.FixOperation("operation-002", instanceID2, internal.OperationTypeProvision)
err = db.Operations().InsertOperation(operation)
require.NoError(t, err)

err = db.Instances().Insert(fixture.FixInstance(instanceID3))
require.NoError(t, err)
operation = fixture.FixOperation("operation-003", instanceID3, internal.OperationTypeProvision)
err = db.Operations().InsertOperation(operation)
require.NoError(t, err)

skrK8sClientProvider := kubeconfig.NewK8sClientFromSecretProvider(kcpClient)

Expand All @@ -190,9 +199,9 @@ 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(), db.Instances(), brokerBindings.NewServiceAccountBindingsManager(skrK8sClientProvider, skrK8sClientProvider))
bindEndpoint := broker.NewBind(*bindingCfg, db, logs, skrK8sClientProvider, skrK8sClientProvider)
getBindingEndpoint := broker.NewGetBinding(logs, db)
unbindEndpoint := broker.NewUnbind(logs, db, brokerBindings.NewServiceAccountBindingsManager(skrK8sClientProvider, skrK8sClientProvider))
apiHandler := handlers.NewApiHandler(broker.KymaEnvironmentBroker{
ServicesEndpoint: nil,
ProvisionEndpoint: nil,
Expand Down Expand Up @@ -459,7 +468,7 @@ func TestCreateBindingEndpoint(t *testing.T) {

})

t.Run("should return error when attempting to add a new binding when the maximum number of bindings has already been reached", func(t *testing.T) {
t.Run("should return error when attempting to add a new binding when the maximum number of non expired bindings has already been reached", func(t *testing.T) {
// given - create max number of bindings
var response *http.Response

Expand Down
165 changes: 155 additions & 10 deletions cmd/broker/binding_test.go
Original file line number Diff line number Diff line change
@@ -1,24 +1,32 @@
package main

import (
"encoding/json"
"fmt"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"io"
"net/http"
"testing"

"github.com/google/uuid"
"github.com/pivotal-cf/brokerapi/v8/domain"
"github.com/stretchr/testify/assert"
)

type ErrorResponse struct {
Description string `json:"description"`
}

func TestBinding(t *testing.T) {
// given
suite := NewBrokerSuiteTest(t)
defer suite.TearDown()
iid := uuid.New().String()
bid := uuid.New().String()

resp := suite.CallAPI("PUT", fmt.Sprintf("oauth/v2/service_instances/%s?accepts_incomplete=true", iid),
resp := suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s?accepts_incomplete=true", iid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15",
Expand All @@ -37,7 +45,7 @@ func TestBinding(t *testing.T) {
suite.WaitForOperationState(opID, domain.Succeeded)

// when
resp = suite.CallAPI("PUT", fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
resp = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15"
Expand All @@ -47,13 +55,13 @@ func TestBinding(t *testing.T) {
suite.Log(string(b))
suite.Log(resp.Status)

respRuntimes := suite.CallAPI("GET", "/runtimes?bindings=true", "")
respRuntimes := suite.CallAPI(http.MethodGet, "/runtimes?bindings=true", "")
b, _ = io.ReadAll(respRuntimes.Body)
suite.Log(string(b))
suite.Log(resp.Status)

t.Run("should return 400 when expiration seconds parameter is string instead of int", func(t *testing.T) {
resp = suite.CallAPI("PUT", fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
resp = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15",
Expand All @@ -62,19 +70,28 @@ func TestBinding(t *testing.T) {
}
}`)
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)

b, err := io.ReadAll(resp.Body)
assert.NoError(t, err)

var apiError ErrorResponse
err = json.Unmarshal(b, &apiError)
assert.NoError(t, err)

assert.Equal(t, "failed to unmarshal parameters: cannot unmarshal string into expiration_seconds of type int", apiError.Description)
})

t.Run("should return 200 when creating a second binding with the same id and params as an existing one", func(t *testing.T) {
bid = uuid.New().String()
resp = suite.CallAPI("PUT", fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
resp = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15",
"parameters": {
"expiration_seconds": 600
}
}`)
resp = suite.CallAPI("PUT", fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
resp = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15",
Expand All @@ -85,7 +102,7 @@ func TestBinding(t *testing.T) {
assert.Equal(t, http.StatusOK, resp.StatusCode)

// "expiration_seconds": 600 is a default value from the config in tests
resp = suite.CallAPI("PUT", fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
resp = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15"
Expand All @@ -95,15 +112,15 @@ func TestBinding(t *testing.T) {

t.Run("should return 409 when creating a second binding with the same id as an existing one but different params", func(t *testing.T) {
bid = uuid.New().String()
resp = suite.CallAPI("PUT", fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
resp = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15",
"parameters": {
"expiration_seconds": 900
}
}`)
resp = suite.CallAPI("PUT", fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
resp = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15",
Expand All @@ -116,3 +133,131 @@ func TestBinding(t *testing.T) {
})

}

func TestDeprovisioningWithExistingBindings(t *testing.T) {
// given
cfg := fixConfig()
// Disable EDP to have all steps successfully executed
cfg.EDP.Disabled = true
suite := NewBrokerSuiteTestWithConfig(t, cfg)
defer suite.TearDown()
iid := uuid.New().String()
bindingID1 := uuid.New().String()
bindingID2 := uuid.New().String()

response := suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s?accepts_incomplete=true", iid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15",
"context": {
"globalaccount_id": "g-account-id",
"subaccount_id": "sub-id",
"user_id": "[email protected]"
},
"parameters": {
"name": "testing-cluster",
"region": "eu-central-1"
}
}`)
opID := suite.DecodeOperationID(response)
suite.processProvisioningByOperationID(opID)
suite.WaitForOperationState(opID, domain.Succeeded)

// when we create two bindings
response = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bindingID1),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15"
}`)
require.Equal(t, http.StatusCreated, response.StatusCode)

response = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bindingID2),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "361c511f-f939-4621-b228-d0fb79a1fe15"
}`)
require.Equal(t, http.StatusCreated, response.StatusCode)

// when we deprovision successfully
response = suite.CallAPI(http.MethodDelete, fmt.Sprintf("oauth/v2/service_instances/%s?accepts_incomplete=true&plan_id=361c511f-f939-4621-b228-d0fb79a1fe15&service_id=47c9dcbf-ff30-448e-ab36-d3bad66ba281", iid),
``)
deprovisioningID := suite.DecodeOperationID(response)
suite.FinishDeprovisioningOperationByProvisioner(deprovisioningID)
suite.WaitForInstanceRemoval(iid)

// when we remove bindings and the instance is already removed
response = suite.CallAPI(http.MethodDelete, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s?plan_id=361c511f-f939-4621-b228-d0fb79a1fe15&service_id=47c9dcbf-ff30-448e-ab36-d3bad66ba281", iid, bindingID1), "")
assert.Equal(t, http.StatusGone, response.StatusCode)
response = suite.CallAPI(http.MethodDelete, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s?plan_id=361c511f-f939-4621-b228-d0fb79a1fe15&service_id=47c9dcbf-ff30-448e-ab36-d3bad66ba281", iid, bindingID2), "")
assert.Equal(t, http.StatusGone, response.StatusCode)

// then expect bindings to be removed
suite.AssertBindingRemoval(iid, bindingID1)
suite.AssertBindingRemoval(iid, bindingID2)
}

func TestRemoveBindingsFromSuspended(t *testing.T) {
// given
cfg := fixConfig()
cfg.Broker.Binding.BindablePlans = []string{"trial"}
suite := NewBrokerSuiteTestWithConfig(t, cfg)
defer suite.TearDown()
iid := uuid.New().String()
bindingID1 := uuid.New().String()
bindingID2 := uuid.New().String()

response := suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/cf-eu10/v2/service_instances/%s?accepts_incomplete=true&plan_id=7d55d31d-35ae-4438-bf13-6ffdfa107d9f&service_id=47c9dcbf-ff30-448e-ab36-d3bad66ba281", iid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "7d55d31d-35ae-4438-bf13-6ffdfa107d9f",
"context": {
"globalaccount_id": "g-account-id",
"subaccount_id": "sub-id",
"user_id": "[email protected]"
},
"parameters": {
"name": "testing-cluster"
}
}`)
opID := suite.DecodeOperationID(response)
suite.processProvisioningByOperationID(opID)
suite.WaitForOperationState(opID, domain.Succeeded)

//then we create two bindings
response = suite.CallAPI(http.MethodPut, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bindingID1),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "7d55d31d-35ae-4438-bf13-6ffdfa107d9f"
}`)
require.Equal(t, http.StatusCreated, response.StatusCode)

response = suite.CallAPI("PUT", fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s", iid, bindingID2),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "7d55d31d-35ae-4438-bf13-6ffdfa107d9f"
}`)
require.Equal(t, http.StatusCreated, response.StatusCode)

//when we suspend Service instance - OSB context update
response = suite.CallAPI(http.MethodPatch, fmt.Sprintf("oauth/cf-eu10/v2/service_instances/%s?accepts_incomplete=true", iid),
`{
"service_id": "47c9dcbf-ff30-448e-ab36-d3bad66ba281",
"plan_id": "7d55d31d-35ae-4438-bf13-6ffdfa107d9f",
"context": {
"globalaccount_id": "g-account-id",
"user_id": "[email protected]",
"active": false
}
}`)
assert.Equal(t, http.StatusOK, response.StatusCode)
suspensionOpID := suite.WaitForLastOperation(iid, domain.InProgress)

suite.FinishDeprovisioningOperationByProvisioner(suspensionOpID)
suite.WaitForOperationState(suspensionOpID, domain.Succeeded)

// when we remove bindings we just return OK
response = suite.CallAPI(http.MethodDelete, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s?plan_id=361c511f-f939-4621-b228-d0fb79a1fe15&service_id=47c9dcbf-ff30-448e-ab36-d3bad66ba281", iid, bindingID1), "")
assert.Equal(t, http.StatusOK, response.StatusCode)
response = suite.CallAPI(http.MethodDelete, fmt.Sprintf("oauth/v2/service_instances/%s/service_bindings/%s?plan_id=361c511f-f939-4621-b228-d0fb79a1fe15&service_id=47c9dcbf-ff30-448e-ab36-d3bad66ba281", iid, bindingID2), "")
assert.Equal(t, http.StatusOK, response.StatusCode)
}
17 changes: 16 additions & 1 deletion cmd/broker/broker_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"testing"
"time"

"github.com/kyma-project/kyma-environment-broker/internal/storage/dberr"

imv1 "github.com/kyma-project/infrastructure-manager/api/v1"

"github.com/kyma-project/kyma-environment-broker/internal/kubeconfig"
Expand Down Expand Up @@ -407,7 +409,7 @@ func (s *BrokerSuiteTest) WaitForOperationState(operationID string, state domain
}
return op.State == state, nil
})
assert.NoError(s.t, err, "timeout waiting for the operation expected state %s != %s. The existing operation %+v", state, op.State, op)
assert.NoError(s.t, err, "timeout waiting for the operation expected state %s. The existing operation %+v", state, op)
}

func (s *BrokerSuiteTest) GetOperation(operationID string) *internal.Operation {
Expand All @@ -431,6 +433,19 @@ func (s *BrokerSuiteTest) WaitForLastOperation(iid string, state domain.LastOper
return op.ID
}

func (s *BrokerSuiteTest) WaitForInstanceRemoval(iid string) {
err := s.poller.Invoke(func() (done bool, err error) {
_, err = s.db.Instances().GetByID(iid)
return dberr.IsNotFound(err), nil
})
assert.NoError(s.t, err, "timeout waiting for the instance %s to be removed", iid)
}

func (s *BrokerSuiteTest) AssertBindingRemoval(iid string, bindingID string) {
_, err := s.db.Bindings().Get(iid, bindingID)
assert.True(s.t, dberr.IsNotFound(err), "bindings should be removed")
}

func (s *BrokerSuiteTest) LastOperation(iid string) *internal.Operation {
op, _ := s.db.Operations().GetLastOperation(iid)
return op
Expand Down
6 changes: 3 additions & 3 deletions cmd/broker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,9 +447,9 @@ func createAPI(router *mux.Router, servicesConfig broker.ServicesConfig, planVal
planDefaults, logs, cfg.KymaDashboardConfig, kcBuilder, convergedCloudRegionProvider, kcpK8sClient),
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(), db.Instances(), brokerBindings.NewServiceAccountBindingsManager(clientProvider, kubeconfigProvider)),
GetBindingEndpoint: broker.NewGetBinding(logs, db.Bindings()),
BindEndpoint: broker.NewBind(cfg.Broker.Binding, db, logs, clientProvider, kubeconfigProvider),
UnbindEndpoint: broker.NewUnbind(logs, db, brokerBindings.NewServiceAccountBindingsManager(clientProvider, kubeconfigProvider)),
GetBindingEndpoint: broker.NewGetBinding(logs, db),
LastBindingOperationEndpoint: broker.NewLastBindingOperation(logs),
}

Expand Down
10 changes: 6 additions & 4 deletions internal/broker/bind_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ type Credentials struct {
Kubeconfig string `json:"kubeconfig"`
}

func NewBind(cfg BindingConfig, instanceStorage storage.Instances, bindingsStorage storage.Bindings, log logrus.FieldLogger, clientProvider broker.ClientProvider, kubeconfigProvider broker.KubeconfigProvider) *BindEndpoint {
func NewBind(cfg BindingConfig, db storage.BrokerStorage, log logrus.FieldLogger, clientProvider broker.ClientProvider, kubeconfigProvider broker.KubeconfigProvider) *BindEndpoint {
return &BindEndpoint{config: cfg,
instancesStorage: instanceStorage,
bindingsStorage: bindingsStorage,
instancesStorage: db.Instances(),
bindingsStorage: db.Bindings(),
log: log.WithField("service", "BindEndpoint"),
serviceAccountBindingManager: broker.NewServiceAccountBindingsManager(clientProvider, kubeconfigProvider),
}
Expand Down Expand Up @@ -114,6 +114,8 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d
err = json.Unmarshal(details.RawParameters, &parameters)
if err != nil {
message := fmt.Sprintf("failed to unmarshal parameters: %s", err)
message = strings.Replace(message, "json: ", "", 1)
message = strings.Replace(message, "Go struct field BindingParams.", "", 1)
return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusBadRequest, message)
}
}
Expand Down Expand Up @@ -170,7 +172,7 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d
}
}
if (bindingCount - expiredCount) >= b.config.MaxBindingsCount {
message := fmt.Sprintf("maximum number of bindings reached: %d", b.config.MaxBindingsCount)
message := fmt.Sprintf("maximum number of non expired bindings reached: %d", b.config.MaxBindingsCount)
return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusBadRequest, message)
}
}
Expand Down
Loading

0 comments on commit e5e2d6f

Please sign in to comment.