From 9c870b2514c3aa1e45e290e86329fb96e3f7ee39 Mon Sep 17 00:00:00 2001 From: Fabian Reh <61311195+CFR0901@users.noreply.github.com> Date: Tue, 27 Oct 2020 13:45:34 +0100 Subject: [PATCH] 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> --- cmd/gitopsUpdateDeployment.go | 12 +-- cmd/gitopsUpdateDeployment_generated.go | 27 +++--- cmd/gitopsUpdateDeployment_test.go | 88 +++++++++---------- cmd/kanikoExecute.go | 23 +++-- pkg/git/git.go | 13 ++- pkg/git/git_test.go | 15 +++- .../metadata/gitopsUpdateDeployment.yaml | 17 ++-- vars/commonPipelineEnvironment.groovy | 13 +++ 8 files changed, 118 insertions(+), 90 deletions(-) diff --git a/cmd/gitopsUpdateDeployment.go b/cmd/gitopsUpdateDeployment.go index 8fe023e8f..42a404075 100644 --- 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") } diff --git a/cmd/gitopsUpdateDeployment_generated.go b/cmd/gitopsUpdateDeployment_generated.go index a1c24aecb..97d5cf371 100644 --- 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"}}, }, }, }, diff --git a/cmd/gitopsUpdateDeployment_test.go b/cmd/gitopsUpdateDeployment_test.go index 9964550d8..7f90865cd 100644 --- 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) diff --git a/cmd/kanikoExecute.go b/cmd/kanikoExecute.go index 898f77a0d..20118734b 100644 --- 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...) } diff --git a/pkg/git/git.go b/pkg/git/git.go index beebab1f3..87e98fce5 100644 --- 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") } diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go index 9e80e86fd..cbee9127c 100644 --- 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 } diff --git a/resources/metadata/gitopsUpdateDeployment.yaml b/resources/metadata/gitopsUpdateDeployment.yaml index db3e30351..d71db78fc 100644 --- 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" diff --git a/vars/commonPipelineEnvironment.groovy b/vars/commonPipelineEnvironment.groovy index 7e4f35272..2bec42995 100644 --- 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() {