add some tests and refactor

ci/autoscaler/scripts/deploy-autoscaler.sh

@@ -134,9 +134,9 @@ function check_ops_files(){
 
 function deploy() {
   # Try to silence Prometheus but do not fail deployment if there's an error
-  #${script_dir}/silence_prometheus_alert.sh "BOSHJobEphemeralDiskPredictWillFill" || true
-  #${script_dir}/silence_prometheus_alert.sh "BOSHJobProcessUnhealthy" || true
-  #${script_dir}/silence_prometheus_alert.sh "BOSHJobUnhealthy" || true
+  ${script_dir}/silence_prometheus_alert.sh "BOSHJobEphemeralDiskPredictWillFill" || true
+  ${script_dir}/silence_prometheus_alert.sh "BOSHJobProcessUnhealthy" || true
+  ${script_dir}/silence_prometheus_alert.sh "BOSHJobUnhealthy" || true
 
   create_manifest
 

src/acceptance/app/custom_metric_test.go

@@ -75,10 +75,8 @@ var _ = Describe("AutoScaler custom metrics", func() {
 					WithTimeout(5 * time.Minute).
 					WithPolling(15 * time.Second).
 					Should(Equal(1))
-
 			})
 		})
-
 	})
 
 	Describe("Custom metrics with neighbour app", func() {
@@ -98,11 +96,7 @@ var _ = Describe("AutoScaler custom metrics", func() {
 
 		})
 		Context("neighbour app sends custom metrics for app B via mtls", func() {
-			JustBeforeEach(func() {
-
-			})
-
-			FWhen("policy is attached with the appToScale with a bound_app mentioned", func() {
+			When("policy is attached with the appToScale with a bound_app mentioned", func() {
 				BeforeEach(func() {
 					policy = GenerateBindingsWithScalingPolicy("bound_app", 1, 2, "test_metric", 100, 500)
 				})
@@ -138,7 +132,7 @@ var _ = Describe("AutoScaler custom metrics", func() {
 				  Job (e3fee92d-1062-4853-a6d4-d017f6b43157) failed: bind could not be completed: Service broker error: invalid policy provided: [{"context":"(root)","description":"Must validate at least one schema (anyOf)"},{"context":"(root)","description":"scaling_rules is required"},{"context":"(root).instance_min_count","description":"Must be greater than or equal to 1"}]
 
 			*/
-			XWhen("policy is not attached with the neighbour app", func() {
+			/*When("policy is not attached with the neighbour app", func() {
 				BeforeEach(func() {
 					policy = GenerateBindingConfiguration("bound_app")
 				})
@@ -156,11 +150,10 @@ var _ = Describe("AutoScaler custom metrics", func() {
 						WithTimeout(5 * time.Minute).
 						WithPolling(15 * time.Second).
 						Should(Equal(1))
-
 				})
-			})
+			})*/
 
-			XWhen("app B tries to send metrics for neighbour app with strategy same_app", func() {
+			When("app B tries to send metrics for neighbour app with strategy same_app", func() {
 				BeforeEach(func() {
 					policy = GenerateBindingsWithScalingPolicy("bound_app", 1, 2, "test_metric", 100, 500)
 				})
@@ -176,7 +169,6 @@ var _ = Describe("AutoScaler custom metrics", func() {
 
 func sendMetricToAutoscaler(config *config.Config, appToScaleGUID string, neighbourAppName string, metricThreshold int, mtls bool) func() (int, error) {
 	return func() (int, error) {
-
 		if mtls {
 			SendMetricMTLS(config, appToScaleGUID, neighbourAppName, metricThreshold)
 		} else {

src/autoscaler/api/broker/broker.go

@@ -515,7 +515,7 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
 	}
 	// set the default custom metrics strategy if not provided
 	if bindingConfiguration.GetCustomMetricsStrategy() == "" {
-		bindingConfiguration.SetDefaultCustomMetricsStrategy("same_app")
+		bindingConfiguration.SetDefaultCustomMetricsStrategy(models.CustomMetricsSameApp)
 	}
 	logger.Info("binding-configuration", lager.Data{"bindingConfiguration": bindingConfiguration})
 
@@ -871,9 +871,7 @@ func isValidCredentialType(credentialType string) bool {
 }
 
 func createServiceBinding(ctx context.Context, bindingDB db.BindingDB, bindingID, instanceID, appGUID string, customMetricsStrategy string) error {
-	//TODO call bindingDB.CreateServiceBindingWithConfigs method. No need to call CreateServiceBinding method
-	// Caution: CHECK the below code may break the existing functionality ??
-	if customMetricsStrategy == "bound_app" || customMetricsStrategy == "same_app" {
+	if customMetricsStrategy == models.CustomMetricsBoundApp || customMetricsStrategy == models.CustomMetricsSameApp {
 		return bindingDB.CreateServiceBinding(ctx, bindingID, instanceID, appGUID, customMetricsStrategy)
 	}
 	return ErrInvalidCustomMetricsStrategy

src/autoscaler/api/brokerserver/broker_handler_test.go

@@ -920,15 +920,10 @@ var _ = Describe("BrokerHandler", func() {
 				Expect(schedulerServer.ReceivedRequests()).To(HaveLen(1))
 			})
 			It("returns the correct binding parameters", func() {
-				creds := &models.CredentialResponse{}
-				responseString := resp.Body.String()
-				err := json.Unmarshal([]byte(responseString), creds)
-				Expect(err).NotTo(HaveOccurred())
-				Expect(*creds.Credentials.CustomMetrics.URL).To(Equal("someURL"))
-				Expect(creds.Credentials.CustomMetrics.MtlsUrl).To(Equal("Mtls-someURL"))
+				verifyCredentialsGenerated(resp)
 			})
 		})
-		Context("Binding configurations are present", func() {
+		When("Binding configurations are present", func() {
 			BeforeEach(func() {
 				bindingPolicy = `{
 				  "configuration": {
@@ -999,10 +994,10 @@ var _ = Describe("BrokerHandler", func() {
 			})
 			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)
 			})
 		})
 
@@ -1507,6 +1502,15 @@ var _ = Describe("BrokerHandler", func() {
 	})
 })
 
+func verifyCredentialsGenerated(resp *httptest.ResponseRecorder) {
+	creds := &models.CredentialResponse{}
+	responseString := resp.Body.String()
+	err := json.Unmarshal([]byte(responseString), creds)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(*creds.Credentials.CustomMetrics.URL).To(Equal("someURL"))
+	Expect(creds.Credentials.CustomMetrics.MtlsUrl).To(Equal("Mtls-someURL"))
+}
+
 func createInstanceCreationRequestBody(defaultPolicy string) []byte {
 	m := json.RawMessage(defaultPolicy)
 	instanceCreationReqBody := &models.InstanceCreationRequestBody{

src/autoscaler/db/db.go

@@ -80,9 +80,6 @@ type BindingDB interface {
 	GetBindingIdsByInstanceId(ctx context.Context, instanceId string) ([]string, error)
 	GetAppBindingByAppId(ctx context.Context, appId string) (string, error)
 	IsAppBoundToSameAutoscaler(ctx context.Context, appId string, appToScaleId string) (bool, error)
-	// FIxme - remove this method
-	CreateServiceBindingWithConfigs(ctx context.Context, bindingId string, serviceInstanceId string, appId string, strategy string) error
-
 	GetCustomMetricStrategyByAppId(ctx context.Context, appId string) (string, error)
 }
 

src/autoscaler/db/sqldb/binding_sqldb.go

@@ -420,22 +420,6 @@ func (bdb *BindingSQLDB) IsAppBoundToSameAutoscaler(ctx context.Context, metricS
 	return false, nil
 }
 
-func (bdb *BindingSQLDB) CreateServiceBindingWithConfigs(ctx context.Context, bindingId string, serviceInstanceId string, appId string, strategy string) error {
-	err := bdb.isBindingExists(ctx, bindingId, serviceInstanceId, appId)
-	if err != nil {
-		return err
-	}
-	query := bdb.sqldb.Rebind("INSERT INTO binding" +
-		"(binding_id, service_instance_id, app_id, created_at, custom_metrics_strategy) " +
-		"VALUES(?, ?, ?, ?,?)")
-	_, err = bdb.sqldb.ExecContext(ctx, query, bindingId, serviceInstanceId, appId, time.Now(), strategy)
-
-	if err != nil {
-		bdb.logger.Error("create-service-binding", err, lager.Data{"query": query, "serviceinstanceid": serviceInstanceId, "bindingid": bindingId, "appid": appId, "strategy": strategy})
-	}
-	return err
-}
-
 func (bdb *BindingSQLDB) GetCustomMetricStrategyByAppId(ctx context.Context, appId string) (string, error) {
 	customMetricsStrategy, err := bdb.fetchCustomMetricStrategyByAppId(ctx, appId)
 	if err != nil {

src/autoscaler/db/sqldb/binding_sqldb_test.go

@@ -383,7 +383,7 @@ var _ = Describe("BindingSqldb", func() {
 					Expect(err).To(Equal(db.ErrAlreadyExists))
 				})
 			})
-			Context("When service binding is created with custom metrics strategy", func() {
+			Context("When service binding is created with custom metrics strategy 'bound_app'", func() {
 				BeforeEach(func() {
 					customMetricsStrategy = "bound_app"
 				})
@@ -392,6 +392,25 @@ var _ = Describe("BindingSqldb", func() {
 					Expect(hasServiceBindingWithCustomMetricStrategy(testBindingId, testInstanceId, customMetricsStrategy)).To(BeTrue())
 				})
 			})
+			Context("When service binding is created with custom metrics strategy 'same_app'", func() {
+				BeforeEach(func() {
+					customMetricsStrategy = "same_app"
+				})
+				It("should succeed", func() {
+					Expect(err).NotTo(HaveOccurred())
+					Expect(hasServiceBindingWithCustomMetricStrategy(testBindingId, testInstanceId, customMetricsStrategy)).To(BeTrue())
+				})
+			})
+
+			When("service binding is created with invalid custom metrics strategy", func() {
+				BeforeEach(func() {
+					customMetricsStrategy = ""
+				})
+				It("should throw an error with foreign key violation", func() {
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("foreign key constraint"))
+				})
+			})
 		})
 	})
 
@@ -674,57 +693,29 @@ var _ = Describe("BindingSqldb", func() {
 
 	})
 
-	Describe("CreateServiceBindingWithConfigs", func() {
+	Describe("GetCustomMetricStrategyByAppId", func() {
 		BeforeEach(func() {
 			err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
 			Expect(err).NotTo(HaveOccurred())
 		})
-		When("configuration bounded_app is provided", func() {
-			JustBeforeEach(func() {
-				err = bdb.CreateServiceBindingWithConfigs(context.Background(), testBindingId, testInstanceId, testAppId, "bound_app")
+		Context("When service instance and binding exists with custom metrics strategy 'bound_app'", func() {
+			BeforeEach(func() {
+				err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "bound_app")
 				Expect(err).NotTo(HaveOccurred())
 			})
-			It("should save the binding in the database", func() {
-				Expect(hasServiceBindingWithCustomMetricStrategy(testBindingId, testInstanceId, "bound_app")).To(BeTrue())
-
+			It("should get the custom metrics strategy from the database", func() {
+				customMetricStrategy, _ := bdb.GetCustomMetricStrategyByAppId(context.Background(), testAppId)
+				Expect(customMetricStrategy).To(Equal("bound_app"))
 			})
 		})
-		When("default configuration is provided", func() {
-			JustBeforeEach(func() {
-				err = bdb.CreateServiceBindingWithConfigs(context.Background(), testBindingId, testInstanceId, testAppId, "same_app")
+		Context("When service instance and binding exists with custom metrics strategy 'same_app'", func() {
+			BeforeEach(func() {
+				err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "same_app")
 				Expect(err).NotTo(HaveOccurred())
 			})
-			It("should save the binding in the database", func() {
-				Expect(hasServiceBindingWithCustomMetricStrategy(testBindingId, testInstanceId, "same_app")).To(BeTrue())
-				//TODO check if the default was set
-
-			})
-		})
-		When("configuration is not provided", func() {
-			JustBeforeEach(func() {
-				err = bdb.CreateServiceBindingWithConfigs(context.Background(), testBindingId, testInstanceId, testAppId, "")
-
-			})
-			It("should throw an error with foreign key violation", func() {
-				Expect(err).To(HaveOccurred())
-			})
-		})
-
-	})
-
-	Describe("GetCustomMetricStrategyByAppId", func() {
-		BeforeEach(func() {
-			err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
-			Expect(err).NotTo(HaveOccurred())
-			// FIXME Use the original createServiceBinding
-			err = bdb.CreateServiceBindingWithConfigs(context.Background(), testBindingId, testInstanceId, testAppId, "bound_app")
-			Expect(err).NotTo(HaveOccurred())
-
-		})
-		Context("When service instance and binding exists", func() {
 			It("should get the custom metrics strategy from the database", func() {
 				customMetricStrategy, _ := bdb.GetCustomMetricStrategyByAppId(context.Background(), testAppId)
-				Expect(customMetricStrategy).To(Equal("bound_app"))
+				Expect(customMetricStrategy).To(Equal("same_app"))
 			})
 		})
 

src/autoscaler/metricsforwarder/config/config_test.go

@@ -391,7 +391,7 @@ health:
 			})
 
 			It("should error", func() {
-				Expect(err).To(MatchError(MatchRegexp("Policy DB url is empty")))
+				Expect(err).To(MatchError(MatchRegexp("configuration error: Policy DB url is empty")))
 			})
 		})
 
@@ -401,7 +401,7 @@ health:
 			})
 
 			It("should error", func() {
-				Expect(err).To(MatchError(MatchRegexp("Configuration error: Binding DB url is empty")))
+				Expect(err).To(MatchError(MatchRegexp("configuration error: Binding DB url is empty")))
 			})
 		})
 

src/autoscaler/metricsforwarder/server/auth/xfcc_auth.go

@@ -2,6 +2,7 @@ package auth
 
 import (
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/db"
+	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/models"
 	"code.cloudfoundry.org/lager/v3"
 	"crypto/x509"
 	"encoding/base64"
@@ -11,8 +12,6 @@ import (
 	"strings"
 )
 
-const customMetricsStrategyBoundApp = "bound_app"
-
 var ErrXFCCHeaderNotFound = errors.New("mTLS authentication method not found")
 var ErrorNoAppIDFound = errors.New("certificate does not contain an app id")
 var ErrorAppIDWrong = errors.New("app is not allowed to send metrics due to invalid app id in certificate")
@@ -54,7 +53,7 @@ func (a *Auth) XFCCAuth(r *http.Request, bindingDB db.BindingDB, appID string) e
 		}
 		a.logger.Info("custom-metrics-submission-strategy", lager.Data{"appID": appID, "submitterAppCert": submitterAppCert, "strategy": customMetricSubmissionStrategy})
 
-		if customMetricSubmissionStrategy == customMetricsStrategyBoundApp {
+		if customMetricSubmissionStrategy == models.CustomMetricsBoundApp {
 			metricSubmissionStrategy = &BoundedMetricsSubmissionStrategy{}
 		} else {
 			metricSubmissionStrategy = &DefaultMetricsSubmissionStrategy{}

src/autoscaler/models/binding_configs.go

@@ -14,6 +14,12 @@ package models
     }
   }
 */
+
+const (
+	CustomMetricsBoundApp = "bound_app"
+	CustomMetricsSameApp  = "same_app"
+)
+
 type BindingConfig struct {
 	Configuration Configuration `json:"configuration"`
 }