Merge pull request #3044 from cloudfoundry/autoscaler-basic-auth-opti…

…onal

Make Basic Authentication Optional for Custom Metrics

jobs/golangapiserver/spec

@@ -66,6 +66,15 @@ properties:
        - broker_username: user2
          broker_password: password2
     default: ''
+  autoscaler.apiserver.broker.default_credential_type:
+    description: |
+      The default credential type generated to authenticate with the custom metrics API. if no credential type is explicitly set.
+      Allowed values:
+       - binding-secret
+       - x509
+      If credential-type "binding-secret" is set, then username and password are generated by the broker. 
+      If credential-type "x509" is set, only instance identity credentials may be used.
+    default: 'binding-secret'
   autoscaler.apiserver.broker.server.catalog:
     description: ""
   autoscaler.apiserver.broker.server.dashboard_redirect_uri:

jobs/golangapiserver/templates/apiserver.yml.erb

@@ -88,6 +88,7 @@ catalog_schema_path: /var/vcap/packages/golangapiserver/catalog.schema.json
 info_file_path: /var/vcap/jobs/golangapiserver/config/info.json
 policy_schema_path: /var/vcap/packages/golangapiserver/policy_json.schema.json
 dashboard_redirect_uri: <%= p("autoscaler.apiserver.broker.server.dashboard_redirect_uri") %>
+default_credential_type: <%= p("autoscaler.apiserver.broker.default_credential_type") %>
 
 health:
   port: <%= p("autoscaler.apiserver.health.port") %>

spec/fixtures/apiserver.yml

@@ -61,3 +61,4 @@ autoscaler:
       broker_credentials:
       - broker_username: fake_b_user_1
         broker_password: fake_b_password_1
+default_credential_type: binding-secret

spec/jobs/golangapiserver/apiserver_spec.rb

@@ -251,5 +251,11 @@
         end
       end
     end
+
+    context "default_credential_type for custom metrics" do
+      it "has a value of binding-secret by default" do
+        expect(rendered_template).to include({"default_credential_type" => "binding-secret"})
+      end
+    end
   end
 end

src/acceptance/app/custom_metric_test.go

@@ -25,6 +25,8 @@ var _ = Describe("AutoScaler custom metrics policy", func() {
 	})
 	AfterEach(AppAfterEach)
 
+	// This test will fail if credential-type is set to X509 in autoscaler broker.
+	// Therefore, only mtls connection will be supported for custom metrics in future
 	Context("when scaling by custom metrics", func() {
 		It("should scale out and scale in", Label(acceptance.LabelSmokeTests), func() {
 			By("Scale out to 2 instances")

src/acceptance/assets/file/policy/policy-with-credential-type.json

@@ -0,0 +1,81 @@
+{
+  "credential-type": "x509",
+  "instance_min_count": 1,
+  "instance_max_count": 4,
+  "scaling_rules": [{
+    "metric_type": "memoryused",
+    "breach_duration_secs": 600,
+    "threshold": 30,
+    "operator": "<",
+    "cool_down_secs": 300,
+    "adjustment": "-1"
+  }, {
+    "metric_type": "memoryused",
+    "breach_duration_secs": 600,
+    "threshold": 90,
+    "operator": ">=",
+    "cool_down_secs": 300,
+    "adjustment": "+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
+    }, {
+      "start_date": "2099-06-27",
+      "end_date": "2099-07-23",
+      "start_time": "11:00",
+      "end_time": "19:30",
+      "days_of_month": [
+        5,
+        15,
+        25
+      ],
+      "instance_min_count": 3,
+      "instance_max_count": 10,
+      "initial_min_instance_count": 5
+    }, {
+      "start_time": "10:00",
+      "end_time": "18:00",
+      "days_of_week": [
+        4,
+        5,
+        6
+      ],
+      "instance_min_count": 1,
+      "instance_max_count": 10
+    }, {
+      "start_time": "11:00",
+      "end_time": "19:30",
+      "days_of_month": [
+        10,
+        20,
+        30
+      ],
+      "instance_min_count": 1,
+      "instance_max_count": 10
+    }],
+    "specific_date": [{
+      "start_date_time": "2099-06-02T10:00",
+      "end_date_time": "2099-06-15T13:59",
+      "instance_min_count": 1,
+      "instance_max_count": 4,
+      "initial_min_instance_count": 2
+    }, {
+      "start_date_time": "2099-01-04T20:00",
+      "end_date_time": "2099-02-19T23:15",
+      "instance_min_count": 2,
+      "instance_max_count": 5,
+      "initial_min_instance_count": 3
+    }]
+  }
+}

src/acceptance/broker/broker_test.go

@@ -107,6 +107,27 @@ var _ = Describe("AutoScaler Service Broker", func() {
 			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)
+			Expect(err).NotTo(HaveOccurred())
+
+			err = helpers.BindServiceToAppWithPolicy(cfg, appName, instance.name(), policyFile)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("checking broker bind response does not have username/password/url but mtls_url only ")
+			appEnvCmd := cf.Cf("env", appName).Wait(cfg.DefaultTimeoutDuration())
+			Expect(appEnvCmd).To(Exit(0), "failed getting app env")
+
+			appEnvCmdOutput := appEnvCmd.Out.Contents()
+			Expect(appEnvCmdOutput).NotTo(ContainSubstring("username"))
+			Expect(appEnvCmdOutput).NotTo(ContainSubstring("password"))
+			Expect(appEnvCmdOutput).NotTo(ContainSubstring("\"url\": \"https://"))
+			Expect(appEnvCmdOutput).To(ContainSubstring("\"mtls_url\": \"https://"))
+
+			instance.unbind(appName)
+		})
+
 		It("bind&unbinds without policy", func() {
 			helpers.BindServiceToApp(cfg, appName, instance.name())
 			bindingParameters := helpers.GetServiceCredentialBindingParameters(cfg, instance.name(), appName)

src/autoscaler/api/broker/broker.go

@@ -47,6 +47,7 @@ var (
 	ErrDeletePolicyForUnbinding    = errors.New("failed to delete policy for unbinding")
 	ErrDeleteServiceBinding        = errors.New("error deleting service binding")
 	ErrCredentialNotDeleted        = errors.New("failed to delete custom metrics credential for unbinding")
+	ErrInvalidCredentialType       = errors.New("invalid credential type provided: allowed values are [binding-secret, x509]")
 )
 
 type Errors []error
@@ -510,6 +511,11 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
 		logger.Error("get-default-policy", err)
 		return result, err
 	}
+	defaultCustomMetricsCredentialType := b.conf.DefaultCustomMetricsCredentialType
+	customMetricsBindingAuthScheme, err := getOrDefaultCredentialType(policyJson, defaultCustomMetricsCredentialType, logger)
+	if err != nil {
+		return result, err
+	}
 	policyGuid, err := uuid.NewV4()
 	if err != nil {
 		b.logger.Error("get-default-policy-create-guid", err, lager.Data{"instanceID": instanceID})
@@ -547,18 +553,33 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
 		}
 		return result, apiresponses.NewFailureResponse(ErrCreatingServiceBinding, http.StatusInternalServerError, actionCreateServiceBinding)
 	}
+	customMetricsCredentials := &models.CustomMetricsCredentials{
+		MtlsUrl: b.conf.MetricsForwarder.MetricsForwarderMtlsUrl,
+	}
+	if !isValidCredentialType(customMetricsBindingAuthScheme.CredentialType) {
+		actionValidateCredentialType := "validate-credential-type" // #nosec G101
+		logger.Error("invalid credential-type provided", err, lager.Data{"credential-type": customMetricsBindingAuthScheme.CredentialType})
+		return result, apiresponses.NewFailureResponseBuilder(
+			ErrInvalidCredentialType, http.StatusBadRequest, actionValidateCredentialType).
+			WithErrorKey(actionValidateCredentialType).
+			Build()
+	}
 
-	// create credentials
-	cred, err := b.credentials.Create(ctx, appGUID, nil)
-	if err != nil {
-		//revert binding creating
-		logger.Error("create-credentials", err)
-
-		err = b.bindingdb.DeleteServiceBindingByAppId(ctx, appGUID)
+	if customMetricsBindingAuthScheme.CredentialType == models.BindingSecret {
+		// create credentials
+		cred, err := b.credentials.Create(ctx, appGUID, nil)
 		if err != nil {
-			logger.Error("revert-binding-creation-due-to-credentials-creation-failure", err)
+			//revert binding creating
+			logger.Error("create-credentials", err)
+
+			err = b.bindingdb.DeleteServiceBindingByAppId(ctx, appGUID)
+			if err != nil {
+				logger.Error("revert-binding-creation-due-to-credentials-creation-failure", err)
+			}
+			return result, apiresponses.NewFailureResponse(ErrCreatingServiceBinding, http.StatusInternalServerError, "revert-binding-creation-due-to-credentials-creation-failure")
 		}
-		return result, apiresponses.NewFailureResponse(ErrCreatingServiceBinding, http.StatusInternalServerError, "revert-binding-creation-due-to-credentials-creation-failure")
+		customMetricsCredentials.URL = &b.conf.MetricsForwarder.MetricsForwarderUrl
+		customMetricsCredentials.Credential = cred
 	}
 
 	// attach policy to appGUID
@@ -567,16 +588,24 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
 	}
 
 	result.Credentials = models.Credentials{
-		CustomMetrics: models.CustomMetricsCredentials{
-			Credential: cred,
-			URL:        b.conf.MetricsForwarder.MetricsForwarderUrl,
-			MtlsUrl:    b.conf.MetricsForwarder.MetricsForwarderMtlsUrl,
-		},
+		CustomMetrics: *customMetricsCredentials,
 	}
-
 	return result, nil
 }
 
+func getOrDefaultCredentialType(policyJson json.RawMessage, credentialTypeConfig string, logger lager.Logger) (*models.CustomMetricsBindingAuthScheme, error) {
+	credentialType := &models.CustomMetricsBindingAuthScheme{CredentialType: credentialTypeConfig}
+	if len(policyJson) != 0 {
+		err := json.Unmarshal(policyJson, &credentialType)
+		if err != nil {
+			logger.Error("error: unmarshal-credential-type", err)
+			return nil, apiresponses.NewFailureResponse(ErrCreatingServiceBinding, http.StatusInternalServerError, "error-unmarshal-credential-type")
+		}
+	}
+	logger.Debug("getOrDefaultCredentialType", lager.Data{"credential-type": credentialType})
+	return credentialType, nil
+}
+
 func (b *Broker) attachPolicyToApp(ctx context.Context, appGUID string, policy *models.ScalingPolicy, policyGuidStr string, logger lager.Logger) error {
 	logger = logger.Session("saving-policy-json", lager.Data{"policy": policy})
 	if policy == nil {
@@ -825,3 +854,6 @@ func (b *Broker) deleteBinding(ctx context.Context, bindingId string, serviceIns
 	}
 	return nil
 }
+func isValidCredentialType(credentialType string) bool {
+	return credentialType == models.BindingSecret || credentialType == models.X509Certificate
+}