Skip to content

Commit

Permalink
allow only bound_app strategy
Browse files Browse the repository at this point in the history
  • Loading branch information
@asalan316
asalan316 committed Nov 6, 2024
1 parent b259ba7 commit 47e56be
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 72 deletions.
14 changes: 7 additions & 7 deletions src/acceptance/broker/broker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,6 @@ var _ = Describe("AutoScaler Service Broker", func() {
instance.unbind(appName)
})

It("bind&unbinds without policy", func() {
helpers.BindServiceToApp(cfg, appName, instance.name())
bindingParameters := helpers.GetServiceCredentialBindingParameters(cfg, instance.name(), appName)
Expect(bindingParameters).Should(MatchJSON("{}"))
instance.unbind(appName)
})

It("binds&unbinds with policy having credential-type as x509", func() {
policyFile := "../assets/file/policy/policy-with-credential-type.json"
_, err := os.ReadFile(policyFile)
Expand All @@ -135,6 +128,13 @@ var _ = Describe("AutoScaler Service Broker", func() {
instance.unbind(appName)
})

It("bind&unbinds without policy", func() {
helpers.BindServiceToApp(cfg, appName, instance.name())
bindingParameters := helpers.GetServiceCredentialBindingParameters(cfg, instance.name(), appName)
Expect(bindingParameters).Should(MatchJSON("{}"))
instance.unbind(appName)
})

It("binds&unbinds with configurations and policy", func() {
policyFile := "../assets/file/policy/policy-with-configuration.json"
policyWithConfig, err := os.ReadFile(policyFile)
Expand Down
77 changes: 50 additions & 27 deletions src/autoscaler/api/broker/broker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"errors"
"fmt"
"net/http"
"reflect"
"regexp"
"strings"

Expand Down Expand Up @@ -499,24 +498,16 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
if details.RawParameters != nil {
policyJson = details.RawParameters
}
bindingConfiguration := &models.BindingConfig{}
if policyJson != nil {
err := json.Unmarshal(policyJson, &bindingConfiguration)
if err != nil {
actionReadBindingConfiguration := "read-binding-configurations"
logger.Error("unmarshal-binding-configuration", err)
return result, apiresponses.NewFailureResponseBuilder(
ErrInvalidConfigurations, http.StatusBadRequest, actionReadBindingConfiguration).
WithErrorKey(actionReadBindingConfiguration).
Build()
}
bindingConfiguration, err := b.getBindingConfigurationFromRequest(policyJson, logger)
if err != nil {
logger.Error("get-binding-configuration-from-request", err)
return result, err
}
// set the default custom metrics strategy if not provided
if bindingConfiguration.GetCustomMetricsStrategy() == "" {
bindingConfiguration.SetCustomMetricsStrategy(models.CustomMetricsSameApp)
bindingConfiguration, err = b.validateOrGetDefaultCustomMetricsStrategy(bindingConfiguration, logger)
if err != nil {
logger.Error("validate-or-get-default-custom-metric-strategy", err)
return result, err
}
logger.Info("binding-configuration", lager.Data{"bindingConfiguration": bindingConfiguration})

policy, err := b.getPolicyFromJsonRawMessage(policyJson, instanceID, details.PlanID)
if err != nil {
logger.Error("get-default-policy", err)
Expand Down Expand Up @@ -549,7 +540,6 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
if err := b.handleExistingBindingsResiliently(ctx, instanceID, appGUID, logger); err != nil {
return result, err
}
// save custom metrics strategy check - bindingConfiguration.CustomMetricsConfig.MetricSubmissionStrategy ! == ""
err = createServiceBinding(ctx, b.bindingdb, bindingID, instanceID, appGUID, bindingConfiguration.GetCustomMetricsStrategy())

if err != nil {
Expand Down Expand Up @@ -603,6 +593,38 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
return result, nil
}

func (b *Broker) validateOrGetDefaultCustomMetricsStrategy(bindingConfiguration *models.BindingConfig, logger lager.Logger) (*models.BindingConfig, error) {
strategy := bindingConfiguration.GetCustomMetricsStrategy()
if strategy == "" {
bindingConfiguration.SetCustomMetricsStrategy(models.CustomMetricsSameApp)
} else if strategy != models.CustomMetricsBoundApp {
actionName := "verify-custom-metrics-strategy"
return bindingConfiguration, apiresponses.NewFailureResponseBuilder(
ErrInvalidCustomMetricsStrategy, http.StatusBadRequest, actionName).
WithErrorKey(actionName).
Build()
}
logger.Info("binding-configuration", lager.Data{"bindingConfiguration": bindingConfiguration})
return bindingConfiguration, nil
}

func (b *Broker) getBindingConfigurationFromRequest(policyJson json.RawMessage, logger lager.Logger) (*models.BindingConfig, error) {
bindingConfiguration := &models.BindingConfig{}
var err error
if policyJson != nil {
err = json.Unmarshal(policyJson, &bindingConfiguration)
if err != nil {
actionReadBindingConfiguration := "read-binding-configurations"
logger.Error("unmarshal-binding-configuration", err)
return bindingConfiguration, apiresponses.NewFailureResponseBuilder(
ErrInvalidConfigurations, http.StatusBadRequest, actionReadBindingConfiguration).
WithErrorKey(actionReadBindingConfiguration).
Build()
}
}
return bindingConfiguration, err
}

func getOrDefaultCredentialType(policyJson json.RawMessage, credentialTypeConfig string, logger lager.Logger) (*models.CustomMetricsBindingAuthScheme, error) {
credentialType := &models.CustomMetricsBindingAuthScheme{CredentialType: credentialTypeConfig}
if len(policyJson) != 0 {
Expand Down Expand Up @@ -734,19 +756,14 @@ func (b *Broker) buildConfigurationIfPresent(customMetricsStrategy string) (*mod
var combinedConfig *models.BindingConfigWithScaling
var bindingConfig *models.BindingConfig

if customMetricsStrategy != "" && customMetricsStrategy != models.CustomMetricsSameApp { //if custom metric was not given in the binding process
if customMetricsStrategy != "" && customMetricsStrategy != models.CustomMetricsSameApp { //if custom metric was given in the binding process
combinedConfig = &models.BindingConfigWithScaling{}
bindingConfig = &models.BindingConfig{}
bindingConfig.SetCustomMetricsStrategy(customMetricsStrategy)
combinedConfig.BindingConfig = *bindingConfig
}
return combinedConfig, bindingConfig
}

func (b *Broker) isEmpty(bindingConfig *models.BindingConfig) bool {
return reflect.DeepEqual(bindingConfig, &models.BindingConfig{})
}

func (b *Broker) getServiceBinding(ctx context.Context, bindingID string) (*models.ServiceBinding, error) {
logger := b.logger.Session("get-service-binding", lager.Data{"bindingID": bindingID})

Expand Down Expand Up @@ -891,8 +908,14 @@ func isValidCredentialType(credentialType string) bool {
}

func createServiceBinding(ctx context.Context, bindingDB db.BindingDB, bindingID, instanceID, appGUID string, customMetricsStrategy string) error {
if customMetricsStrategy == models.CustomMetricsBoundApp || customMetricsStrategy == models.CustomMetricsSameApp {
return bindingDB.CreateServiceBinding(ctx, bindingID, instanceID, appGUID, customMetricsStrategy)
switch customMetricsStrategy {
case models.CustomMetricsBoundApp, models.CustomMetricsSameApp:
err := bindingDB.CreateServiceBinding(ctx, bindingID, instanceID, appGUID, customMetricsStrategy)
if err != nil {
return err
}
default:
return ErrInvalidCustomMetricsStrategy
}
return ErrInvalidCustomMetricsStrategy
return nil
}
148 changes: 111 additions & 37 deletions src/autoscaler/api/brokerserver/broker_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -923,16 +923,14 @@ var _ = Describe("BrokerHandler", func() {
verifyCredentialsGenerated(resp)
})
})
When("Binding configurations and policy are present", func() {
BeforeEach(func() {
bindingPolicy = `{
Context("Binding configurations", func() {
When("invalid custom strategy provided in the binding parameters", func() {
BeforeEach(func() {
bindingPolicy = `{
"configuration": {
"custom_metrics": {
"auth": {
"credential_type": "binding_secret"
},
"metric_submission_strategy": {
"allow_from": "bound_app"
"allow_from": "same_app"
}
}
},
Expand Down Expand Up @@ -961,10 +959,10 @@ var _ = Describe("BrokerHandler", func() {
"adjustment":"-1"
}]
}`
bindingRequestBody.Policy = json.RawMessage(bindingPolicy)
body, err = json.Marshal(bindingRequestBody)
Expect(err).NotTo(HaveOccurred())
bindingPolicy = `{
bindingRequestBody.Policy = json.RawMessage(bindingPolicy)
body, err = json.Marshal(bindingRequestBody)
Expect(err).NotTo(HaveOccurred())
bindingPolicy = `{
"instance_max_count":4,
"instance_min_count":1,
"schedules": {
Expand All @@ -990,19 +988,17 @@ var _ = Describe("BrokerHandler", func() {
"adjustment":"-1"
}]
}`
verifyScheduleIsUpdatedInScheduler(testAppId, bindingPolicy)
})
It("succeeds with 201", func() {
Expect(resp.Code).To(Equal(http.StatusCreated))
By("updating the scheduler")
Expect(schedulerServer.ReceivedRequests()).To(HaveLen(1))
Expect(bindingdb.CreateServiceBindingCallCount()).To(Equal(1))
verifyCredentialsGenerated(resp)
verifyScheduleIsUpdatedInScheduler(testAppId, bindingPolicy)
})
It("should fail with 400", func() {
Expect(resp.Code).To(Equal(http.StatusBadRequest))
Expect(resp.Body.String()).To(MatchJSON(`{"error": "verify-custom-metrics-strategy", "description": "error: custom metrics strategy not supported"}`))

})
})
})
When("Binding configurations are empty", func() {
BeforeEach(func() {
bindingPolicy = `{
When("are empty", func() {
BeforeEach(func() {
bindingPolicy = `{
"instance_max_count":4,
"instance_min_count":1,
"schedules": {
Expand All @@ -1028,23 +1024,101 @@ var _ = Describe("BrokerHandler", func() {
"adjustment":"-1"
}]
}`
bindingRequestBody.Policy = json.RawMessage(bindingPolicy)
body, err = json.Marshal(bindingRequestBody)
Expect(err).NotTo(HaveOccurred())
bindingRequestBody.Policy = json.RawMessage(bindingPolicy)
body, err = json.Marshal(bindingRequestBody)
Expect(err).NotTo(HaveOccurred())

verifyScheduleIsUpdatedInScheduler(testAppId, bindingPolicy)
})
It("set the default custom metrics strategy", func() {
_, _, _, _, customMetricsStrategy := bindingdb.CreateServiceBindingArgsForCall(0)
Expect(customMetricsStrategy).To(Equal(models.CustomMetricsSameApp))
verifyScheduleIsUpdatedInScheduler(testAppId, bindingPolicy)
})
It("set the default custom metrics strategy", func() {
_, _, _, _, customMetricsStrategy := bindingdb.CreateServiceBindingArgsForCall(0)
Expect(customMetricsStrategy).To(Equal(models.CustomMetricsSameApp))

})
It("succeeds with 201", func() {
Expect(resp.Code).To(Equal(http.StatusCreated))
By("updating the scheduler")
Expect(schedulerServer.ReceivedRequests()).To(HaveLen(1))
Expect(bindingdb.CreateServiceBindingCallCount()).To(Equal(1))
verifyCredentialsGenerated(resp)
})
})
It("succeeds with 201", func() {
Expect(resp.Code).To(Equal(http.StatusCreated))
By("updating the scheduler")
Expect(schedulerServer.ReceivedRequests()).To(HaveLen(1))
Expect(bindingdb.CreateServiceBindingCallCount()).To(Equal(1))
verifyCredentialsGenerated(resp)
When("policy and binding configuration are present", func() {
BeforeEach(func() {
bindingPolicy = `{
"configuration": {
"custom_metrics": {
"auth": {
"credential_type": "binding_secret"
},
"metric_submission_strategy": {
"allow_from": "bound_app"
}
}
},
"instance_max_count":4,
"instance_min_count":1,
"schedules": {
"timezone": "Asia/Shanghai",
"recurring_schedule": [{
"start_time": "10:00",
"end_time": "18:00",
"days_of_week": [
1,
2,
3
],
"instance_min_count": 1,
"instance_max_count": 10,
"initial_min_instance_count": 5
}]
},
"scaling_rules":[
{
"metric_type":"memoryused",
"threshold":30,
"operator":"<",
"adjustment":"-1"
}]
}`
bindingRequestBody.Policy = json.RawMessage(bindingPolicy)
body, err = json.Marshal(bindingRequestBody)
Expect(err).NotTo(HaveOccurred())
bindingPolicy = `{
"instance_max_count":4,
"instance_min_count":1,
"schedules": {
"timezone": "Asia/Shanghai",
"recurring_schedule": [{
"start_time": "10:00",
"end_time": "18:00",
"days_of_week": [
1,
2,
3
],
"instance_min_count": 1,
"instance_max_count": 10,
"initial_min_instance_count": 5
}]
},
"scaling_rules":[
{
"metric_type":"memoryused",
"threshold":30,
"operator":"<",
"adjustment":"-1"
}]
}`
verifyScheduleIsUpdatedInScheduler(testAppId, bindingPolicy)
})
It("succeeds with 201", func() {
Expect(resp.Code).To(Equal(http.StatusCreated))
By("updating the scheduler")
Expect(schedulerServer.ReceivedRequests()).To(HaveLen(1))
Expect(bindingdb.CreateServiceBindingCallCount()).To(Equal(1))
verifyCredentialsGenerated(resp)
})
})
})

Expand Down
3 changes: 2 additions & 1 deletion src/autoscaler/models/binding_configs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ package models

const (
CustomMetricsBoundApp = "bound_app"
CustomMetricsSameApp = "same_app"
// CustomMetricsSameApp default value if not specified
CustomMetricsSameApp = "same_app"
)

type BindingConfig struct {
Expand Down

0 comments on commit 47e56be

Please sign in to comment.