abapAddonAssemblyKitReserveNextPackages additional checks & move stage (#3247)

* new checks for commitIDs * new checks for commitIDs * relocate step from build stage to initial checks + refac * log list * fix log + check * log format * fix unit tests Co-authored-by: Christian Luttenberger <42861202+bluesbrother84@users.noreply.github.com>
2025-02-03 13:21:41 +02:00 · 2021-11-04 22:04:00 +01:00 · 2021-11-04 22:04:00 +01:00 · f1a5b6a918
commit f1a5b6a918
parent aa9fbdf241
7 changed files with 135 additions and 30 deletions
--- a/cmd/abapAddonAssemblyKitReserveNextPackages.go
+++ b/cmd/abapAddonAssemblyKitReserveNextPackages.go
@ -1,7 +1,6 @@
 package cmd

 import (
-	"encoding/json"
 	"fmt"
 	"time"

@ -34,11 +33,16 @@ func abapAddonAssemblyKitReserveNextPackages(config abapAddonAssemblyKitReserveN

 func runAbapAddonAssemblyKitReserveNextPackages(config *abapAddonAssemblyKitReserveNextPackagesOptions, telemetryData *telemetry.CustomData, client piperhttp.Sender,
 	cpe *abapAddonAssemblyKitReserveNextPackagesCommonPipelineEnvironment, maxRuntimeInMinutes time.Duration, pollIntervalsInSeconds time.Duration) error {
-	conn := new(abapbuild.Connector)
-	conn.InitAAKaaS(config.AbapAddonAssemblyKitEndpoint, config.Username, config.Password, client)

-	var addonDescriptor abaputils.AddonDescriptor
-	json.Unmarshal([]byte(config.AddonDescriptor), &addonDescriptor)
+	conn := new(abapbuild.Connector)
+	if err := conn.InitAAKaaS(config.AbapAddonAssemblyKitEndpoint, config.Username, config.Password, client); err != nil {
+		return err
+	}
+
+	addonDescriptor := new(abaputils.AddonDescriptor)
+	if err := addonDescriptor.InitFromJSONstring(config.AddonDescriptor); err != nil {
+		return errors.Wrap(err, "Reading AddonDescriptor failed [Make sure abapAddonAssemblyKit...CheckCVs|CheckPV steps have been run before]")
+	}

 	packagesWithRepos, err := reservePackages(addonDescriptor.Repositories, *conn)
 	if err != nil {
@ -49,20 +53,55 @@ func runAbapAddonAssemblyKitReserveNextPackages(config *abapAddonAssemblyKitRese
 	if err != nil {
 		return err
 	}
-	addonDescriptor.Repositories = copyFieldsToRepositories(packagesWithRepos)
+
+	addonDescriptor.Repositories, err = checkAndCopyFieldsToRepositories(packagesWithRepos)
+	if err != nil {
+		return err
+	}
+
 	log.Entry().Info("Writing package names, types, status, namespace and predecessorCommitID to CommonPipelineEnvironment")
-	backToCPE, _ := json.Marshal(addonDescriptor)
-	cpe.abap.addonDescriptor = string(backToCPE)
+	cpe.abap.addonDescriptor = addonDescriptor.AsJSONstring()
 	return nil
 }

-func copyFieldsToRepositories(pckgWR []aakaas.PackageWithRepository) []abaputils.Repository {
+func checkAndCopyFieldsToRepositories(pckgWR []aakaas.PackageWithRepository) ([]abaputils.Repository, error) {
 	var repos []abaputils.Repository
+
+	log.Entry().Infof("%-30v | %-20v | %-6v | %-40v | %-40v", "Software Component", "Package", "Status", "CommitID (from addon.yml)", "PredecessorCommitID (from AAKaaS)")
+
 	for i := range pckgWR {
+
+		log.Entry().Infof("%-30v | %-20v | %-6v | %-40v | %-40v", pckgWR[i].Repo.Name, pckgWR[i].Package.PackageName, pckgWR[i].Package.Status, pckgWR[i].Repo.CommitID, pckgWR[i].Package.PredecessorCommitID)
+
+		if pckgWR[i].Package.Status == aakaas.PackageStatusReleased {
+			//Ensure for Packages with Status R that CommitID of package = the one from addon.yml, beware of short commitID in addon.yml
+			addonYAMLcommitIDLength := len(pckgWR[i].Repo.CommitID)
+			if len(pckgWR[i].Package.CommitID) < addonYAMLcommitIDLength {
+				return repos, errors.New("Provided CommitIDs have wrong length: " + pckgWR[i].Repo.CommitID + "(addon.yml) longer than the one from AAKaaS " + pckgWR[i].Package.CommitID)
+			}
+			packageCommitIDsubsting := pckgWR[i].Package.CommitID[0:addonYAMLcommitIDLength]
+			if pckgWR[i].Repo.CommitID != packageCommitIDsubsting {
+				log.Entry().Error("package " + pckgWR[i].Package.PackageName + " was already build but with commit " + pckgWR[i].Package.CommitID + ", not with " + pckgWR[i].Repo.CommitID)
+				log.Entry().Error("If you want to build a new package make sure to increase the dotted-version-string in addon.yml")
+				log.Entry().Error("If you do NOT want to build a new package enter the commitID " + pckgWR[i].Package.CommitID + " for software component " + pckgWR[i].Repo.Name + " in addon.yml")
+				return repos, errors.New("commit of released package does not match with addon.yml")
+			}
+		} else if pckgWR[i].Package.PredecessorCommitID != "" {
+			//Check for newly reserved packages which are to be build that CommitID from addon.yml != PreviousCommitID [this will result in an error as no delta can be calculated]
+			addonYAMLcommitIDLength := len(pckgWR[i].Repo.CommitID)
+			if len(pckgWR[i].Package.PredecessorCommitID) < addonYAMLcommitIDLength {
+				return repos, errors.New("Provided CommitIDs have wrong length: " + pckgWR[i].Repo.CommitID + "(addon.yml) longer than the one from AAKaaS " + pckgWR[i].Package.CommitID)
+			}
+			packagePredecessorCommitIDsubsting := pckgWR[i].Package.PredecessorCommitID[0:addonYAMLcommitIDLength]
+			if pckgWR[i].Repo.CommitID == packagePredecessorCommitIDsubsting {
+				return repos, errors.New("CommitID of package" + pckgWR[i].Package.PackageName + "is the same as the on of the predecessor package. Make sure to change both the dotted-version-string AND the commitID in addon.yml")
+			}
+		}
+
 		pckgWR[i].Package.CopyFieldsToRepo(&pckgWR[i].Repo)
 		repos = append(repos, pckgWR[i].Repo)
 	}
-	return repos
+	return repos, nil
 }

 func pollReserveNextPackages(pckgWR []aakaas.PackageWithRepository, maxRuntimeInMinutes time.Duration, pollIntervalsInSeconds time.Duration) error {
--- a/cmd/abapAddonAssemblyKitReserveNextPackages_test.go
+++ b/cmd/abapAddonAssemblyKitReserveNextPackages_test.go
@ -14,6 +14,8 @@ import (

 func TestReserveNextPackagesStep(t *testing.T) {
 	var config abapAddonAssemblyKitReserveNextPackagesOptions
+	config.Username = "dummy"
+	config.Password = "dummy"
 	var cpe abapAddonAssemblyKitReserveNextPackagesCommonPipelineEnvironment
 	timeout := time.Duration(5 * time.Second)
 	pollInterval := time.Duration(1 * time.Second)
@ -23,10 +25,12 @@ func TestReserveNextPackagesStep(t *testing.T) {
 				{
 					Name:        "/DRNMSPC/COMP01",
 					VersionYAML: "1.0.0.",
+					CommitID:    "hugo",
 				},
 				{
 					Name:        "/DRNMSPC/COMP02",
 					VersionYAML: "1.0.0.",
+					CommitID:    "something40charslongxxxxxxxxxxxxxxxxxxxx",
 				},
 			},
 		}
@ -98,28 +102,87 @@ func TestInitPackage(t *testing.T) {

 // ********************* Test copyFieldsToRepositories *******************
 func TestCopyFieldsToRepositoriesPackage(t *testing.T) {
-	t.Run("test copyFieldsToRepositories", func(t *testing.T) {
-		pckgWR := []aakaas.PackageWithRepository{
-			{
-				Package: aakaas.Package{
-					ComponentName: "/DRNMSPC/COMP01",
-					VersionYAML:   "1.0.0",
-					PackageName:   "SAPK-001AAINDRNMSPC",
-					Type:          "AOI",
-					Status:        aakaas.PackageStatusPlanned,
-					Namespace:     "/DRNMSPC/",
-				},
-				Repo: abaputils.Repository{
-					Name:        "/DRNMSPC/COMP01",
-					VersionYAML: "1.0.0",
-				},
+	pckgWR := []aakaas.PackageWithRepository{
+		{
+			Package: aakaas.Package{
+				ComponentName: "/DRNMSPC/COMP01",
+				VersionYAML:   "1.0.0",
+				PackageName:   "SAPK-001AAINDRNMSPC",
+				Type:          "AOI",
+				Namespace:     "/DRNMSPC/",
 			},
-		}
-		repos := copyFieldsToRepositories(pckgWR)
+			Repo: abaputils.Repository{
+				Name:        "/DRNMSPC/COMP01",
+				VersionYAML: "1.0.0",
+			},
+		},
+	}
+
+	t.Run("test copyFieldsToRepositories Planned success w/o predecessorcommitID", func(t *testing.T) {
+		pckgWR[0].Package.Status = aakaas.PackageStatusPlanned
+		pckgWR[0].Package.PredecessorCommitID = ""
+		pckgWR[0].Repo.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		pckgWR[0].Package.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		repos, err := checkAndCopyFieldsToRepositories(pckgWR)
 		assert.Equal(t, "SAPK-001AAINDRNMSPC", repos[0].PackageName)
 		assert.Equal(t, "AOI", repos[0].PackageType)
 		assert.Equal(t, string(aakaas.PackageStatusPlanned), repos[0].Status)
 		assert.Equal(t, "/DRNMSPC/", repos[0].Namespace)
+		assert.NoError(t, err)
+	})
+
+	t.Run("test copyFieldsToRepositories Planned success with predecessorcommitID", func(t *testing.T) {
+		pckgWR[0].Package.Status = aakaas.PackageStatusPlanned
+		pckgWR[0].Package.PredecessorCommitID = "something40charslongPREDECESSORyyyyyyyyy"
+		pckgWR[0].Repo.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		pckgWR[0].Package.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		_, err := checkAndCopyFieldsToRepositories(pckgWR)
+		assert.NoError(t, err)
+	})
+
+	t.Run("test copyFieldsToRepositories Planned error with predecessorcommitID same as commitID", func(t *testing.T) {
+		pckgWR[0].Package.Status = aakaas.PackageStatusPlanned
+		pckgWR[0].Package.PredecessorCommitID = pckgWR[0].Repo.CommitID
+		pckgWR[0].Repo.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		pckgWR[0].Package.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		_, err := checkAndCopyFieldsToRepositories(pckgWR)
+		assert.Error(t, err)
+	})
+
+	t.Run("test copyFieldsToRepositories Planned error with too long commitID in addon.yml", func(t *testing.T) {
+		pckgWR[0].Package.Status = aakaas.PackageStatusPlanned
+		pckgWR[0].Package.PredecessorCommitID = "something40charslongPREDECESSORyyyyyyyyy"
+		pckgWR[0].Repo.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxxtoolong"
+		pckgWR[0].Package.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		_, err := checkAndCopyFieldsToRepositories(pckgWR)
+		assert.Error(t, err)
+	})
+
+	t.Run("test copyFieldsToRepositories Released success", func(t *testing.T) {
+		pckgWR[0].Package.Status = aakaas.PackageStatusReleased
+		pckgWR[0].Package.PredecessorCommitID = "" //released packages do not have this attribute
+		pckgWR[0].Repo.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		pckgWR[0].Package.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		_, err := checkAndCopyFieldsToRepositories(pckgWR)
+		assert.NoError(t, err)
+	})
+
+	t.Run("test copyFieldsToRepositories Released error, different commitIDs", func(t *testing.T) {
+		pckgWR[0].Package.Status = aakaas.PackageStatusReleased
+		pckgWR[0].Package.PredecessorCommitID = "" //released packages do not have this attribute
+		pckgWR[0].Repo.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxx"
+		pckgWR[0].Package.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxO"
+		_, err := checkAndCopyFieldsToRepositories(pckgWR)
+		assert.Error(t, err)
+	})
+
+	t.Run("test copyFieldsToRepositories Released error with too long commitID in addon.yml", func(t *testing.T) {
+		pckgWR[0].Package.Status = aakaas.PackageStatusReleased
+		pckgWR[0].Package.PredecessorCommitID = "" //released packages do not have this attribute
+		pckgWR[0].Repo.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxxtoolong"
+		pckgWR[0].Package.CommitID = "something40charslongxxxxxxxxxxxxxxxxxxxO"
+		_, err := checkAndCopyFieldsToRepositories(pckgWR)
+		assert.Error(t, err)
 	})
 }

@ -320,6 +383,7 @@ var responseReserveNextPackagePostReleased = `{
            "PatchLevel": "0000",
            "Predecessor": "",
            "PredecessorCommitId": "",
+			"CommitId": "something40charslongxxxxxxxxxxxxxxxxxxxx",
            "Status": "R"
        }
    }
--- a/documentation/docs/pipelines/abapEnvironment/stages/build.md
+++ b/documentation/docs/pipelines/abapEnvironment/stages/build.md
@ -7,7 +7,6 @@ This stage is responsible for building an ABAP add-on for the SAP BTP, ABAP envi
 The following steps are executed in this stage:

 - [cloudFoundryCreateServiceKey](../../../steps/cloudFoundryCreateServiceKey.md)
- [abapAddonAssemblyKitReserveNextPackages](../../../steps/abapAddonAssemblyKitReserveNextPackages.md)
 - [abapEnvironmentAssemblePackages](../../../steps/abapEnvironmentAssemblePackages.md)
 - [abapAddonAssemblyKitRegisterPackages](../../../steps/abapAddonAssemblyKitRegisterPackages.md)
 - [abapAddonAssemblyKitReleasePackages](../../../steps/abapAddonAssemblyKitReleasePackages.md)
--- a/documentation/docs/pipelines/abapEnvironment/stages/initialChecks.md
+++ b/documentation/docs/pipelines/abapEnvironment/stages/initialChecks.md
@ -8,6 +8,7 @@ The following steps are executed in this stage:

 - [abapAddonAssemblyKitCheckPV](../../../steps/abapAddonAssemblyKitCheckPV.md)
 - [abapAddonAssemblyKitCheckCVs](../../../steps/abapAddonAssemblyKitCheckCVs.md)
+- [abapAddonAssemblyKitReserveNextPackages](../../../steps/abapAddonAssemblyKitReserveNextPackages.md)

 ## Stage Parameters

--- a/pkg/abap/aakaas/package.go
+++ b/pkg/abap/aakaas/package.go
@ -38,10 +38,11 @@ type jsonPackage struct {
 type Package struct {
 	abapbuild.Connector
 	ComponentName       string
-	PackageName         string `json:"Name"`
 	VersionYAML         string
+	PackageName         string        `json:"Name"`
 	Type                string        `json:"Type"`
 	PredecessorCommitID string        `json:"PredecessorCommitId"`
+	CommitID            string        `json:"CommitId"`
 	Status              PackageStatus `json:"Status"`
 	Namespace           string        `json:"Namespace"`
 }
@ -90,6 +91,7 @@ func (p *Package) ReserveNext() error {
 	p.PredecessorCommitID = jPck.DeterminePackage.Package.PredecessorCommitID
 	p.Status = jPck.DeterminePackage.Package.Status
 	p.Namespace = jPck.DeterminePackage.Package.Namespace
+	p.CommitID = jPck.DeterminePackage.Package.CommitID
 	log.Entry().Infof("Reservation of package %s started", p.PackageName)
 	return nil
 }
--- a/vars/abapEnvironmentPipelineStageBuild.groovy
+++ b/vars/abapEnvironmentPipelineStageBuild.groovy
@ -26,7 +26,6 @@ void call(Map parameters = [:]) {

    piperStageWrapper (script: script, stageName: stageName, stashContent: [], stageLocking: false) {
        cloudFoundryCreateServiceKey script: parameters.script
-        abapAddonAssemblyKitReserveNextPackages script: parameters.script
        abapEnvironmentAssemblePackages script: parameters.script
        abapAddonAssemblyKitRegisterPackages script: parameters.script
        abapAddonAssemblyKitReleasePackages script: parameters.script
--- a/vars/abapEnvironmentPipelineStageInitialChecks.groovy
+++ b/vars/abapEnvironmentPipelineStageInitialChecks.groovy
@ -21,6 +21,7 @@ void call(Map parameters = [:]) {
    piperStageWrapper (script: script, stageName: stageName, stashContent: [], stageLocking: false) {
        abapAddonAssemblyKitCheckPV script: parameters.script
        abapAddonAssemblyKitCheckCVs script: parameters.script
+        abapAddonAssemblyKitReserveNextPackages script: parameters.script
    }

 }