Change to default cf create-service implementation (#4224)

* Change to default cf create-service

* Adapt test

* Adapt tests

* Remove comment

---------

Co-authored-by: tiloKo <70266685+tiloKo@users.noreply.github.com>

cmd/abapEnvironmentCreateSystem.go

@@ -3,15 +3,12 @@ package cmd
 import (
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
-	"os"
 
 	"github.com/SAP/jenkins-library/pkg/abaputils"
 	"github.com/SAP/jenkins-library/pkg/cloudfoundry"
 	"github.com/SAP/jenkins-library/pkg/command"
 	"github.com/SAP/jenkins-library/pkg/log"
 	"github.com/SAP/jenkins-library/pkg/telemetry"
-	"github.com/ghodss/yaml"
 	"github.com/google/uuid"
 )
 
@@ -41,37 +38,28 @@ func runAbapEnvironmentCreateSystem(config *abapEnvironmentCreateSystemOptions,
 		}
 		return runCloudFoundryCreateService(&createServiceConfig, telemetryData, cf)
 	}
-	// if no manifest file is provided, it is created with the provided config values
-	manifestYAML, err := generateManifestYAML(config)
+
+	cfConfig, err := generateServiceParameterString(config)
 	if err != nil {
-		return err
+		log.Entry().Fatalf("Could not generate parameter string")
 	}
-
-	// writing the yaml into a temporary file
-	path, _ := os.Getwd()
-	path = path + "/generated_service_manifest-" + u.getUUID() + ".yml"
-	log.Entry().Debugf("Path: %s", path)
-	err = ioutil.WriteFile(path, manifestYAML, 0644)
-	if err != nil {
-		log.SetErrorCategory(log.ErrorConfiguration)
-		return fmt.Errorf("%s: %w", "Could not generate manifest file for the cloud foundry cli", err)
-	}
-
-	defer os.Remove(path)
-
 	// Calling cloudFoundryCreateService with the respective parameters
 	createServiceConfig := cloudFoundryCreateServiceOptions{
-		CfAPIEndpoint:   config.CfAPIEndpoint,
-		CfOrg:           config.CfOrg,
-		CfSpace:         config.CfSpace,
-		Username:        config.Username,
-		Password:        config.Password,
-		ServiceManifest: path,
+		CfAPIEndpoint:         config.CfAPIEndpoint,
+		CfOrg:                 config.CfOrg,
+		CfSpace:               config.CfSpace,
+		Username:              config.Username,
+		Password:              config.Password,
+		CfService:             config.CfService,
+		CfServicePlan:         config.CfServicePlan,
+		CfServiceInstanceName: config.CfServiceInstance,
+		CfCreateServiceConfig: cfConfig,
+		CfAsync:               false,
 	}
 	return runCloudFoundryCreateService(&createServiceConfig, telemetryData, cf)
 }
 
-func generateManifestYAML(config *abapEnvironmentCreateSystemOptions) ([]byte, error) {
+func generateServiceParameterString(config *abapEnvironmentCreateSystemOptions) (string, error) {
 	addonProduct := ""
 	addonVersion := ""
 	parentSaaSAppName := ""
@@ -79,7 +67,7 @@ func generateManifestYAML(config *abapEnvironmentCreateSystemOptions) ([]byte, e
 		descriptor, err := abaputils.ReadAddonDescriptor(config.AddonDescriptorFileName)
 		if err != nil {
 			log.SetErrorCategory(log.ErrorConfiguration)
-			return nil, fmt.Errorf("Cloud not read addonProduct and addonVersion from %s: %w", config.AddonDescriptorFileName, err)
+			return "", fmt.Errorf("Cloud not read addonProduct and addonVersion from %s: %w", config.AddonDescriptorFileName, err)
 		}
 		addonProduct = descriptor.AddonProduct
 		addonVersion = descriptor.AddonVersionYAML
@@ -103,37 +91,10 @@ func generateManifestYAML(config *abapEnvironmentCreateSystemOptions) ([]byte, e
 	log.Entry().Debugf("Service Parameters: %s", serviceParametersString)
 	if err != nil {
 		log.SetErrorCategory(log.ErrorConfiguration)
-		return nil, fmt.Errorf("Could not generate parameter string for the cloud foundry cli: %w", err)
+		return "", fmt.Errorf("Could not generate parameter string for the cloud foundry cli: %w", err)
 	}
 
-	/*
-		Generating the temporary manifest yaml file
-	*/
-	service := Service{
-		Name:       config.CfServiceInstance,
-		Broker:     config.CfService,
-		Plan:       config.CfServicePlan,
-		Parameters: serviceParametersString,
-	}
-
-	serviceManifest := serviceManifest{CreateServices: []Service{service}}
-	errorMessage := "Could not generate manifest for the cloud foundry cli"
-
-	// converting the golang structure to json
-	manifestJSON, err := json.Marshal(serviceManifest)
-	if err != nil {
-		return nil, fmt.Errorf("%s: %w", errorMessage, err)
-	}
-
-	// converting the json to yaml
-	manifestYAML, err := yaml.JSONToYAML(manifestJSON)
-	if err != nil {
-		return nil, fmt.Errorf("%s: %w", errorMessage, err)
-	}
-
-	log.Entry().Debug(string(manifestYAML))
-
-	return manifestYAML, nil
+	return serviceParametersString, nil
 }
 
 type abapSystemParameters struct {

cmd/abapEnvironmentCreateSystem_test.go

@@ -15,7 +15,7 @@ func TestRunAbapEnvironmentCreateSystem(t *testing.T) {
 	cf := cloudfoundry.CFUtils{Exec: m}
 	u := &uuidMock{}
 
-	t.Run("Create service with generated manifest", func(t *testing.T) {
+	t.Run("Create service with cf create-service", func(t *testing.T) {
 		defer cfMockCleanup(m)
 		config := abapEnvironmentCreateSystemOptions{
 			CfAPIEndpoint:     "https://api.endpoint.com",
@@ -27,12 +27,11 @@ func TestRunAbapEnvironmentCreateSystem(t *testing.T) {
 			CfServiceInstance: "testName",
 			CfServicePlan:     "testPlan",
 		}
-		wd, _ := os.Getwd()
 		err := runAbapEnvironmentCreateSystem(&config, nil, cf, u)
 		if assert.NoError(t, err) {
 			assert.Equal(t, []mock.ExecCall{
 				{Execution: (*mock.Execution)(nil), Async: false, Exec: "cf", Params: []string{"login", "-a", "https://api.endpoint.com", "-o", "testOrg", "-s", "testSpace", "-u", "testUser", "-p", "testPassword"}},
-				{Execution: (*mock.Execution)(nil), Async: false, Exec: "cf", Params: []string{"create-service-push", "--no-push", "--service-manifest", wd + "/generated_service_manifest-my-uuid.yml"}},
+				{Execution: (*mock.Execution)(nil), Async: false, Exec: "cf", Params: []string{"create-service", config.CfService, config.CfServicePlan, config.CfServiceInstance, "-c", "{\"is_development_allowed\":false}", "--wait"}},
 				{Execution: (*mock.Execution)(nil), Async: false, Exec: "cf", Params: []string{"logout"}}},
 				m.Calls)
 		}
@@ -88,57 +87,7 @@ func TestRunAbapEnvironmentCreateSystem(t *testing.T) {
 
 func TestManifestGeneration(t *testing.T) {
 
-	t.Run("Create service with generated manifest", func(t *testing.T) {
-		config := abapEnvironmentCreateSystemOptions{
-			CfAPIEndpoint:                  "https://api.endpoint.com",
-			CfOrg:                          "testOrg",
-			CfSpace:                        "testSpace",
-			Username:                       "testUser",
-			Password:                       "testPassword",
-			CfService:                      "testService",
-			CfServiceInstance:              "testName",
-			CfServicePlan:                  "testPlan",
-			AbapSystemAdminEmail:           "user@example.com",
-			AbapSystemID:                   "H02",
-			AbapSystemIsDevelopmentAllowed: true,
-			AbapSystemSizeOfPersistence:    4,
-			AbapSystemSizeOfRuntime:        4,
-			AddonDescriptorFileName:        "addon.yml",
-		}
-
-		dir := t.TempDir()
-		oldCWD, _ := os.Getwd()
-		_ = os.Chdir(dir)
-		// clean up tmp dir
-		defer func() {
-			_ = os.Chdir(oldCWD)
-		}()
-
-		addonYML := `addonProduct: myProduct
-addonVersion: 1.2.3
-repositories:
-  - name: '/DMO/REPO'
-`
-
-		addonYMLBytes := []byte(addonYML)
-		err := ioutil.WriteFile("addon.yml", addonYMLBytes, 0644)
-
-		expectedResult := `create-services:
-- broker: testService
-  name: testName
-  parameters: '{"admin_email":"user@example.com","is_development_allowed":true,"sapsystemname":"H02","size_of_persistence":4,"size_of_runtime":4}'
-  plan: testPlan
-`
-
-		resultBytes, err := generateManifestYAML(&config)
-
-		if assert.NoError(t, err) {
-			result := string(resultBytes)
-			assert.Equal(t, expectedResult, result, "Result not as expected")
-		}
-	})
-
-	t.Run("Create service with generated manifest - with addon", func(t *testing.T) {
+	t.Run("Create service with addon - development", func(t *testing.T) {
 		config := abapEnvironmentCreateSystemOptions{
 			CfAPIEndpoint:                  "https://api.endpoint.com",
 			CfOrg:                          "testOrg",
@@ -174,17 +123,11 @@ repositories:
 		addonYMLBytes := []byte(addonYML)
 		err := ioutil.WriteFile("addon.yml", addonYMLBytes, 0644)
 
-		expectedResult := `create-services:
-- broker: testService
-  name: testName
-  parameters: '{"admin_email":"user@example.com","is_development_allowed":true,"sapsystemname":"H02","size_of_persistence":4,"size_of_runtime":4,"addon_product_name":"myProduct","addon_product_version":"1.2.3","parent_saas_appname":"addon_test"}'
-  plan: testPlan
-`
+		expectedResult := "{\"admin_email\":\"user@example.com\",\"is_development_allowed\":true,\"sapsystemname\":\"H02\",\"size_of_persistence\":4,\"size_of_runtime\":4,\"addon_product_name\":\"myProduct\",\"addon_product_version\":\"1.2.3\",\"parent_saas_appname\":\"addon_test\"}"
 
-		resultBytes, err := generateManifestYAML(&config)
+		result, err := generateServiceParameterString(&config)
 
 		if assert.NoError(t, err) {
-			result := string(resultBytes)
 			assert.Equal(t, expectedResult, result, "Result not as expected")
 		}
 	})
@@ -224,22 +167,17 @@ repositories:
 		addonYMLBytes := []byte(addonYML)
 		err := ioutil.WriteFile("addon.yml", addonYMLBytes, 0644)
 
-		expectedResult := `create-services:
-- broker: testService
-  name: testName
-  parameters: '{"admin_email":"user@example.com","is_development_allowed":true,"sapsystemname":"H02","size_of_persistence":4,"size_of_runtime":4}'
-  plan: testPlan
-`
+		expectedResult := "{\"admin_email\":\"user@example.com\",\"is_development_allowed\":true,\"sapsystemname\":\"H02\",\"size_of_persistence\":4,\"size_of_runtime\":4}"
 
-		resultBytes, err := generateManifestYAML(&config)
+		result, err := generateServiceParameterString(&config)
 
 		if assert.NoError(t, err) {
-			result := string(resultBytes)
 			assert.Equal(t, expectedResult, result, "Result not as expected")
 		}
 	})
 
-	t.Run("Create service with generated manifest - with addon", func(t *testing.T) {
+	t.Run("Create service with addon - no development", func(t *testing.T) {
+
 		config := abapEnvironmentCreateSystemOptions{
 			CfAPIEndpoint:                  "https://api.endpoint.com",
 			CfOrg:                          "testOrg",
@@ -275,17 +213,11 @@ repositories:
 		addonYMLBytes := []byte(addonYML)
 		err := ioutil.WriteFile("addon.yml", addonYMLBytes, 0644)
 
-		expectedResult := `create-services:
-- broker: testService
-  name: testName
-  parameters: '{"admin_email":"user@example.com","is_development_allowed":false,"sapsystemname":"H02","size_of_persistence":4,"size_of_runtime":4,"addon_product_name":"myProduct","addon_product_version":"1.2.3","parent_saas_appname":"addon_test"}'
-  plan: testPlan
-`
+		expectedResult := "{\"admin_email\":\"user@example.com\",\"is_development_allowed\":false,\"sapsystemname\":\"H02\",\"size_of_persistence\":4,\"size_of_runtime\":4,\"addon_product_name\":\"myProduct\",\"addon_product_version\":\"1.2.3\",\"parent_saas_appname\":\"addon_test\"}"
 
-		resultBytes, err := generateManifestYAML(&config)
+		result, err := generateServiceParameterString(&config)
 
 		if assert.NoError(t, err) {
-			result := string(resultBytes)
 			assert.Equal(t, expectedResult, result, "Result not as expected")
 		}
 	})

cmd/cloudFoundryCreateService.go

@@ -73,6 +73,9 @@ func cloudFoundryCreateServiceRequest(config *cloudFoundryCreateServiceOptions,
 	if config.CfServiceTags != "" {
 		cfCreateServiceScript = append(cfCreateServiceScript, "-t", config.CfServiceTags)
 	}
+	if !config.CfAsync {
+		cfCreateServiceScript = append(cfCreateServiceScript, "--wait")
+	}
 	if config.ServiceManifest != "" && fileExists(config.ServiceManifest) {
 
 		cfCreateServiceScript = []string{"create-service-push", "--no-push", "--service-manifest", config.ServiceManifest}

cmd/cloudFoundryCreateService_generated.go

@@ -30,6 +30,7 @@ type cloudFoundryCreateServiceOptions struct {
 	ServiceManifest        string   `json:"serviceManifest,omitempty"`
 	ManifestVariables      []string `json:"manifestVariables,omitempty"`
 	ManifestVariablesFiles []string `json:"manifestVariablesFiles,omitempty"`
+	CfAsync                bool     `json:"cfAsync,omitempty"`
 }
 
 // CloudFoundryCreateServiceCommand Creates one or multiple Services in Cloud Foundry
@@ -147,6 +148,7 @@ func addCloudFoundryCreateServiceFlags(cmd *cobra.Command, stepConfig *cloudFoun
 	cmd.Flags().StringVar(&stepConfig.ServiceManifest, "serviceManifest", `service-manifest.yml`, "Path to Cloud Foundry Service Manifest in YAML format for multiple service creations that are being passed to a Create-Service-Push Cloud Foundry cli plugin")
 	cmd.Flags().StringSliceVar(&stepConfig.ManifestVariables, "manifestVariables", []string{}, "Defines a List of variables as key-value Map objects used for variable substitution within the file given by the Manifest. Defaults to an empty list, if not specified otherwise. This can be used to set variables like it is provided by `cf push --var key=value`. The order of the maps of variables given in the list is relevant in case there are conflicting variable names and values between maps contained within the list. In case of conflicts, the last specified map in the list will win. Though each map entry in the list can contain more than one key-value pair for variable substitution, it is recommended to stick to one entry per map, and rather declare more maps within the list. The reason is that if a map in the list contains more than one key-value entry, and the entries are conflicting, the conflict resolution behavior is undefined (since map entries have no sequence). Variables defined via `manifestVariables` always win over conflicting variables defined via any file given by `manifestVariablesFiles` - no matter what is declared before. This is the same behavior as can be observed when using `cf push --var` in combination with `cf push --vars-file`")
 	cmd.Flags().StringSliceVar(&stepConfig.ManifestVariablesFiles, "manifestVariablesFiles", []string{}, "Defines the manifest variables Yaml files to be used to replace variable references in manifest. This parameter is optional and will default to `manifest-variables.yml`. This can be used to set variable files like it is provided by `cf push --vars-file <file>`. If the manifest is present and so are all variable files, a variable substitution will be triggered that uses the `cfManifestSubstituteVariables` step before deployment. The format of variable references follows the Cloud Foundry standard in `https://docs.cloudfoundry.org/devguide/deploy-apps/manifest-attributes.html#variable-substitution`")
+	cmd.Flags().BoolVar(&stepConfig.CfAsync, "cfAsync", true, "Decides if the service creation runs asynchronously")
 
 	cmd.MarkFlagRequired("cfApiEndpoint")
 	cmd.MarkFlagRequired("username")
@@ -322,6 +324,15 @@ func cloudFoundryCreateServiceMetadata() config.StepData {
 						Aliases:     []config.Alias{{Name: "cloudFoundry/manifestVariablesFiles"}, {Name: "cfManifestVariablesFiles"}},
 						Default:     []string{},
 					},
+					{
+						Name:        "cfAsync",
+						ResourceRef: []config.ResourceReference{},
+						Scope:       []string{"GENERAL", "PARAMETERS", "STAGES", "STEPS"},
+						Type:        "bool",
+						Mandatory:   false,
+						Aliases:     []config.Alias{},
+						Default:     true,
+					},
 				},
 			},
 			Containers: []config.Container{

cmd/cloudFoundryCreateService_test.go

@@ -34,11 +34,12 @@ func TestCloudFoundryCreateService(t *testing.T) {
 			CfService:             "testService",
 			CfServiceInstanceName: "testName",
 			CfServicePlan:         "testPlan",
+			CfAsync:               false,
 		}
 		error := runCloudFoundryCreateService(&config, &telemetryData, cf)
 		if assert.NoError(t, error) {
 			assert.Equal(t, []mock.ExecCall{{Execution: (*mock.Execution)(nil), Async: false, Exec: "cf", Params: []string{"login", "-a", "https://api.endpoint.com", "-o", "testOrg", "-s", "testSpace", "-u", "testUser", "-p", "testPassword"}},
-				{Execution: (*mock.Execution)(nil), Async: false, Exec: "cf", Params: []string{"create-service", "testService", "testPlan", "testName"}},
+				{Execution: (*mock.Execution)(nil), Async: false, Exec: "cf", Params: []string{"create-service", "testService", "testPlan", "testName", "--wait"}},
 				{Execution: (*mock.Execution)(nil), Async: false, Exec: "cf", Params: []string{"logout"}}},
 				m.Calls)
 		}
@@ -56,6 +57,7 @@ func TestCloudFoundryCreateService(t *testing.T) {
 			CfServiceInstanceName: "testName",
 			CfServicePlan:         "testPlan",
 			CfServiceTags:         "testTag, testTag2",
+			CfAsync:               true,
 		}
 		error := runCloudFoundryCreateService(&config, &telemetryData, cf)
 		if assert.NoError(t, error) {
@@ -77,6 +79,7 @@ func TestCloudFoundryCreateService(t *testing.T) {
 			CfServiceInstanceName: "testName",
 			CfServicePlan:         "testPlan",
 			CfServiceBroker:       "testBroker",
+			CfAsync:               true,
 		}
 		error := runCloudFoundryCreateService(&config, &telemetryData, cf)
 		if assert.NoError(t, error) {
@@ -98,6 +101,7 @@ func TestCloudFoundryCreateService(t *testing.T) {
 			CfServiceInstanceName: "testName",
 			CfServicePlan:         "testPlan",
 			CfCreateServiceConfig: "testConfig.json",
+			CfAsync:               true,
 		}
 		error := runCloudFoundryCreateService(&config, &telemetryData, cf)
 		if assert.NoError(t, error) {
@@ -126,17 +130,17 @@ func TestCloudFoundryCreateService(t *testing.T) {
 			_ = os.Chdir(oldCWD)
 		}()
 
-		manifestFileString := `  
+		manifestFileString := `
 		---
 		create-services:
 		- name:   ((name))
 		  broker: "testBroker"
 		  plan:   "testPlan"
-		
+
 		- name:   ((name2))
 		  broker: "testBroker"
 		  plan:   "testPlan"
-		
+
 		- name:   "test3"
 		  broker: "testBroker"
 		  plan:   "testPlan"`
@@ -155,6 +159,7 @@ func TestCloudFoundryCreateService(t *testing.T) {
 			Password:          "testPassword",
 			ServiceManifest:   "manifestTest.yml",
 			ManifestVariables: manifestVariables,
+			CfAsync:           false, // should be ignored
 		}
 		error := runCloudFoundryCreateService(&config, &telemetryData, cf)
 		if assert.NoError(t, error) {
@@ -178,17 +183,17 @@ func TestCloudFoundryCreateService(t *testing.T) {
 		varsFileString := `name: test1
 		name2: test2`
 
-		manifestFileString := `  
+		manifestFileString := `
 		---
 		create-services:
 		- name:   ((name))
 		  broker: "testBroker"
 		  plan:   "testPlan"
-		
+
 		- name:   ((name2))
 		  broker: "testBroker"
 		  plan:   "testPlan"
-		
+
 		- name:   "test3"
 		  broker: "testBroker"
 		  plan:   "testPlan"`

resources/metadata/cloudFoundryCreateService.yaml

@@ -190,6 +190,16 @@ spec:
         aliases:
           - name: cloudFoundry/manifestVariablesFiles
           - name: cfManifestVariablesFiles
+      - name: cfAsync
+        type: bool
+        description: Decides if the service creation runs asynchronously
+        scope:
+          - GENERAL
+          - PARAMETERS
+          - STAGES
+          - STEPS
+        mandatory: false
+        default: true
   containers:
     - name: cf
       image: ppiper/cf-cli:latest