Increase cf cli to v8 for ABAP steps (#4183)

* Increase docker image version * Add --wait * Test * Adapt to new cf cli * Parse both for cf cli v8 and v7 * Remove input * Adapt to feedback * Check for nil error
2025-10-30 23:57:50 +02:00 · 2023-01-12 09:39:14 +01:00
parent a0698adc33
commit 74b5527f1c
20 changed files with 101 additions and 35 deletions
--- a/cmd/abapEnvironmentCheckoutBranch_generated.go
+++ b/cmd/abapEnvironmentCheckoutBranch_generated.go
@@ -273,7 +273,7 @@ func abapEnvironmentCheckoutBranchMetadata() config.StepData {
 				},
 			},
 			Containers: []config.Container{
-				{Name: "cf", Image: "ppiper/cf-cli:7"},
+				{Name: "cf", Image: "ppiper/cf-cli:v12"},
 			},
 		},
 	}
--- a/cmd/abapEnvironmentCloneGitRepo_generated.go
+++ b/cmd/abapEnvironmentCloneGitRepo_generated.go
@@ -273,7 +273,7 @@ func abapEnvironmentCloneGitRepoMetadata() config.StepData {
 				},
 			},
 			Containers: []config.Container{
-				{Name: "cf", Image: "ppiper/cf-cli:7"},
+				{Name: "cf", Image: "ppiper/cf-cli:v12"},
 			},
 		},
 	}
--- a/cmd/abapEnvironmentCreateSystem_generated.go
+++ b/cmd/abapEnvironmentCreateSystem_generated.go
@@ -349,7 +349,7 @@ func abapEnvironmentCreateSystemMetadata() config.StepData {
 				},
 			},
 			Containers: []config.Container{
-				{Name: "cf", Image: "ppiper/cf-cli:7"},
+				{Name: "cf", Image: "ppiper/cf-cli:v12"},
 			},
 		},
 	}
--- a/cmd/abapEnvironmentCreateTag_generated.go
+++ b/cmd/abapEnvironmentCreateTag_generated.go
@@ -317,7 +317,7 @@ func abapEnvironmentCreateTagMetadata() config.StepData {
 				},
 			},
 			Containers: []config.Container{
-				{Name: "cf", Image: "ppiper/cf-cli:7"},
+				{Name: "cf", Image: "ppiper/cf-cli:v12"},
 			},
 		},
 	}
--- a/cmd/abapEnvironmentPullGitRepo_generated.go
+++ b/cmd/abapEnvironmentPullGitRepo_generated.go
@@ -295,7 +295,7 @@ func abapEnvironmentPullGitRepoMetadata() config.StepData {
 				},
 			},
 			Containers: []config.Container{
-				{Name: "cf", Image: "ppiper/cf-cli:7"},
+				{Name: "cf", Image: "ppiper/cf-cli:v12"},
 			},
 		},
 	}
--- a/cmd/abapEnvironmentPushATCSystemConfig_generated.go
+++ b/cmd/abapEnvironmentPushATCSystemConfig_generated.go
@@ -263,7 +263,7 @@ func abapEnvironmentPushATCSystemConfigMetadata() config.StepData {
 				},
 			},
 			Containers: []config.Container{
-				{Name: "cf", Image: "ppiper/cf-cli:7"},
+				{Name: "cf", Image: "ppiper/cf-cli:v12"},
 			},
 		},
 	}
--- a/cmd/abapEnvironmentRunATCCheck_generated.go
+++ b/cmd/abapEnvironmentRunATCCheck_generated.go
@@ -297,7 +297,7 @@ func abapEnvironmentRunATCCheckMetadata() config.StepData {
 				},
 			},
 			Containers: []config.Container{
-				{Name: "cf", Image: "ppiper/cf-cli:7"},
+				{Name: "cf", Image: "ppiper/cf-cli:v12"},
 			},
 		},
 	}
--- a/cmd/abapEnvironmentRunAUnitTest_generated.go
+++ b/cmd/abapEnvironmentRunAUnitTest_generated.go
@@ -286,7 +286,7 @@ func abapEnvironmentRunAUnitTestMetadata() config.StepData {
 				},
 			},
 			Containers: []config.Container{
-				{Name: "cf", Image: "ppiper/cf-cli:7"},
+				{Name: "cf", Image: "ppiper/cf-cli:v12"},
 			},
 		},
 	}
--- a/cmd/cloudFoundryCreateServiceKey.go
+++ b/cmd/cloudFoundryCreateServiceKey.go
@@ -9,6 +9,8 @@ import (
 	"github.com/SAP/jenkins-library/pkg/telemetry"
 )

+const cfCliSynchronousRequestFlag = "--wait"
+
 func cloudFoundryCreateServiceKey(options cloudFoundryCreateServiceKeyOptions, telemetryData *telemetry.CustomData) {
 	// for command execution use Command
 	c := command.Command{}
@@ -51,10 +53,11 @@ func runCloudFoundryCreateServiceKey(options *cloudFoundryCreateServiceKeyOption
 	log.Entry().Info("Creating Service Key")

 	var cfCreateServiceKeyScript []string
+	// the --wait option was added for cf cli v8 in order to ensure a synchronous creation of the servie key that was default in v7 or earlier
 	if options.CfServiceKeyConfig == "" {
-		cfCreateServiceKeyScript = []string{"create-service-key", options.CfServiceInstance, options.CfServiceKeyName}
+		cfCreateServiceKeyScript = []string{"create-service-key", options.CfServiceInstance, options.CfServiceKeyName, cfCliSynchronousRequestFlag}
 	} else {
-		cfCreateServiceKeyScript = []string{"create-service-key", options.CfServiceInstance, options.CfServiceKeyName, "-c", options.CfServiceKeyConfig}
+		cfCreateServiceKeyScript = []string{"create-service-key", options.CfServiceInstance, options.CfServiceKeyName, "-c", options.CfServiceKeyConfig, cfCliSynchronousRequestFlag}
 	}
 	err := c.RunExecutable("cf", cfCreateServiceKeyScript...)
 	if err != nil {
--- a/cmd/cloudFoundryCreateServiceKey_test.go
+++ b/cmd/cloudFoundryCreateServiceKey_test.go
@@ -29,7 +29,7 @@ func TestCloudFoundryCreateServiceKey(t *testing.T) {
 		error := runCloudFoundryCreateServiceKey(&config, &telemetryData, &execRunner, &cfUtilsMock)
 		if error == nil {
 			assert.Equal(t, "cf", execRunner.Calls[0].Exec)
-			assert.Equal(t, []string{"create-service-key", "testInstance", "testKey"}, execRunner.Calls[0].Params)
+			assert.Equal(t, []string{"create-service-key", "testInstance", "testKey", cfCliSynchronousRequestFlag}, execRunner.Calls[0].Params)
 		}
 	})
 	t.Run("CF Create Service Key with service Key config: Success case", func(t *testing.T) {
@@ -50,7 +50,7 @@ func TestCloudFoundryCreateServiceKey(t *testing.T) {
 		error := runCloudFoundryCreateServiceKey(&config, &telemetryData, &execRunner, &cfUtilsMock)
 		if error == nil {
 			assert.Equal(t, "cf", execRunner.Calls[0].Exec)
-			assert.Equal(t, []string{"create-service-key", "testInstance", "testKey", "-c", "testconfig.yml"}, execRunner.Calls[0].Params)
+			assert.Equal(t, []string{"create-service-key", "testInstance", "testKey", "-c", "testconfig.yml", cfCliSynchronousRequestFlag}, execRunner.Calls[0].Params)
 		}
 	})
 	t.Run("CF Create Service Key with service Key config: Success case", func(t *testing.T) {
@@ -71,7 +71,7 @@ func TestCloudFoundryCreateServiceKey(t *testing.T) {
 		error := runCloudFoundryCreateServiceKey(&config, &telemetryData, &execRunner, &cfUtilsMock)
 		if error == nil {
 			assert.Equal(t, "cf", execRunner.Calls[0].Exec)
-			assert.Equal(t, []string{"create-service-key", "testInstance", "testKey", "-c", "{\"scenario_id\":\"SAP_COM_0510\",\"type\":\"basic\"}"}, execRunner.Calls[0].Params)
+			assert.Equal(t, []string{"create-service-key", "testInstance", "testKey", "-c", "{\"scenario_id\":\"SAP_COM_0510\",\"type\":\"basic\"}", cfCliSynchronousRequestFlag}, execRunner.Calls[0].Params)
 		}
 	})
 }
@@ -134,7 +134,7 @@ func TestCloudFoundryCreateServiceKeyErrorMessages(t *testing.T) {
 			CfServiceKeyConfig: "{\"scenario_id\":\"SAP_COM_0510\",\"type\":\"basic\"}",
 		}
 		m := make(map[string]error)
-		m["cf create-service-key testInstance testKey -c {\"scenario_id\":\"SAP_COM_0510\",\"type\":\"basic\"}"] = errors.New(errorMessage)
+		m["cf create-service-key testInstance testKey -c {\"scenario_id\":\"SAP_COM_0510\",\"type\":\"basic\"} --wait"] = errors.New(errorMessage)
 		execRunner := mock.ExecMockRunner{
 			ShouldFailOnCommand: m,
 		}
--- a/pkg/abaputils/abaputils.go
+++ b/pkg/abaputils/abaputils.go
@@ -80,6 +80,7 @@ func (abaputils *AbapUtils) GetAbapCommunicationArrangementInfo(options AbapEnvi
 // ReadServiceKeyAbapEnvironment from Cloud Foundry and returns it. Depending on user/developer requirements if he wants to perform further Cloud Foundry actions
 func ReadServiceKeyAbapEnvironment(options AbapEnvironmentOptions, c command.ExecRunner) (AbapServiceKey, error) {

+	var abapServiceKeyV8 AbapServiceKeyV8
 	var abapServiceKey AbapServiceKey
 	var serviceKeyJSON string
 	var err error
@@ -100,18 +101,33 @@ func ReadServiceKeyAbapEnvironment(options AbapEnvironmentOptions, c command.Exe

 	if err != nil {
 		// Executing cfReadServiceKeyScript failed
-		return abapServiceKey, err
+		return abapServiceKeyV8.Credentials, err
 	}

-	// parse
-	json.Unmarshal([]byte(serviceKeyJSON), &abapServiceKey)
+	// Depending on the cf cli version, the service key may be returned in a different format. For compatibility reason, both formats are supported
+	unmarshalErrorV8 := json.Unmarshal([]byte(serviceKeyJSON), &abapServiceKeyV8)
+	if abapServiceKeyV8 == (AbapServiceKeyV8{}) {
+		if unmarshalErrorV8 != nil {
+			log.Entry().Debug(unmarshalErrorV8.Error())
+		}
+		log.Entry().Debug("Could not parse the service key in the cf cli v8 format.")
+	} else {
+		log.Entry().Info("Service Key read successfully")
+		return abapServiceKeyV8.Credentials, nil
+	}
+
+	unmarshalError := json.Unmarshal([]byte(serviceKeyJSON), &abapServiceKey)
 	if abapServiceKey == (AbapServiceKey{}) {
-		log.SetErrorCategory(log.ErrorInfrastructure)
-		return abapServiceKey, errors.New("Parsing the service key failed. Service key is empty")
+		if unmarshalError != nil {
+			log.Entry().Debug(unmarshalError.Error())
+		}
+		log.Entry().Debug("Could not parse the service key in the cf cli v7 format.")
+	} else {
+		log.Entry().Info("Service Key read successfully")
+		return abapServiceKey, nil
 	}
-
-	log.Entry().Info("Service Key read successfully")
-	return abapServiceKey, nil
+	log.SetErrorCategory(log.ErrorInfrastructure)
+	return abapServiceKeyV8.Credentials, errors.New("Parsing the service key failed for all supported formats. Service key is empty")
 }

 /*
@@ -287,6 +303,12 @@ type AbapErrorMessage struct {
 	Value string `json:"value"`
 }

+// AbapServiceKeyV8 contains the new format of an ABAP service key
+
+type AbapServiceKeyV8 struct {
+	Credentials AbapServiceKey `json:"credentials"`
+}
+
 // AbapServiceKey contains information about an ABAP service key
 type AbapServiceKey struct {
 	SapCloudService    string         `json:"sap.cloud.service"`
--- a/pkg/abaputils/abaputils_test.go
+++ b/pkg/abaputils/abaputils_test.go
@@ -69,9 +69,50 @@ func TestCloudFoundryGetAbapCommunicationInfo(t *testing.T) {
 		assert.Equal(t, "", connectionDetails.Password)
 		assert.Equal(t, "", connectionDetails.XCsrfToken)

-		assert.EqualError(t, err, "Read service key failed: Parsing the service key failed. Service key is empty")
+		assert.EqualError(t, err, "Read service key failed: Parsing the service key failed for all supported formats. Service key is empty")
 	})
-	t.Run("CF GetAbapCommunicationArrangementInfo - Success", func(t *testing.T) {
+	t.Run("CF GetAbapCommunicationArrangementInfo - Success V8", func(t *testing.T) {
+
+		//given
+
+		const testURL = "https://testurl.com"
+		const oDataURL = "/sap/opu/odata/sap/MANAGE_GIT_REPOSITORY/Pull"
+		const username = "test_user"
+		const password = "test_password"
+		const serviceKey = `
+		cf comment test \n\n
+		{ "credentials": {"sap.cloud.service":"com.sap.cloud.abap","url": "` + testURL + `" ,"systemid":"H01","abap":{"username":"` + username + `","password":"` + password + `","communication_scenario_id": "SAP_COM_0510","communication_arrangement_id": "SK_I6CBIRFZPPJDKYNATQA32W","communication_system_id": "SK_I6CBIRFZPPJDKYNATQA32W","communication_inbound_user_id": "CC0000000001","communication_inbound_user_auth_mode": "2"},"binding":{"env": "cf","version": "0.0.1.1","type": "basic","id": "i6cBiRfZppJdKynaTqa32W"},"preserve_host_header": true} }`
+
+		options := AbapEnvironmentOptions{
+			CfAPIEndpoint:     "https://api.endpoint.com",
+			CfSpace:           "testSpace",
+			CfOrg:             "testOrg",
+			CfServiceInstance: "testInstance",
+			Username:          "testUser",
+			Password:          "testPassword",
+			CfServiceKeyName:  "testServiceKeyName",
+		}
+
+		m := &mock.ExecMockRunner{}
+		m.StdoutReturn = map[string]string{"cf service-key testInstance testServiceKeyName": serviceKey}
+		var autils = AbapUtils{
+			Exec: m,
+		}
+		//when
+		var connectionDetails ConnectionDetailsHTTP
+		var err error
+		connectionDetails, err = autils.GetAbapCommunicationArrangementInfo(options, oDataURL)
+
+		//then
+		assert.Equal(t, testURL+oDataURL, connectionDetails.URL)
+		assert.Equal(t, username, connectionDetails.User)
+		assert.Equal(t, password, connectionDetails.Password)
+		assert.Equal(t, "", connectionDetails.XCsrfToken)
+
+		assert.NoError(t, err)
+	})
+
+	t.Run("CF GetAbapCommunicationArrangementInfo - Success V7", func(t *testing.T) {

 		//given

@@ -124,7 +165,7 @@ func TestHostGetAbapCommunicationInfo(t *testing.T) {
 		const password = "test_password"
 		const serviceKey = `
 		cf comment test \n\n
-		{"sap.cloud.service":"com.sap.cloud.abap","url": "` + testURL + `" ,"systemid":"XYZ","abap":{"username":"` + username + `","password":"` + password + `","communication_scenario_id": "SAP_COM_XYZ","communication_arrangement_id": "SK_testing","communication_system_id": "SK_testing","communication_inbound_user_id": "CC0000000000","communication_inbound_user_auth_mode": "2"},"binding":{"env": "cf","version": "0.0.1.1","type": "basic","id": "i6cBiRfZppJdtestKynaTqa32W"},"preserve_host_header": true}`
+		{ "credentials": {"sap.cloud.service":"com.sap.cloud.abap","url": "` + testURL + `" ,"systemid":"XYZ","abap":{"username":"` + username + `","password":"` + password + `","communication_scenario_id": "SAP_COM_XYZ","communication_arrangement_id": "SK_testing","communication_system_id": "SK_testing","communication_inbound_user_id": "CC0000000000","communication_inbound_user_auth_mode": "2"},"binding":{"env": "cf","version": "0.0.1.1","type": "basic","id": "i6cBiRfZppJdtestKynaTqa32W"},"preserve_host_header": true} }`

 		options := AbapEnvironmentOptions{
 			Host:     testURL,
@@ -161,7 +202,7 @@ func TestHostGetAbapCommunicationInfo(t *testing.T) {
 		const password = "test_password"
 		const serviceKey = `
 		cf comment test \n\n
-		{"sap.cloud.service":"com.sap.cloud.abap","url": "` + testURL + `" ,"systemid":"H01","abap":{"username":"` + username + `","password":"` + password + `","communication_scenario_id": "SAP_COM_0510","communication_arrangement_id": "SK_I6CBIRFZPPJDKYNATQA32W","communication_system_id": "SK_I6CBIRFZPPJDKYNATQA32W","communication_inbound_user_id": "CC0000000001","communication_inbound_user_auth_mode": "2"},"binding":{"env": "cf","version": "0.0.1.1","type": "basic","id": "i6cBiRfZppJdKynaTqa32W"},"preserve_host_header": true}`
+		{ "credentials": {"sap.cloud.service":"com.sap.cloud.abap","url": "` + testURL + `" ,"systemid":"H01","abap":{"username":"` + username + `","password":"` + password + `","communication_scenario_id": "SAP_COM_0510","communication_arrangement_id": "SK_I6CBIRFZPPJDKYNATQA32W","communication_system_id": "SK_I6CBIRFZPPJDKYNATQA32W","communication_inbound_user_id": "CC0000000001","communication_inbound_user_auth_mode": "2"},"binding":{"env": "cf","version": "0.0.1.1","type": "basic","id": "i6cBiRfZppJdKynaTqa32W"},"preserve_host_header": true} }`

 		options := AbapEnvironmentOptions{
 			Host:     testURL,
@@ -222,7 +263,7 @@ func TestReadServiceKeyAbapEnvironment(t *testing.T) {
 		assert.Equal(t, "", abapKey.SystemID)
 		assert.Equal(t, "", abapKey.URL)

-		assert.EqualError(t, err, "Parsing the service key failed. Service key is empty")
+		assert.EqualError(t, err, "Parsing the service key failed for all supported formats. Service key is empty")
 	})
 }

--- a/resources/metadata/abapEnvironmentCheckoutBranch.yaml
+++ b/resources/metadata/abapEnvironmentCheckoutBranch.yaml
@@ -129,4 +129,4 @@ spec:
          - name: cfServiceKeyName
  containers:
    - name: cf
-      image: ppiper/cf-cli:7
+      image: ppiper/cf-cli:v12
--- a/resources/metadata/abapEnvironmentCloneGitRepo.yaml
+++ b/resources/metadata/abapEnvironmentCloneGitRepo.yaml
@@ -129,4 +129,4 @@ spec:
          - name: cfServiceKey
  containers:
    - name: cf
-      image: ppiper/cf-cli:7
+      image: ppiper/cf-cli:v12
--- a/resources/metadata/abapEnvironmentCreateSystem.yaml
+++ b/resources/metadata/abapEnvironmentCreateSystem.yaml
@@ -192,4 +192,4 @@ spec:
        default: false
  containers:
    - name: cf
-      image: ppiper/cf-cli:7
+      image: ppiper/cf-cli:v12
--- a/resources/metadata/abapEnvironmentCreateTag.yaml
+++ b/resources/metadata/abapEnvironmentCreateTag.yaml
@@ -159,4 +159,4 @@ spec:
          - name: cfServiceKey
  containers:
    - name: cf
-      image: ppiper/cf-cli:7
+      image: ppiper/cf-cli:v12
--- a/resources/metadata/abapEnvironmentPullGitRepo.yaml
+++ b/resources/metadata/abapEnvironmentPullGitRepo.yaml
@@ -142,4 +142,4 @@ spec:
        default: false
  containers:
    - name: cf
-      image: ppiper/cf-cli:7
+      image: ppiper/cf-cli:v12
--- a/resources/metadata/abapEnvironmentPushATCSystemConfig.yaml
+++ b/resources/metadata/abapEnvironmentPushATCSystemConfig.yaml
@@ -129,4 +129,4 @@ spec:

  containers:
    - name: cf
-      image: ppiper/cf-cli:7
+      image: ppiper/cf-cli:v12
--- a/resources/metadata/abapEnvironmentRunATCCheck.yaml
+++ b/resources/metadata/abapEnvironmentRunATCCheck.yaml
@@ -156,4 +156,4 @@ spec:
        mandatory: false
  containers:
    - name: cf
-      image: ppiper/cf-cli:7
+      image: ppiper/cf-cli:v12
--- a/resources/metadata/abapEnvironmentRunAUnitTest.yaml
+++ b/resources/metadata/abapEnvironmentRunAUnitTest.yaml
@@ -147,4 +147,4 @@ spec:
        mandatory: false
  containers:
    - name: cf
-      image: ppiper/cf-cli:7
+      image: ppiper/cf-cli:v12