Add flag to transfer API/disable delete across platforms (#490)

api/api.go

@@ -49,25 +49,27 @@ const osbVersion = "2.13"
 
 // Settings type to be loaded from the environment
 type Settings struct {
-	TokenIssuerURL  string   `mapstructure:"token_issuer_url" description:"url of the token issuer which to use for validating tokens"`
-	ClientID        string   `mapstructure:"client_id" description:"id of the client from which the token must be issued"`
-	TokenBasicAuth  bool     `mapstructure:"token_basic_auth" description:"specifies if client credentials to the authorization server should be sent in the header as basic auth (true) or in the body (false)"`
-	ProtectedLabels []string `mapstructure:"protected_labels" description:"defines labels which cannot be modified/added by REST API requests"`
-	OSBVersion      string   `mapstructure:"-"`
-	MaxPageSize     int      `mapstructure:"max_page_size" description:"maximum number of items that could be returned in a single page"`
-	DefaultPageSize int      `mapstructure:"default_page_size" description:"default number of items returned in a single page if not specified in request"`
+	TokenIssuerURL         string   `mapstructure:"token_issuer_url" description:"url of the token issuer which to use for validating tokens"`
+	ClientID               string   `mapstructure:"client_id" description:"id of the client from which the token must be issued"`
+	TokenBasicAuth         bool     `mapstructure:"token_basic_auth" description:"specifies if client credentials to the authorization server should be sent in the header as basic auth (true) or in the body (false)"`
+	ProtectedLabels        []string `mapstructure:"protected_labels" description:"defines labels which cannot be modified/added by REST API requests"`
+	OSBVersion             string   `mapstructure:"-"`
+	MaxPageSize            int      `mapstructure:"max_page_size" description:"maximum number of items that could be returned in a single page"`
+	DefaultPageSize        int      `mapstructure:"default_page_size" description:"default number of items returned in a single page if not specified in request"`
+	EnableInstanceTransfer bool     `mapstructure:"enable_instance_transfer" description:"whether service instance transfer is enabled or not"`
 }
 
 // DefaultSettings returns default values for API settings
 func DefaultSettings() *Settings {
 	return &Settings{
-		TokenIssuerURL:  "",
-		ClientID:        "",
-		TokenBasicAuth:  true, // RFC 6749 section 2.3.1
-		OSBVersion:      osbVersion,
-		MaxPageSize:     200,
-		DefaultPageSize: 50,
-		ProtectedLabels: []string{},
+		TokenIssuerURL:         "",
+		ClientID:               "",
+		TokenBasicAuth:         true, // RFC 6749 section 2.3.1
+		ProtectedLabels:        []string{},
+		OSBVersion:             osbVersion,
+		MaxPageSize:            200,
+		DefaultPageSize:        50,
+		EnableInstanceTransfer: false,
 	}
 }
 
@@ -148,7 +150,7 @@ func New(ctx context.Context, e env.Environment, options *Options) (*web.API, er
 			filters.NewPlansFilterByVisibility(options.Repository),
 			filters.NewServicesFilterByVisibility(options.Repository),
 			&filters.CheckBrokerCredentialsFilter{},
-			filters.NewServiceInstanceTransferFilter(options.Repository),
+			filters.NewServiceInstanceTransferFilter(options.Repository, options.APISettings.EnableInstanceTransfer),
 		},
 		Registry: health.NewDefaultRegistry(),
 	}, nil

api/api_test.go

@@ -18,9 +18,10 @@ package api_test
 
 import (
 	"context"
-	"github.com/Peripli/service-manager/operations"
 	"testing"
 
+	"github.com/Peripli/service-manager/operations"
+
 	"github.com/Peripli/service-manager/pkg/env/envfakes"
 
 	"github.com/Peripli/service-manager/storage"

api/filters/check_instance_transfer_filter.go

@@ -33,12 +33,14 @@ import (
 const ServiceInstanceTransferFilterName = "ServiceInstanceTransferFilter"
 
 type serviceInstanceTransferFilter struct {
-	repository storage.Repository
+	repository             storage.Repository
+	enableInstanceTransfer bool
 }
 
-func NewServiceInstanceTransferFilter(repository storage.Repository) *serviceInstanceTransferFilter {
+func NewServiceInstanceTransferFilter(repository storage.Repository, enableInstanceTransfer bool) *serviceInstanceTransferFilter {
 	return &serviceInstanceTransferFilter{
-		repository: repository,
+		repository:             repository,
+		enableInstanceTransfer: enableInstanceTransfer,
 	}
 }
 
@@ -54,6 +56,14 @@ func (f *serviceInstanceTransferFilter) Run(req *web.Request, next web.Handler)
 		return next.Handle(req)
 	}
 
+	if !f.enableInstanceTransfer {
+		return nil, &util.HTTPError{
+			ErrorType:   "TransferDisabled",
+			Description: "Instance transfer is disabled in this service-manager installation",
+			StatusCode:  http.StatusBadRequest,
+		}
+	}
+
 	instanceID := req.PathParams[web.PathParamResourceID]
 	if instanceID == "" {
 		return next.Handle(req)

api/filters/platform_id_instance_validation_filter.go

@@ -57,20 +57,16 @@ func (*PlatformIDInstanceValidationFilter) Run(req *web.Request, next web.Handle
 	var err error
 	switch req.Request.Method {
 	case http.MethodPost:
-		if platformID != "" && platformID != types.SMPlatform {
-			return nil, &util.HTTPError{
-				ErrorType:   "BadRequest",
-				Description: fmt.Sprintf("Providing %s property during provisioning/updating with a value different from %s is forbidden", platformIDProperty, types.SMPlatform),
-				StatusCode:  http.StatusBadRequest,
-			}
-		}
 		if platformID == "" {
 			req.Body, err = sjson.SetBytes(req.Body, platformIDProperty, types.SMPlatform)
 			if err != nil {
 				return nil, err
 			}
 		}
 	case http.MethodPatch:
+		fallthrough
+		//TODO delete case remove when deletion of instances from all platforms is enabled
+	case http.MethodDelete:
 		// we don't want to explicitly add SMPlatform criteria for patch if instance is being migrated to SM
 		if platformID != types.SMPlatform {
 			byPlatformID := query.ByField(query.EqualsOperator, platformIDProperty, types.SMPlatform)
@@ -80,8 +76,6 @@ func (*PlatformIDInstanceValidationFilter) Run(req *web.Request, next web.Handle
 			}
 			req.Request = req.WithContext(ctx)
 		}
-	case http.MethodDelete:
-		// we don't want to explicitly add SMPlatform criteria for delete - this allows deleting instances from other platforms that are unreachable
 	}
 
 	return next.Handle(req)

storage/interceptors/smaap_service_instance_interceptor.go

@@ -338,6 +338,10 @@ func (i *ServiceInstanceInterceptor) AroundTxDelete(f storage.InterceptDeleteAro
 
 		if instances.Len() != 0 {
 			instance := instances.ItemAt(0).(*types.ServiceInstance)
+			//TODO remove when deletion of instances from all platforms is enabled
+			if instance.PlatformID != types.SMPlatform {
+				return f(ctx, deletionCriteria...)
+			}
 			operation, found := opcontext.Get(ctx)
 			if !found {
 				return fmt.Errorf("operation missing from context")

test/common/test_context.go

@@ -32,6 +32,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/mitchellh/mapstructure"
+
 	"github.com/Peripli/service-manager/test/tls_settings"
 	"github.com/tidwall/gjson"
 	"golang.org/x/crypto/bcrypt"
@@ -74,6 +76,7 @@ type TestContextBuilder struct {
 	tenantTokenClaims  map[string]interface{}
 
 	shouldSkipBasicAuthClient bool
+	basicAuthPlatformName     string
 
 	Environment func(f ...func(set *pflag.FlagSet)) env.Environment
 	Servers     map[string]FakeServer
@@ -386,6 +389,12 @@ func TestEnv(additionalFlagFuncs ...func(set *pflag.FlagSet)) env.Environment {
 	return env
 }
 
+func (tcb *TestContextBuilder) WithBasicAuthPlatformName(name string) *TestContextBuilder {
+	tcb.basicAuthPlatformName = name
+
+	return tcb
+}
+
 func (tcb *TestContextBuilder) SkipBasicAuthClientSetup(shouldSkip bool) *TestContextBuilder {
 	tcb.shouldSkipBasicAuthClient = shouldSkip
 
@@ -449,6 +458,10 @@ func (tcb *TestContextBuilder) Build() *TestContext {
 	return tcb.BuildWithListener(nil, true)
 }
 
+func (tcb *TestContextBuilder) BuildWithCleanup(shouldCleanup bool) *TestContext {
+	return tcb.BuildWithListener(nil, shouldCleanup)
+}
+
 func (tcb *TestContextBuilder) BuildWithoutCleanup() *TestContext {
 	return tcb.SkipBasicAuthClientSetup(true).BuildWithListener(nil, false)
 }
@@ -508,8 +521,10 @@ func (tcb *TestContextBuilder) BuildWithListener(listener net.Listener, cleanup
 	}
 
 	if !tcb.shouldSkipBasicAuthClient {
-		platformJSON := MakePlatform("tcb-platform-test", "tcb-platform-test", "platform-type", "test-platform")
-		platform := RegisterPlatformInSM(platformJSON, testContext.SMWithOAuth, map[string]string{})
+		platform, err := tcb.prepareTestPlatform(testContext.SMWithOAuth)
+		if err != nil {
+			panic(err)
+		}
 		SMWithBasic := SM.Builder(func(req *httpexpect.Request) {
 			username, password := platform.Credentials.Basic.Username, platform.Credentials.Basic.Password
 			req.WithBasicAuth(username, password).WithClient(tcb.HttpClient)
@@ -521,6 +536,34 @@ func (tcb *TestContextBuilder) BuildWithListener(listener net.Listener, cleanup
 	return testContext
 }
 
+func (tcb *TestContextBuilder) prepareTestPlatform(smClient *SMExpect) (*types.Platform, error) {
+	if tcb.basicAuthPlatformName == "" {
+		tcb.basicAuthPlatformName = "basic-auth-default-test-platform"
+		resp := smClient.GET(web.PlatformsURL + "/" + tcb.basicAuthPlatformName).Expect()
+		if resp.Raw().StatusCode == http.StatusNotFound {
+			platformJSON := MakePlatform(tcb.basicAuthPlatformName, tcb.basicAuthPlatformName, "platform-type", "test-platform")
+			platform := RegisterPlatformInSM(platformJSON, smClient, map[string]string{})
+			return platform, nil
+		}
+
+		if resp.Raw().StatusCode != http.StatusOK {
+			panic(resp.Raw().Status)
+		}
+		var platform types.Platform
+		if err := mapstructure.Decode(resp.JSON().Object().Raw(), &platform); err != nil {
+			return nil, err
+		}
+
+		return &platform, nil
+	}
+
+	smClient.DELETE(web.PlatformsURL + "/" + tcb.basicAuthPlatformName)
+	platformJSON := MakePlatform(tcb.basicAuthPlatformName, tcb.basicAuthPlatformName, "platform-type", "test-platform")
+	platform := RegisterPlatformInSM(platformJSON, smClient, map[string]string{})
+
+	return platform, nil
+}
+
 func NewSMListener() (net.Listener, error) {
 	minPort := 8100
 	maxPort := 9999

test/osb_test/poll_instance_last_operation_test.go

@@ -21,8 +21,6 @@ import (
 	"fmt"
 	"net/http"
 
-	"github.com/Peripli/service-manager/pkg/web"
-
 	"github.com/Peripli/service-manager/pkg/query"
 	"github.com/Peripli/service-manager/pkg/types"
 	. "github.com/onsi/ginkgo"
@@ -128,8 +126,9 @@ var _ = Describe("Get Service Instance Last Operation", func() {
 					brokerServer.ServiceInstanceHandler = parameterizedHandler(http.StatusOK, `{}`)
 
 					By(fmt.Sprintf("Deleting instance with id %s", SID))
-					ctx.SMWithOAuthForTenant.DELETE(web.ServiceInstancesURL+"/"+SID).WithQuery("async", false).
-						Expect().Status(http.StatusOK)
+					byID := query.ByField(query.EqualsOperator, "id", SID)
+					err := ctx.SMRepository.Delete(context.TODO(), types.ServiceInstanceType, byID)
+					Expect(err).ToNot(HaveOccurred())
 
 					ctx.SMWithOAuth.GET("/v1/service_instances/"+SID).WithHeader(brokerAPIVersionHeaderKey, brokerAPIVersionHeaderValue).
 						Expect().Status(http.StatusNotFound)
@@ -360,8 +359,9 @@ var _ = Describe("Get Service Instance Last Operation", func() {
 					brokerServer.ServiceInstanceHandler = parameterizedHandler(http.StatusOK, `{}`)
 
 					By(fmt.Sprintf("Deleting instance with id %s", SID))
-					ctx.SMWithOAuthForTenant.DELETE(web.ServiceInstancesURL+"/"+SID).WithQuery("async", false).
-						Expect().Status(http.StatusOK)
+					byID := query.ByField(query.EqualsOperator, "id", SID)
+					err := ctx.SMRepository.Delete(context.TODO(), types.ServiceInstanceType, byID)
+					Expect(err).ToNot(HaveOccurred())
 
 					ctx.SMWithOAuth.GET("/v1/service_instances/"+SID).WithHeader(brokerAPIVersionHeaderKey, brokerAPIVersionHeaderValue).
 						Expect().Status(http.StatusNotFound)
@@ -504,8 +504,9 @@ var _ = Describe("Get Service Instance Last Operation", func() {
 					brokerServer.ServiceInstanceHandler = parameterizedHandler(http.StatusOK, `{}`)
 
 					By(fmt.Sprintf("Deleting instance with id %s", SID))
-					ctx.SMWithOAuthForTenant.DELETE(web.ServiceInstancesURL+"/"+SID).WithQuery("async", false).
-						Expect().Status(http.StatusOK)
+					byID := query.ByField(query.EqualsOperator, "id", SID)
+					err := ctx.SMRepository.Delete(context.TODO(), types.ServiceInstanceType, byID)
+					Expect(err).ToNot(HaveOccurred())
 
 					ctx.SMWithOAuth.GET("/v1/service_instances/"+SID).WithHeader(brokerAPIVersionHeaderKey, brokerAPIVersionHeaderValue).
 						Expect().Status(http.StatusNotFound)
@@ -583,8 +584,9 @@ var _ = Describe("Get Service Instance Last Operation", func() {
 					brokerServer.ServiceInstanceHandler = parameterizedHandler(http.StatusOK, `{}`)
 
 					By(fmt.Sprintf("Deleting instance with id %s", SID))
-					ctx.SMWithOAuthForTenant.DELETE(web.ServiceInstancesURL+"/"+SID).WithQuery("async", false).
-						Expect().Status(http.StatusOK)
+					byID := query.ByField(query.EqualsOperator, "id", SID)
+					err := ctx.SMRepository.Delete(context.TODO(), types.ServiceInstanceType, byID)
+					Expect(err).ToNot(HaveOccurred())
 
 					ctx.SMWithOAuth.GET("/v1/service_instances/"+SID).WithHeader(brokerAPIVersionHeaderKey, brokerAPIVersionHeaderValue).
 						Expect().Status(http.StatusNotFound)
@@ -677,8 +679,9 @@ var _ = Describe("Get Service Instance Last Operation", func() {
 					brokerServer.ServiceInstanceHandler = parameterizedHandler(http.StatusOK, `{}`)
 
 					By(fmt.Sprintf("Deleting instance with id %s", SID))
-					ctx.SMWithOAuthForTenant.DELETE(web.ServiceInstancesURL+"/"+SID).WithQuery("async", false).
-						Expect().Status(http.StatusOK)
+					byID := query.ByField(query.EqualsOperator, "id", SID)
+					err := ctx.SMRepository.Delete(context.TODO(), types.ServiceInstanceType, byID)
+					Expect(err).ToNot(HaveOccurred())
 
 					ctx.SMWithOAuth.GET("/v1/service_instances/"+SID).WithHeader(brokerAPIVersionHeaderKey, brokerAPIVersionHeaderValue).
 						Expect().Status(http.StatusNotFound)

test/security_test/security_test.go

@@ -38,7 +38,7 @@ var _ = Describe("Service Manager Security Tests", func() {
 	)
 
 	BeforeEach(func() {
-		contextBuilder = common.NewTestContextBuilder()
+		contextBuilder = common.NewTestContextBuilder().WithBasicAuthPlatformName("security-tests-platform")
 	})
 
 	JustBeforeEach(func() {