Skip to content

Commit

Permalink
Fix create Service Binding when secret exists (#826)
Browse files Browse the repository at this point in the history
* Block SB creation when secret exists

* Add unit test for POST SB when secret exists

* go mod tidy
  • Loading branch information
szwedm authored Aug 21, 2024
1 parent 1ea1802 commit 95a81d7
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 5 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ require (
github.com/prometheus/common v0.53.0 // indirect
github.com/prometheus/procfs v0.14.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/stretchr/objx v0.5.2 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/exp v0.0.0-20240416160154-fe59bbe5cc7f // indirect
golang.org/x/net v0.25.0 // indirect
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,6 @@ github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUz
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
Expand Down
22 changes: 22 additions & 0 deletions internal/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
servicemanager "github.com/kyma-project/btp-manager/internal/service-manager"
"github.com/kyma-project/btp-manager/internal/service-manager/types"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -223,6 +224,16 @@ func (a *API) CreateServiceBinding(writer http.ResponseWriter, request *http.Req
a.handleError(writer, err)
return
}
secretExists, err := a.secretExists(serviceBindingRequest.SecretName, serviceBindingRequest.SecretNamespace)
if err != nil {
a.handleError(writer, err)
return
}
if secretExists {
secretExistsErr := fmt.Errorf("secret \"%s\" in \"%s\" namespace already exists", serviceBindingRequest.SecretName, serviceBindingRequest.SecretNamespace)
a.handleError(writer, secretExistsErr, http.StatusConflict)
return
}
si, err := a.smClient.ServiceInstance(serviceBindingRequest.ServiceInstanceID)
if err != nil {
a.handleError(writer, err)
Expand Down Expand Up @@ -439,6 +450,17 @@ func (a *API) ServiceBindingsSecrets(sbs *types.ServiceBindings) responses.Servi
return serviceBindingsSecrets
}

func (a *API) secretExists(secretName, secretNamespace string) (bool, error) {
existingSecret, err := a.secretManager.GetByNameAndNamespace(context.Background(), secretName, secretNamespace)
if err != nil {
if k8serrors.IsNotFound(err) {
return false, nil
}
return false, err
}
return existingSecret != nil && existingSecret.Name == secretName && existingSecret.Namespace == secretNamespace, nil
}

func generateSecretFromSISBData(si *types.ServiceInstance, sb *types.ServiceBinding, createSBRequest *requests.CreateServiceBinding) (*corev1.Secret, error) {
var secretName, secretNamespace string
var err error
Expand Down
41 changes: 41 additions & 0 deletions internal/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
"log/slog"
"net/http"
"testing"
Expand Down Expand Up @@ -555,6 +557,45 @@ func TestAPI(t *testing.T) {
fakeSM.RespondWithData()
})

t.Run("POST Service Binding 409 error", func(t *testing.T) {
// given
secretName, secretNamespace := "sb-test-01-secret", "default"
sbCreateRequest := requests.CreateServiceBinding{
Name: "sb-test-01",
ServiceInstanceID: "a7e240d6-e348-4fc0-a54c-7b7bfe9b9da6",
Parameters: []byte(`{"param1": "value1", "param2": "value2"}`),
SecretName: secretName,
SecretNamespace: secretNamespace,
}
sbCreateRequestJSON, err := json.Marshal(sbCreateRequest)
require.NoError(t, err)

existingSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: secretNamespace,
},
StringData: map[string]string{"foo": "bar"},
}
require.NoError(t, secretMgr.Create(context.TODO(), existingSecret))

req, err := http.NewRequest(http.MethodPost, apiAddr+"/api/service-bindings", bytes.NewBuffer(sbCreateRequestJSON))
require.NoError(t, err)
resp, err := apiClient.Do(req)
require.NoError(t, err)

defer resp.Body.Close()
msgBytes, err := io.ReadAll(resp.Body)
require.NoError(t, err)

// then
require.Equal(t, http.StatusConflict, resp.StatusCode)
assert.Contains(t, fmt.Sprintf("secret \"%s\" in \"%s\" namespace already exists", secretName, secretNamespace), string(msgBytes))

// cleanup
err = secretMgr.Delete(context.TODO(), existingSecret)
})

t.Run("DELETE Service Binding by ID", func(t *testing.T) {
// given
sbID := "318a16c3-7c80-485f-b55c-918629012c9a"
Expand Down
3 changes: 2 additions & 1 deletion internal/cluster-object/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -37,7 +38,7 @@ func (p *FakeSecretManager) GetByNameAndNamespace(ctx context.Context, name, nam
return secret, nil
}
}
return nil, fmt.Errorf("secret not found")
return nil, errors.NewNotFound(corev1.Resource("secret"), name)
}

func (p *FakeSecretManager) Clean() {
Expand Down
2 changes: 1 addition & 1 deletion internal/cluster-object/secret_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (p *SecretManager) GetByNameAndNamespace(ctx context.Context, name, namespa
secret := &corev1.Secret{}
if err := p.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, secret); err != nil {
if k8serrors.IsNotFound(err) {
p.logger.Warn(fmt.Sprintf("secret \"%s\" not found in \"%s\" namespace", name, namespace))
p.logger.Info(fmt.Sprintf("secret \"%s\" not found in \"%s\" namespace", name, namespace))
return nil, err
}
p.logger.Error(fmt.Sprintf("failed to fetch \"%s\" secret in \"%s\" namespace", name, namespace), "error", err)
Expand Down

0 comments on commit 95a81d7

Please sign in to comment.