diff --git a/internal/broker/bind_create.go b/internal/broker/bind_create.go index 5eae2147aa..ca52dd838b 100644 --- a/internal/broker/bind_create.go +++ b/internal/broker/bind_create.go @@ -178,7 +178,6 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d } var kubeconfig string - var expiresAt time.Time binding := &internal.Binding{ ID: bindingID, InstanceID: instanceID, @@ -187,17 +186,9 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d UpdatedAt: time.Now(), ExpirationSeconds: int64(expirationSeconds), + ExpiresAt: time.Now().Add(time.Duration(expirationSeconds) * time.Second), CreatedBy: bindingContext.CreatedBy(), } - // get kubeconfig for the instance - kubeconfig, expiresAt, err = b.serviceAccountBindingManager.Create(ctx, instance, bindingID, expirationSeconds) - if err != nil { - message := fmt.Sprintf("failed to create a Kyma binding using service account's kubeconfig: %s", err) - return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusBadRequest, message) - } - - binding.ExpiresAt = expiresAt - binding.Kubeconfig = kubeconfig err = b.bindingsStorage.Insert(binding) switch { @@ -207,7 +198,23 @@ func (b *BindEndpoint) Bind(ctx context.Context, instanceID, bindingID string, d case err != nil: message := fmt.Sprintf("failed to insert Kyma binding into storage: %s", err) return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusInternalServerError, message) + } + + // create kubeconfig for the instance + var expiresAt time.Time + kubeconfig, expiresAt, err = b.serviceAccountBindingManager.Create(ctx, instance, bindingID, expirationSeconds) + if err != nil { + message := fmt.Sprintf("failed to create a Kyma binding using service account's kubeconfig: %s", err) + return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusBadRequest, message) + } + binding.ExpiresAt = expiresAt + binding.Kubeconfig = kubeconfig + + err = b.bindingsStorage.Update(binding) + if err != nil { + message := fmt.Sprintf("failed to update Kyma binding in storage: %s", err) + return domain.Binding{}, apiresponses.NewFailureResponse(fmt.Errorf(message), http.StatusInternalServerError, message) } return domain.Binding{ diff --git a/internal/broker/bind_create_test.go b/internal/broker/bind_create_test.go index dfdb807c53..6901a2dd29 100644 --- a/internal/broker/bind_create_test.go +++ b/internal/broker/bind_create_test.go @@ -3,16 +3,120 @@ package broker import ( "context" "encoding/json" + "fmt" + "net/http/httptest" "testing" + "time" "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "k8s.io/client-go/kubernetes" + + "code.cloudfoundry.org/lager" "github.com/kyma-project/kyma-environment-broker/internal/fixture" "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/sirupsen/logrus" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type Kubeconfig struct { + Users []User `yaml:"users"` +} + +type User struct { + Name string `yaml:"name"` + User struct { + Token string `yaml:"token"` + } `yaml:"user"` +} + +const ( + instanceID1 = "1" + instanceID2 = "2" + instanceID3 = "max-bindings" + maxBindingsCount = 10 ) +var httpServer *httptest.Server + +type provider struct { +} + +func (p *provider) K8sClientSetForRuntimeID(runtimeID string) (kubernetes.Interface, error) { + return nil, fmt.Errorf("error") +} + +func (p *provider) KubeconfigForRuntimeID(runtimeID string) ([]byte, error) { + return []byte{}, nil +} + +func TestCreateBindingEndpoint(t *testing.T) { + t.Log("test create binding endpoint") + + // Given + //// logger + logs := logrus.New() + logs.SetLevel(logrus.DebugLevel) + logs.SetFormatter(&logrus.JSONFormatter{ + TimestampFormat: time.RFC3339Nano, + }) + + brokerLogger := lager.NewLogger("test") + brokerLogger.RegisterSink(lager.NewWriterSink(logs.Writer(), lager.DEBUG)) + + //// schema + + //// database + db := storage.NewMemoryStorage() + + err := db.Instances().Insert(fixture.FixInstance(instanceID1)) + require.NoError(t, err) + + err = db.Instances().Insert(fixture.FixInstance(instanceID2)) + require.NoError(t, err) + + err = db.Instances().Insert(fixture.FixInstance(instanceID3)) + require.NoError(t, err) + + //// binding configuration + bindingCfg := &BindingConfig{ + Enabled: true, + BindablePlans: EnablePlans{ + fixture.PlanName, + }, + MaxBindingsCount: maxBindingsCount, + } + + //// api handler + bindEndpoint := NewBind(*bindingCfg, db, logs, &provider{}, &provider{}) + + // test relies on checking if got nil on kubeconfig provider but the instance got inserted either way + t.Run("should INSERT binding despite error on k8s api call", func(t *testing.T) { + // given + _, err := db.Bindings().Get(instanceID1, "binding-id") + require.Error(t, err) + require.True(t, dberr.IsNotFound(err)) + + // when + _, err = bindEndpoint.Bind(context.Background(), instanceID1, "binding-id", domain.BindDetails{ + ServiceID: "123", + PlanID: fixture.PlanId, + }, false) + + require.Error(t, err) + + binding, err := db.Bindings().Get(instanceID1, "binding-id") + require.NoError(t, err) + require.Equal(t, instanceID1, binding.InstanceID) + require.Equal(t, "binding-id", binding.ID) + + require.NotNil(t, binding.ExpiresAt) + require.Empty(t, binding.Kubeconfig) + }) +} + func TestCreatedBy(t *testing.T) { emptyStr := "" email := "john.smith@email.com" diff --git a/internal/storage/driver/memory/binding.go b/internal/storage/driver/memory/binding.go index 678007ee5b..70a7398e0f 100644 --- a/internal/storage/driver/memory/binding.go +++ b/internal/storage/driver/memory/binding.go @@ -32,8 +32,20 @@ func (s *Binding) Insert(binding *internal.Binding) error { s.mu.Lock() defer s.mu.Unlock() - if _, found := s.data[binding.ID]; found { - return dberr.AlreadyExists("binding with id %s already exist", binding.ID) + if foundBinding, found := s.data[binding.ID]; found && binding.InstanceID == foundBinding.InstanceID { + return dberr.AlreadyExists("binding with id %s already exists", binding.ID) + } + s.data[binding.ID] = *binding + + return nil +} + +func (s *Binding) Update(binding *internal.Binding) error { + s.mu.Lock() + defer s.mu.Unlock() + + if foundBinding, found := s.data[binding.ID]; !(found && binding.InstanceID == foundBinding.InstanceID) { + return dberr.AlreadyExists("binding with id %s does not exist", binding.ID) } s.data[binding.ID] = *binding diff --git a/internal/storage/driver/postsql/binding.go b/internal/storage/driver/postsql/binding.go index f69dff73ba..128c78c42b 100644 --- a/internal/storage/driver/postsql/binding.go +++ b/internal/storage/driver/postsql/binding.go @@ -60,6 +60,22 @@ func (s *Binding) Insert(binding *internal.Binding) error { return nil } +func (s *Binding) Update(binding *internal.Binding) error { + dto, err := s.toBindingDTO(binding) + if err != nil { + return err + } + + sess := s.NewWriteSession() + err = sess.UpdateBinding(dto) + + if err != nil { + return fmt.Errorf("while updating binding with ID %s: %w", binding.ID, err) + } + + return nil +} + func (s *Binding) Delete(instanceID, bindingID string) error { sess := s.NewWriteSession() return sess.DeleteBinding(instanceID, bindingID) diff --git a/internal/storage/ext.go b/internal/storage/ext.go index 7b82921e12..5759da6a56 100644 --- a/internal/storage/ext.go +++ b/internal/storage/ext.go @@ -130,6 +130,7 @@ type SubaccountStates interface { type Bindings interface { Insert(binding *internal.Binding) error + Update(binding *internal.Binding) error Get(instanceID string, bindingID string) (*internal.Binding, error) ListByInstanceID(instanceID string) ([]internal.Binding, error) Delete(instanceID, bindingID string) error diff --git a/internal/storage/postsql/factory.go b/internal/storage/postsql/factory.go index 3969b472a4..394671372d 100644 --- a/internal/storage/postsql/factory.go +++ b/internal/storage/postsql/factory.go @@ -83,6 +83,7 @@ type WriteSession interface { DeleteOperationByID(operationID string) dberr.Error InsertInstanceArchived(instance dbmodel.InstanceArchivedDTO) dberr.Error InsertBinding(binding dbmodel.BindingDTO) dberr.Error + UpdateBinding(binding dbmodel.BindingDTO) dberr.Error DeleteBinding(instanceID, bindingID string) dberr.Error } diff --git a/internal/storage/postsql/write.go b/internal/storage/postsql/write.go index 25beca7966..ac7503cc74 100644 --- a/internal/storage/postsql/write.go +++ b/internal/storage/postsql/write.go @@ -56,6 +56,21 @@ func (ws writeSession) InsertBinding(binding dbmodel.BindingDTO) dberr.Error { return nil } +func (ws writeSession) UpdateBinding(binding dbmodel.BindingDTO) dberr.Error { + _, err := ws.update(BindingsTableName). + Set("kubeconfig", binding.Kubeconfig). + Set("expires_at", binding.ExpiresAt). + Where(dbr.Eq("id", binding.ID)). + Where(dbr.Eq("instance_id", binding.InstanceID)). + Exec() + + if err != nil { + return dberr.Internal("Failed to update record to Binding table: %s", err) + } + + return nil +} + func (ws writeSession) InsertInstanceArchived(instance dbmodel.InstanceArchivedDTO) dberr.Error { _, err := ws.insertInto(InstancesArchivedTableName). Pair("instance_id", instance.InstanceID).