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

Timeout of creation Service Binding #1388

Merged
merged 22 commits into from
Oct 31, 2024
Prev Previous commit
Next Next commit
wip
ukff committed Oct 29, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 2c885058aa9359df9d1a7c02125f5c49ac6e7b1e
2 changes: 1 addition & 1 deletion cmd/broker/main.go
Original file line number Diff line number Diff line change
@@ -461,7 +461,7 @@ func createAPI(router *mux.Router, servicesConfig broker.ServicesConfig, planVal
"/oauth/{region}/", // oauth2 handled by Ory with region
} {
route := router.PathPrefix(prefix).Subrouter()
broker.AttachRoutes(route, kymaEnvBroker, logger)
broker.AttachRoutes(route, kymaEnvBroker, logger, cfg.Broker.Binding.CreateBindTimeout)
}

respWriter := httputil.NewResponseWriter(logs, cfg.DevelopmentMode)
35 changes: 4 additions & 31 deletions internal/broker/bind_create.go
Original file line number Diff line number Diff line change
@@ -13,8 +13,8 @@
broker "github.com/kyma-project/kyma-environment-broker/internal/broker/bindings"
"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/apiresponses"

"github.com/pivotal-cf/brokerapi/domain/apiresponses"

Check failure on line 17 in internal/broker/bind_create.go

GitHub Actions / run-govulncheck

no required module provides package github.com/pivotal-cf/brokerapi/domain/apiresponses; to add it:

Check failure on line 17 in internal/broker/bind_create.go

GitHub Actions / run-govulncheck

could not import github.com/pivotal-cf/brokerapi/domain/apiresponses (invalid package name: "")

Check failure on line 17 in internal/broker/bind_create.go

GitHub Actions / run-govulncheck

no required module provides package github.com/pivotal-cf/brokerapi/domain/apiresponses; to add it:

Check failure on line 17 in internal/broker/bind_create.go

GitHub Actions / run-govulncheck

could not import github.com/pivotal-cf/brokerapi/domain/apiresponses (invalid package name: "")

Check failure on line 17 in internal/broker/bind_create.go

GitHub Actions / run-go-tests / build

no required module provides package github.com/pivotal-cf/brokerapi/domain/apiresponses; to add it:

Check failure on line 17 in internal/broker/bind_create.go

GitHub Actions / run-go-tests / build

no required module provides package github.com/pivotal-cf/brokerapi/domain/apiresponses; to add it:
"github.com/pivotal-cf/brokerapi/v8/domain"
"github.com/sirupsen/logrus"
)
@@ -30,7 +30,7 @@
MaxExpirationSeconds int `envconfig:"default=7200"`
MinExpirationSeconds int `envconfig:"default=600"`
MaxBindingsCount int `envconfig:"default=10"`
Timeout time.Duration `envconfig:"default=15s"`
CreateBindTimeout time.Duration `envconfig:"default=15s"`
}

type BindEndpoint struct {
@@ -82,36 +82,7 @@
b.log.Infof("Bind parameters: %s", string(details.RawParameters))
b.log.Infof("Bind context: %s", string(details.RawContext))
b.log.Infof("Bind asyncAllowed: %v", asyncAllowed)
ctx, cancel := context.WithCancel(ctx)
timer := time.NewTimer(b.config.Timeout)
defer timer.Stop()

execResult := make(chan domain.Binding)
execError := make(chan error)
go func() {
defer cancel()
result, err := b.execute(ctx, instanceID, bindingID, details, asyncAllowed)
if err != nil {
execError <- err
}
execResult <- result
}()

select {
case <-timer.C:
return domain.Binding{}, fmt.Errorf("timeout")
case result := <-execResult:
return result, nil
case err := <-execError:
if err != nil {
return domain.Binding{}, err
}
}

return domain.Binding{}, nil
}

func (b *BindEndpoint) execute(ctx context.Context, instanceID, bindingID string, details domain.BindDetails, asyncAllowed bool) (domain.Binding, error) {
if !b.config.Enabled {
return domain.Binding{}, fmt.Errorf("not supported")
}
@@ -249,6 +220,8 @@
ExpiresAt: binding.ExpiresAt.Format(expiresAtLayout),
},
}, nil

return domain.Binding{}, nil
}

func (b *BindEndpoint) IsPlanBindable(planName string) bool {
2 changes: 1 addition & 1 deletion internal/broker/bind_create_test.go
Original file line number Diff line number Diff line change
@@ -231,7 +231,7 @@
MaxExpirationSeconds: 7200,
MinExpirationSeconds: 600,
MaxBindingsCount: 10,
Timeout: time.Duration(1 * time.Nanosecond),
CreateBindTimeout: time.Duration(1 * time.Nanosecond),
}
svc := NewBind(*bindingCfg, st.Instances(), st.Bindings(), logrus.New(), nil, nil)
result, err := svc.Bind(context.Background(), instanceID, bindingID, domain.BindDetails{}, false)
13 changes: 11 additions & 2 deletions internal/broker/server.go
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ package broker
import (
"context"
"net/http"
"time"

"code.cloudfoundry.org/lager"
"github.com/gorilla/mux"
@@ -11,8 +12,16 @@ import (
"github.com/pivotal-cf/brokerapi/v8/middlewares"
)

type createBindApiHandler struct {
handler handlers.APIHandler
}

func (h createBindApiHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
h.handler.Bind(rw, r)
}

// copied from github.com/pivotal-cf/brokerapi/api.go
func AttachRoutes(router *mux.Router, serviceBroker domain.ServiceBroker, logger lager.Logger) *mux.Router {
func AttachRoutes(router *mux.Router, serviceBroker domain.ServiceBroker, logger lager.Logger, createBindTimeout time.Duration) *mux.Router {
apiHandler := handlers.NewApiHandler(serviceBroker, logger)
deprovision := func(w http.ResponseWriter, req *http.Request) {
req2 := req.WithContext(context.WithValue(req.Context(), "User-Agent", req.Header.Get("User-Agent")))
@@ -27,7 +36,7 @@ func AttachRoutes(router *mux.Router, serviceBroker domain.ServiceBroker, logger
router.HandleFunc("/v2/service_instances/{instance_id}", apiHandler.Update).Methods("PATCH")

router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.GetBinding).Methods("GET")
router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.Bind).Methods("PUT")
router.Handle("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", http.TimeoutHandler(createBindApiHandler{}, createBindTimeout, "request timeout: time exceeded %s")).Methods("PUT")
router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}", apiHandler.Unbind).Methods("DELETE")

router.HandleFunc("/v2/service_instances/{instance_id}/service_bindings/{binding_id}/last_operation", apiHandler.LastBindingOperation).Methods("GET")

Unchanged files with check annotations Beta

t.Run("should contain 'bindable' set to true", func(t *testing.T) {
// given
var (
name = "testServiceName"

Check failure on line 123 in internal/broker/services_test.go

GitHub Actions / run-go-linter

declared and not used: name (typecheck)

Check failure on line 123 in internal/broker/services_test.go

GitHub Actions / run-go-linter

declared and not used: name (typecheck)

Check failure on line 123 in internal/broker/services_test.go

GitHub Actions / run-go-linter

declared and not used: name (typecheck)

Check failure on line 123 in internal/broker/services_test.go

GitHub Actions / run-go-linter

declared and not used: name (typecheck)
supportURL = "example.com/support"

Check failure on line 124 in internal/broker/services_test.go

GitHub Actions / run-go-linter

declared and not used: supportURL (typecheck)

Check failure on line 124 in internal/broker/services_test.go

GitHub Actions / run-go-linter

declared and not used: supportURL (typecheck)

Check failure on line 124 in internal/broker/services_test.go

GitHub Actions / run-go-linter

declared and not used: supportURL (typecheck)

Check failure on line 124 in internal/broker/services_test.go

GitHub Actions / run-go-linter

declared and not used: supportURL (typecheck)
)
cfg := broker.Config{
EnablePlans: []string{"gcp", "azure", "sap-converged-cloud", "aws", "free"},