fix(api,worker): add default value to tag parameter in gitClone (#3227)

Signed-off-by: Benjamin Coenen <benjamin.coenen@corp.ovh.com>
ovh · Aug 22, 2018 · 34edf19 · 34edf19
1 parent 3a8dad3
commit 34edf19
Show file tree

Hide file tree

Showing 11 changed files with 102 additions and 48 deletions.
diff --git a/engine/api/action/builtin.go b/engine/api/action/builtin.go
@@ -126,7 +126,7 @@ The public key have to be granted on your repository`,
 	gitclone.Parameter(sdk.Parameter{
 		Name:        "tag",
 		Description: "Useful when you want to git clone a specific tag",
-		Value:       "",
+		Value:       sdk.DefaultGitCloneParameterTagValue,
 		Type:        sdk.StringParameter,
 		Advanced:    true,
 	})

diff --git a/engine/sql/122_git_clone_default_tag.sql b/engine/sql/122_git_clone_default_tag.sql
@@ -0,0 +1,5 @@
+-- +migrate Up
+UPDATE action_parameter SET value = '{{.git.tag}}' WHERE name='tag' AND action_id=(select id from action where name = 'GitClone' and type = 'Builtin');
+
+-- +migrate Down
+UPDATE action_parameter SET value = '' WHERE name='tag' AND action_id=(select id from action where name = 'GitClone' and type = 'Builtin');
diff --git a/engine/worker/builtin_gitclone.go b/engine/worker/builtin_gitclone.go
@@ -340,46 +340,46 @@ func extractInfo(w *currentWorker, dir string, params *[]sdk.Parameter, tag, bra
 		sendLog(fmt.Sprintf("cds.semver: %s", cdsSemver))
 	}
 
-	if (branch == "" || branch == "{{.git.branch}}") && tag == "" {
-		if info.Branch != "" {
-			gitBranch := sdk.Variable{
-				Name:  "git.branch",
-				Type:  sdk.StringVariable,
-				Value: info.Branch,
-			}
+	if tag != "" && tag != sdk.DefaultGitCloneParameterTagValue {
+		sendLog(fmt.Sprintf("git.tag: %s", tag))
+	} else {
+		if branch == "" || branch == "{{.git.branch}}" {
+			if info.Branch != "" {
+				gitBranch := sdk.Variable{
+					Name:  "git.branch",
+					Type:  sdk.StringVariable,
+					Value: info.Branch,
+				}
 
-			if _, err := w.addVariableInPipelineBuild(gitBranch, params); err != nil {
-				return fmt.Errorf("Error on addVariableInPipelineBuild (branch): %s", err)
+				if _, err := w.addVariableInPipelineBuild(gitBranch, params); err != nil {
+					return fmt.Errorf("Error on addVariableInPipelineBuild (branch): %s", err)
+				}
+				sendLog(fmt.Sprintf("git.branch: %s", info.Branch))
+			} else {
+				sendLog("git.branch: [empty]")
 			}
-			sendLog(fmt.Sprintf("git.branch: %s", info.Branch))
-		} else {
-			sendLog("git.branch: [empty]")
+		} else if branch != "" && branch != "{{.git.branch}}" {
+			sendLog(fmt.Sprintf("git.branch: %s", branch))
 		}
-	} else if branch != "" && branch != "{{.git.branch}}" {
-		sendLog(fmt.Sprintf("git.branch: %s", branch))
-	}
 
-	if tag != "" {
-		sendLog(fmt.Sprintf("git.tag: %s", tag))
-	}
-
-	if commit == "" || commit == "{{.git.hash}}" {
-		if info.Hash != "" {
-			gitHash := sdk.Variable{
-				Name:  "git.hash",
-				Type:  sdk.StringVariable,
-				Value: info.Hash,
-			}
+		if commit == "" || commit == "{{.git.hash}}" {
+			if info.Hash != "" {
+				gitHash := sdk.Variable{
+					Name:  "git.hash",
+					Type:  sdk.StringVariable,
+					Value: info.Hash,
+				}
 
-			if _, err := w.addVariableInPipelineBuild(gitHash, params); err != nil {
-				return fmt.Errorf("Error on addVariableInPipelineBuild (hash): %s", err)
+				if _, err := w.addVariableInPipelineBuild(gitHash, params); err != nil {
+					return fmt.Errorf("Error on addVariableInPipelineBuild (hash): %s", err)
+				}
+				sendLog(fmt.Sprintf("git.hash: %s", info.Hash))
+			} else {
+				sendLog("git.hash: [empty]")
 			}
-			sendLog(fmt.Sprintf("git.hash: %s", info.Hash))
 		} else {
-			sendLog("git.hash: [empty]")
+			sendLog(fmt.Sprintf("git.hash: %s", commit))
 		}
-	} else {
-		sendLog(fmt.Sprintf("git.hash: %s", commit))
 	}
 
 	if message == "" {

diff --git a/sdk/action.go b/sdk/action.go
@@ -87,6 +87,8 @@ const (
 	ReleaseAction             = "Release"
 	CheckoutApplicationAction = "CheckoutApplication"
 	DeployApplicationAction   = "DeployApplication"
+
+	DefaultGitCloneParameterTagValue = "{{.git.tag}}"
 )
 
 // NewAction instanciate a new Action

diff --git a/sdk/exportentities/action.go b/sdk/exportentities/action.go
@@ -160,7 +160,7 @@ func newSteps(a sdk.Action) []Step {
 					gitCloneArgs["submodules"] = submodules.Value
 				}
 				tag := sdk.ParameterFind(&act.Parameters, "tag")
-				if tag != nil && tag.Value != "" {
+				if tag != nil && tag.Value != "" && tag.Value != sdk.DefaultGitCloneParameterTagValue {
 					gitCloneArgs["tag"] = tag.Value
 				}
 				s["gitClone"] = gitCloneArgs

diff --git a/sdk/vcs/git/git_clone.go b/sdk/vcs/git/git_clone.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"strings"
 	"time"
+
+	"github.com/ovh/cds/sdk"
 )
 
 // CloneOpts is a optional structs for git clone command
@@ -58,11 +60,11 @@ func prepareGitCloneCommands(repo string, path string, opts *CloneOpts) (string,
 			gitcmd.args = append(gitcmd.args, "--depth", fmt.Sprintf("%d", opts.Depth))
 		}
 
-		if opts.Branch != "" || opts.Tag != "" {
-			if opts.Branch != "" {
-				gitcmd.args = append(gitcmd.args, "--branch", opts.Branch)
-			} else {
+		if opts.Branch != "" || (opts.Tag != "" && opts.Tag != sdk.DefaultGitCloneParameterTagValue) {
+			if opts.Tag != "" && opts.Tag != sdk.DefaultGitCloneParameterTagValue {
 				gitcmd.args = append(gitcmd.args, "--branch", opts.Tag)
+			} else {
+				gitcmd.args = append(gitcmd.args, "--branch", opts.Branch)
 			}
 		} else if opts.SingleBranch {
 			gitcmd.args = append(gitcmd.args, "--single-branch")
@@ -82,7 +84,7 @@ func prepareGitCloneCommands(repo string, path string, opts *CloneOpts) (string,
 
 	allCmd = append(allCmd, gitcmd)
 
-	if opts != nil && opts.CheckoutCommit != "" {
+	if opts != nil && opts.CheckoutCommit != "" && opts.Tag == "" {
 		resetCmd := cmd{
 			cmd:  "git",
 			args: []string{"reset", "--hard", opts.CheckoutCommit},

diff --git a/tests/clictl_action.yml b/tests/clictl_action.yml
@@ -44,7 +44,7 @@ testcases:
   - script: {{.cds.build.cdsctl}} action export CDS_TestIT_GitClone > ./tests/fixtures/clictl_action_CDS_TestIT_GitClone.exported
   - script: diff ./tests/fixtures/clictl_action_CDS_TestIT_Disabled.exported ./tests/fixtures/action_disabled.yml
   - script: diff ./tests/fixtures/clictl_action_CDS_TestIT_Enabled.exported ./tests/fixtures/action_enabled.yml
-  - script: diff ./tests/fixtures/clictl_action_CDS_TestIT_GitClone.exported ./tests/fixtures/action_git_clone.yml
+  - script: diff ./tests/fixtures/clictl_action_CDS_TestIT_GitClone.exported ./tests/fixtures/action_git_clone_default.yml
 - name: action list
   steps:
-  - script: {{.cds.build.cdsctl}} action list
+  - script: {{.cds.build.cdsctl}} action list
diff --git a/tests/fixtures/action_git_clone.yml b/tests/fixtures/action_git_clone.yml
@@ -13,15 +13,17 @@ steps:
     directory: '{{.cds.workspace}}'
     submodules: "false"
     url: '{{.git.url}}'
+    tag: '{{.git.tag}}'
 - gitClone:
     branch: '{{.git.branch}}'
     commit: '{{.git.hash}}'
     directory: '{{.cds.workspace}}'
     url: '{{.git.url}}'
+    tag: '{{.git.tag}}'
 - gitClone:
     branch: '{{.git.branch}}'
     commit: '{{.git.hash}}'
     depth: "10"
     directory: '{{.cds.workspace}}'
     url: '{{.git.url}}'
-
+    tag: '{{.git.tag}}'
diff --git a/tests/fixtures/action_git_clone_default.yml b/tests/fixtures/action_git_clone_default.yml
@@ -0,0 +1,27 @@
+version: v1.0
+name: CDS_TestIT_GitClone
+parameters:
+  name:
+    type: string
+requirements:
+- binary: git
+steps:
+- gitClone:
+    branch: '{{.git.branch}}'
+    commit: '{{.git.hash}}'
+    depth: "10"
+    directory: '{{.cds.workspace}}'
+    submodules: "false"
+    url: '{{.git.url}}'
+- gitClone:
+    branch: '{{.git.branch}}'
+    commit: '{{.git.hash}}'
+    directory: '{{.cds.workspace}}'
+    url: '{{.git.url}}'
+- gitClone:
+    branch: '{{.git.branch}}'
+    commit: '{{.git.hash}}'
+    depth: "10"
+    directory: '{{.cds.workspace}}'
+    url: '{{.git.url}}'
+
diff --git a/ui/src/app/shared/action/action.component.ts b/ui/src/app/shared/action/action.component.ts
@@ -74,8 +74,8 @@ export class ActionComponent implements OnDestroy, OnInit {
 
     keyEvent(event: KeyboardEvent) {
       if (event.key === 's' && (event.ctrlKey || event.metaKey)) {
-          this.sendActionEvent('update');
           event.preventDefault();
+          setTimeout(() => this.sendActionEvent('update'));
       }
     }
 

diff --git a/ui/src/app/shared/action/step/step.html b/ui/src/app/shared/action/step/step.html
@@ -84,8 +84,16 @@
                 </label>
             </div>
             <div class="fourteen wide column">
-                <app-parameter-value [edit]="edit" [type]="p.type" [keys]="keys" [(value)]="p.value" [editList]="false" [suggest]="suggest"
-                                    (valueUpdating)="action.hasChanged = true" [ref]="originalParam.get(p.name)"></app-parameter-value>
+                <app-parameter-value
+                  [edit]="edit"
+                  [type]="p.type"
+                  [keys]="keys"
+                  [(value)]="p.value"
+                  [editList]="false"
+                  [suggest]="suggest"
+                  (valueUpdating)="action.hasChanged = true"
+                  [ref]="originalParam.get(p.name)">
+                </app-parameter-value>
             </div>
         </ng-container>
     </div>
@@ -94,7 +102,7 @@
         {{'parameters_advanced' | translate}}
         <i class="caret down icon" *ngIf="collapsed_advanced"></i>
         <i class="caret right icon" *ngIf="!collapsed_advanced"></i>
-    </div>    
+    </div>
 
     <ng-container *ngIf="withAdvanced && collapsed_advanced">
         <div class="row" *ngFor="let p of step.parameters">
@@ -108,8 +116,16 @@
                     </label>
                 </div>
                 <div class="fourteen wide column">
-                    <app-parameter-value [edit]="edit" [type]="p.type" [keys]="keys" [(value)]="p.value" [editList]="false" [suggest]="suggest"
-                                        (valueUpdating)="action.hasChanged = true" [ref]="originalParam.get(p.name)"></app-parameter-value>
+                    <app-parameter-value
+                      [edit]="edit"
+                      [type]="p.type"
+                      [keys]="keys"
+                      [(value)]="p.value"
+                      [editList]="false"
+                      [suggest]="suggest"
+                      (valueUpdating)="action.hasChanged = true"
+                      [ref]="originalParam.get(p.name)">
+                    </app-parameter-value>
                 </div>
             </ng-container>
         </div>