Gitops update deployment fixes (#2234)

* makes containerImage not mandatory * Adds kubectl container * Adds log statement to debug * adds general to container image * removes GENERAL again Removes condition from Kubectl container * removes workDir * marks logs as debug * adds workingdir again * Adds author to commits * Adds commit time now * remove deprecated and reorder * adds deprecated again to containerRegistryUrl Adds GENERAL scope to containerImage * updates generated file * Renames containerImageNameTag * adds else case * adds debug log * code cleanup * adds debug log * revert * adds debug logs * revert * makes root path not hidden * revert * Read container properties * Removes debug message * Removes debug message * Removes general scope again * Fixes unit test Co-authored-by: Oliver Nocon <33484802+OliverNocon@users.noreply.github.com>
2025-10-30 23:57:50 +02:00 · 2020-10-27 13:45:34 +01:00
parent 9d737575aa
commit 9c870b2514
8 changed files with 118 additions and 90 deletions
--- a/cmd/gitopsUpdateDeployment.go
+++ b/cmd/gitopsUpdateDeployment.go
@@ -17,7 +17,7 @@ import (
 )

 type iGitopsUpdateDeploymentGitUtils interface {
-	CommitSingleFile(filePath, commitMessage string) (plumbing.Hash, error)
+	CommitSingleFile(filePath, commitMessage, author string) (plumbing.Hash, error)
 	PushChangesToRepository(username, password string) error
 	PlainClone(username, password, serverURL, directory string) error
 	ChangeBranch(branchName string) error
@@ -40,8 +40,8 @@ type gitopsUpdateDeploymentGitUtils struct {
 	repository *git.Repository
 }

-func (g *gitopsUpdateDeploymentGitUtils) CommitSingleFile(filePath, commitMessage string) (plumbing.Hash, error) {
-	return gitUtil.CommitSingleFile(filePath, commitMessage, g.worktree)
+func (g *gitopsUpdateDeploymentGitUtils) CommitSingleFile(filePath, commitMessage, author string) (plumbing.Hash, error) {
+	return gitUtil.CommitSingleFile(filePath, commitMessage, author, g.worktree)
 }

 func (g *gitopsUpdateDeploymentGitUtils) PushChangesToRepository(username, password string) error {
@@ -147,7 +147,7 @@ func runKubeCtlCommand(command gitopsUpdateDeploymentExecRunner, patchString str
 func buildRegistryPlusImage(config *gitopsUpdateDeploymentOptions) (string, error) {
 	registryURL := config.ContainerRegistryURL
 	if registryURL == "" {
-		return config.ContainerImage, nil
+		return config.ContainerImageNameTag, nil
 	}

 	url, err := docker.ContainerRegistryFromURL(registryURL)
@@ -157,11 +157,11 @@ func buildRegistryPlusImage(config *gitopsUpdateDeploymentOptions) (string, erro
 	if url != "" {
 		url = url + "/"
 	}
-	return url + config.ContainerImage, nil
+	return url + config.ContainerImageNameTag, nil
 }

 func commitAndPushChanges(config *gitopsUpdateDeploymentOptions, gitUtils iGitopsUpdateDeploymentGitUtils) (plumbing.Hash, error) {
-	commit, err := gitUtils.CommitSingleFile(config.FilePath, config.CommitMessage)
+	commit, err := gitUtils.CommitSingleFile(config.FilePath, config.CommitMessage, config.Username)
 	if err != nil {
 		return [20]byte{}, errors.Wrap(err, "committing changes failed")
 	}
--- a/cmd/gitopsUpdateDeployment_generated.go
+++ b/cmd/gitopsUpdateDeployment_generated.go
@@ -14,15 +14,15 @@ import (
 )

 type gitopsUpdateDeploymentOptions struct {
-	BranchName           string `json:"branchName,omitempty"`
-	CommitMessage        string `json:"commitMessage,omitempty"`
-	ServerURL            string `json:"serverUrl,omitempty"`
-	Username             string `json:"username,omitempty"`
-	Password             string `json:"password,omitempty"`
-	FilePath             string `json:"filePath,omitempty"`
-	ContainerName        string `json:"containerName,omitempty"`
-	ContainerRegistryURL string `json:"containerRegistryUrl,omitempty"`
-	ContainerImage       string `json:"containerImage,omitempty"`
+	BranchName            string `json:"branchName,omitempty"`
+	CommitMessage         string `json:"commitMessage,omitempty"`
+	ServerURL             string `json:"serverUrl,omitempty"`
+	Username              string `json:"username,omitempty"`
+	Password              string `json:"password,omitempty"`
+	FilePath              string `json:"filePath,omitempty"`
+	ContainerName         string `json:"containerName,omitempty"`
+	ContainerRegistryURL  string `json:"containerRegistryUrl,omitempty"`
+	ContainerImageNameTag string `json:"containerImageNameTag,omitempty"`
 }

 // GitopsUpdateDeploymentCommand Updates Kubernetes Deployment Manifest in an Infrastructure Git Repository
@@ -96,7 +96,7 @@ func addGitopsUpdateDeploymentFlags(cmd *cobra.Command, stepConfig *gitopsUpdate
 	cmd.Flags().StringVar(&stepConfig.FilePath, "filePath", os.Getenv("PIPER_filePath"), "Relative path in the git repository to the deployment descriptor file that shall be updated")
 	cmd.Flags().StringVar(&stepConfig.ContainerName, "containerName", os.Getenv("PIPER_containerName"), "The name of the container to update")
 	cmd.Flags().StringVar(&stepConfig.ContainerRegistryURL, "containerRegistryUrl", os.Getenv("PIPER_containerRegistryUrl"), "http(s) url of the Container registry where the image is located")
-	cmd.Flags().StringVar(&stepConfig.ContainerImage, "containerImage", os.Getenv("PIPER_containerImage"), "Container image name with version tag to annotate in the deployment configuration.")
+	cmd.Flags().StringVar(&stepConfig.ContainerImageNameTag, "containerImageNameTag", os.Getenv("PIPER_containerImageNameTag"), "Container image name with version tag to annotate in the deployment configuration.")

 	cmd.MarkFlagRequired("commitMessage")
 	cmd.MarkFlagRequired("serverUrl")
@@ -104,7 +104,6 @@ func addGitopsUpdateDeploymentFlags(cmd *cobra.Command, stepConfig *gitopsUpdate
 	cmd.MarkFlagRequired("password")
 	cmd.MarkFlagRequired("filePath")
 	cmd.MarkFlagRequired("containerName")
-	cmd.MarkFlagRequired("containerImage")
 }

 // retrieve step metadata
@@ -199,7 +198,7 @@ func gitopsUpdateDeploymentMetadata() config.StepData {
 						Aliases:   []config.Alias{{Name: "dockerRegistryUrl"}},
 					},
 					{
-						Name: "containerImage",
+						Name: "containerImageNameTag",
 						ResourceRef: []config.ResourceReference{
 							{
 								Name:  "commonPipelineEnvironment",
@@ -208,8 +207,8 @@ func gitopsUpdateDeploymentMetadata() config.StepData {
 						},
 						Scope:     []string{"PARAMETERS", "STAGES", "STEPS"},
 						Type:      "string",
-						Mandatory: true,
-						Aliases:   []config.Alias{{Name: "image"}, {Name: "containerImageNameTag"}},
+						Mandatory: false,
+						Aliases:   []config.Alias{{Name: "image"}, {Name: "containerImage"}},
 					},
 				},
 			},
--- a/cmd/gitopsUpdateDeployment_test.go
+++ b/cmd/gitopsUpdateDeployment_test.go
@@ -17,8 +17,8 @@ func TestBuildRegistryPlusImage(t *testing.T) {
 	t.Parallel()
 	t.Run("build full image", func(t *testing.T) {
 		registryImage, err := buildRegistryPlusImage(&gitopsUpdateDeploymentOptions{
-			ContainerRegistryURL: "https://myregistry.com/registry/containers",
-			ContainerImage:       "myFancyContainer:1337",
+			ContainerRegistryURL:  "https://myregistry.com/registry/containers",
+			ContainerImageNameTag: "myFancyContainer:1337",
 		})
 		assert.NoError(t, err)
 		assert.Equal(t, "myregistry.com/myFancyContainer:1337", registryImage)
@@ -26,16 +26,16 @@ func TestBuildRegistryPlusImage(t *testing.T) {

 	t.Run("without registry", func(t *testing.T) {
 		registryImage, err := buildRegistryPlusImage(&gitopsUpdateDeploymentOptions{
-			ContainerRegistryURL: "",
-			ContainerImage:       "myFancyContainer:1337",
+			ContainerRegistryURL:  "",
+			ContainerImageNameTag: "myFancyContainer:1337",
 		})
 		assert.NoError(t, err)
 		assert.Equal(t, "myFancyContainer:1337", registryImage)
 	})
 	t.Run("without faulty URL", func(t *testing.T) {
 		_, err := buildRegistryPlusImage(&gitopsUpdateDeploymentOptions{
-			ContainerRegistryURL: "//myregistry.com/registry/containers",
-			ContainerImage:       "myFancyContainer:1337",
+			ContainerRegistryURL:  "//myregistry.com/registry/containers",
+			ContainerImageNameTag: "myFancyContainer:1337",
 		})
 		assert.Error(t, err)
 		assert.EqualError(t, err, "registry URL could not be extracted: invalid registry url")
@@ -46,15 +46,15 @@ func TestRunGitopsUpdateDeployment(t *testing.T) {
 	t.Parallel()
 	t.Run("successful run", func(t *testing.T) {
 		var configuration = &gitopsUpdateDeploymentOptions{
-			BranchName:           "main",
-			CommitMessage:        "This is the commit message",
-			ServerURL:            "https://github.com",
-			Username:             "admin3",
-			Password:             "validAccessToken",
-			FilePath:             "dir1/dir2/depl.yaml",
-			ContainerName:        "myContainer",
-			ContainerRegistryURL: "https://myregistry.com/registry/containers",
-			ContainerImage:       "myFancyContainer:1337",
+			BranchName:            "main",
+			CommitMessage:         "This is the commit message",
+			ServerURL:             "https://github.com",
+			Username:              "admin3",
+			Password:              "validAccessToken",
+			FilePath:              "dir1/dir2/depl.yaml",
+			ContainerName:         "myContainer",
+			ContainerRegistryURL:  "https://myregistry.com/registry/containers",
+			ContainerImageNameTag: "myFancyContainer:1337",
 		}

 		gitUtilsMock := &validGitUtilsMock{}
@@ -76,15 +76,15 @@ func TestRunGitopsUpdateDeployment(t *testing.T) {

 	t.Run("invalid URL", func(t *testing.T) {
 		var configuration = &gitopsUpdateDeploymentOptions{
-			BranchName:           "main",
-			CommitMessage:        "This is the commit message",
-			ServerURL:            "https://github.com",
-			Username:             "admin3",
-			Password:             "validAccessToken",
-			FilePath:             "dir1/dir2/depl.yaml",
-			ContainerName:        "myContainer",
-			ContainerRegistryURL: "//myregistry.com/registry/containers",
-			ContainerImage:       "myFancyContainer:1337",
+			BranchName:            "main",
+			CommitMessage:         "This is the commit message",
+			ServerURL:             "https://github.com",
+			Username:              "admin3",
+			Password:              "validAccessToken",
+			FilePath:              "dir1/dir2/depl.yaml",
+			ContainerName:         "myContainer",
+			ContainerRegistryURL:  "//myregistry.com/registry/containers",
+			ContainerImageNameTag: "myFancyContainer:1337",
 		}

 		gitUtilsMock := &validGitUtilsMock{}
@@ -95,15 +95,15 @@ func TestRunGitopsUpdateDeployment(t *testing.T) {

 	t.Run("error on plane clone", func(t *testing.T) {
 		var configuration = &gitopsUpdateDeploymentOptions{
-			BranchName:           "main",
-			CommitMessage:        "This is the commit message",
-			ServerURL:            "https://github.com",
-			Username:             "admin3",
-			Password:             "validAccessToken",
-			FilePath:             "dir1/dir2/depl.yaml",
-			ContainerName:        "myContainer",
-			ContainerRegistryURL: "https://myregistry.com/registry/containers",
-			ContainerImage:       "myFancyContainer:1337",
+			BranchName:            "main",
+			CommitMessage:         "This is the commit message",
+			ServerURL:             "https://github.com",
+			Username:              "admin3",
+			Password:              "validAccessToken",
+			FilePath:              "dir1/dir2/depl.yaml",
+			ContainerName:         "myContainer",
+			ContainerRegistryURL:  "https://myregistry.com/registry/containers",
+			ContainerImageNameTag: "myFancyContainer:1337",
 		}

 		err := runGitopsUpdateDeployment(configuration, nil, &gitUtilsMockErrorClone{}, piperutils.Files{})
@@ -112,15 +112,15 @@ func TestRunGitopsUpdateDeployment(t *testing.T) {

 	t.Run("error on temp dir creation", func(t *testing.T) {
 		var configuration = &gitopsUpdateDeploymentOptions{
-			BranchName:           "main",
-			CommitMessage:        "This is the commit message",
-			ServerURL:            "https://github.com",
-			Username:             "admin3",
-			Password:             "validAccessToken",
-			FilePath:             "dir1/dir2/depl.yaml",
-			ContainerName:        "myContainer",
-			ContainerRegistryURL: "https://myregistry.com/registry/containers",
-			ContainerImage:       "myFancyContainer:1337",
+			BranchName:            "main",
+			CommitMessage:         "This is the commit message",
+			ServerURL:             "https://github.com",
+			Username:              "admin3",
+			Password:              "validAccessToken",
+			FilePath:              "dir1/dir2/depl.yaml",
+			ContainerName:         "myContainer",
+			ContainerRegistryURL:  "https://myregistry.com/registry/containers",
+			ContainerImageNameTag: "myFancyContainer:1337",
 		}

 		err := runGitopsUpdateDeployment(configuration, nil, &gitopsUpdateDeploymentGitUtils{}, filesMockErrorTempDirCreation{})
@@ -165,7 +165,7 @@ func (filesMockErrorTempDirCreation) RemoveAll(string) error {

 type gitUtilsMockErrorClone struct{}

-func (gitUtilsMockErrorClone) CommitSingleFile(string, string) (plumbing.Hash, error) {
+func (gitUtilsMockErrorClone) CommitSingleFile(string, string, string) (plumbing.Hash, error) {
 	panic("implement me")
 }

@@ -199,7 +199,7 @@ func (v *validGitUtilsMock) ChangeBranch(branchName string) error {
 	return nil
 }

-func (v *validGitUtilsMock) CommitSingleFile(string, string) (plumbing.Hash, error) {
+func (v *validGitUtilsMock) CommitSingleFile(string, string, string) (plumbing.Hash, error) {
 	matches, _ := piperutils.Files{}.Glob("*/dir1/dir2/depl.yaml")
 	fileRead, _ := piperutils.Files{}.FileRead(matches[0])
 	v.savedFile = string(fileRead)
--- a/cmd/kanikoExecute.go
+++ b/cmd/kanikoExecute.go
@@ -64,17 +64,6 @@ func runKanikoExecute(config *kanikoExecuteOptions, telemetryData *telemetry.Cus

 	if !piperutils.ContainsString(config.BuildOptions, "--destination") {
 		dest := []string{"--no-push"}
-		if len(config.ContainerImage) > 0 {
-			containerRegistry, err := docker.ContainerRegistryFromImage(config.ContainerImage)
-			if err != nil {
-				return errors.Wrapf(err, "invalid registry part in image %v", config.ContainerImage)
-			}
-			// errors are already caught with previous call to docker.ContainerRegistryFromImage
-			containerImageNameTag, _ := docker.ContainerImageNameTagFromImage(config.ContainerImage)
-			dest = []string{"--destination", config.ContainerImage}
-			commonPipelineEnvironment.container.registryURL = fmt.Sprintf("https://%v", containerRegistry)
-			commonPipelineEnvironment.container.imageNameTag = containerImageNameTag
-		}
 		if len(config.ContainerRegistryURL) > 0 && len(config.ContainerImageName) > 0 && len(config.ContainerImageTag) > 0 {
 			containerRegistry, err := docker.ContainerRegistryFromURL(config.ContainerRegistryURL)
 			if err != nil {
@@ -84,6 +73,16 @@ func runKanikoExecute(config *kanikoExecuteOptions, telemetryData *telemetry.Cus
 			dest = []string{"--destination", fmt.Sprintf("%v/%v", containerRegistry, containerImageTag)}
 			commonPipelineEnvironment.container.registryURL = config.ContainerRegistryURL
 			commonPipelineEnvironment.container.imageNameTag = containerImageTag
+		} else if len(config.ContainerImage) > 0 {
+			containerRegistry, err := docker.ContainerRegistryFromImage(config.ContainerImage)
+			if err != nil {
+				return errors.Wrapf(err, "invalid registry part in image %v", config.ContainerImage)
+			}
+			// errors are already caught with previous call to docker.ContainerRegistryFromImage
+			containerImageNameTag, _ := docker.ContainerImageNameTagFromImage(config.ContainerImage)
+			dest = []string{"--destination", config.ContainerImage}
+			commonPipelineEnvironment.container.registryURL = fmt.Sprintf("https://%v", containerRegistry)
+			commonPipelineEnvironment.container.imageNameTag = containerImageNameTag
 		}
 		config.BuildOptions = append(config.BuildOptions, dest...)
 	}
@@ -130,7 +129,7 @@ func certificateUpdate(certLinks []string, httpClient piperhttp.Sender, fileUtil
 		if err != nil {
 			return errors.Wrap(err, "error reading response")
 		}
-		response.Body.Close()
+		_ = response.Body.Close()
 		content = append(content, []byte("\n")...)
 		caCerts = append(caCerts, content...)
 	}
--- a/pkg/git/git.go
+++ b/pkg/git/git.go
@@ -3,8 +3,10 @@ package git
 import (
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing"
+	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/go-git/go-git/v5/plumbing/transport/http"
 	"github.com/pkg/errors"
+	"time"
 )

 // utilsWorkTree interface abstraction of git.Worktree to enable tests
@@ -27,17 +29,20 @@ type utilsGit interface {

 // CommitSingleFile Commits the file located in the relative file path with the commitMessage to the given worktree.
 // In case of errors, the error is returned. In the successful case the commit is provided.
-func CommitSingleFile(filePath, commitMessage string, worktree *git.Worktree) (plumbing.Hash, error) {
-	return commitSingleFile(filePath, commitMessage, worktree)
+func CommitSingleFile(filePath, commitMessage, author string, worktree *git.Worktree) (plumbing.Hash, error) {
+	return commitSingleFile(filePath, commitMessage, author, worktree)
 }

-func commitSingleFile(filePath, commitMessage string, worktree utilsWorkTree) (plumbing.Hash, error) {
+func commitSingleFile(filePath, commitMessage, author string, worktree utilsWorkTree) (plumbing.Hash, error) {
 	_, err := worktree.Add(filePath)
 	if err != nil {
 		return [20]byte{}, errors.Wrap(err, "failed to add file to git")
 	}

-	commit, err := worktree.Commit(commitMessage, &git.CommitOptions{})
+	commit, err := worktree.Commit(commitMessage, &git.CommitOptions{
+		All:    true,
+		Author: &object.Signature{Name: author, When: time.Now()},
+	})
 	if err != nil {
 		return [20]byte{}, errors.Wrap(err, "failed to commit file")
 	}
--- a/pkg/git/git_test.go
+++ b/pkg/git/git_test.go
@@ -11,13 +11,16 @@ import (
 func TestCommit(t *testing.T) {
 	t.Parallel()
 	t.Run("successful run", func(t *testing.T) {
-		hash, err := commitSingleFile(".", "message", &WorktreeMock{})
+		worktreeMock := WorktreeMock{}
+		hash, err := commitSingleFile(".", "message", "user", &worktreeMock)
 		assert.NoError(t, err)
 		assert.Equal(t, plumbing.Hash([20]byte{4, 5, 6}), hash)
+		assert.Equal(t, "user", worktreeMock.author)
+		assert.True(t, worktreeMock.commitAll)
 	})

 	t.Run("error adding file", func(t *testing.T) {
-		_, err := commitSingleFile(".", "message", WorktreeMockFailing{
+		_, err := commitSingleFile(".", "message", "user", WorktreeMockFailing{
 			failingAdd: true,
 		})
 		assert.Error(t, err)
@@ -25,7 +28,7 @@ func TestCommit(t *testing.T) {
 	})

 	t.Run("error committing file", func(t *testing.T) {
-		_, err := commitSingleFile(".", "message", WorktreeMockFailing{
+		_, err := commitSingleFile(".", "message", "user", WorktreeMockFailing{
 			failingCommit: true,
 		})
 		assert.Error(t, err)
@@ -159,13 +162,17 @@ type WorktreeMock struct {
 	expectedBranchName string
 	checkedOutBranch   string
 	create             bool
+	author             string
+	commitAll          bool
 }

 func (WorktreeMock) Add(string) (plumbing.Hash, error) {
 	return [20]byte{1, 2, 3}, nil
 }

-func (WorktreeMock) Commit(string, *git.CommitOptions) (plumbing.Hash, error) {
+func (w *WorktreeMock) Commit(_ string, options *git.CommitOptions) (plumbing.Hash, error) {
+	w.author = options.Author.Name
+	w.commitAll = options.All
 	return [20]byte{4, 5, 6}, nil
 }

--- a/resources/metadata/gitopsUpdateDeployment.yaml
+++ b/resources/metadata/gitopsUpdateDeployment.yaml
@@ -98,18 +98,23 @@ spec:
        resourceRef:
          - name: commonPipelineEnvironment
            param: container/registryUrl
-      - name: containerImage
+      - name: containerImageNameTag
        aliases:
          - name: image
            deprecated: true
-          - name: containerImageNameTag
+          - name: containerImage
        type: string
        description: Container image name with version tag to annotate in the deployment configuration.
-        resourceRef:
-          - name: commonPipelineEnvironment
-            param: container/imageNameTag
-        mandatory: true
        scope:
          - PARAMETERS
          - STAGES
          - STEPS
+        resourceRef:
+          - name: commonPipelineEnvironment
+            param: container/imageNameTag
+  containers:
+    - image: dtzar/helm-kubectl:2.12.1
+      workingDir: /config
+      options:
+        - name: -u
+          value: "0"
--- a/vars/commonPipelineEnvironment.groovy
+++ b/vars/commonPipelineEnvironment.groovy
@@ -241,6 +241,19 @@ class commonPipelineEnvironment implements Serializable {
                valueMap[param] = fileContent
            }
        })
+
+        def containerValues = script.findFiles(glob: '.pipeline/commonPipelineEnvironment/container/*')
+        containerValues.each({f ->
+            def fileContent = script.readFile(f.getPath())
+            def fileName = f.getName()
+            def param = fileName.split('/')[fileName.split('\\/').size()-1]
+            if (param.endsWith(".json")){
+                param = param.replace(".json","")
+                containerProperties[param] = script.readJSON(test: fileContent)
+            }else{
+                containerProperties[param] = fileContent
+            }
+        })
    }

    List getCustomDefaults() {