Enable ssh configration for pipedv1 by ffjlabo · Pull Request #6201 · pipe-cd/pipecd

ffjlabo · 2025-09-03T03:38:53Z

What this PR does:

as title
To realize this, we decided to define a type alias for configv1.PipedGit in pkg/config.

Currently, it's ok because configv1.PipedGit and config.PipedGit are the same definition.

// in pkg/config
type PipedGit = configv1.PipedGit

⚠️ We need to consider the backward capability for v0 when fixing configv1.PipedGit.
We decided to do it to fix the config easily in the future, and we will support only pipedv1.

Why we need it:

The current pipedv1 can't clone git repo with ssh.

Which issue(s) this PR fixes:

Part of #5259

Does this PR introduce a user-facing change?:

How are users affected by this change:
Is this breaking change:
How to migrate (if breaking change):

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

codecov · 2025-09-03T03:40:59Z

Codecov Report

❌ Patch coverage is 9.09091% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 28.78%. Comparing base (3415269) to head (05ee84e).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/app/pipedv1/cmd/piped/piped.go	0.00%	9 Missing ⚠️
pkg/git/ssh_config.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6201      +/-   ##
==========================================
- Coverage   28.80%   28.78%   -0.03%     
==========================================
  Files         559      559              
  Lines       59568    59520      -48     
==========================================
- Hits        17160    17132      -28     
+ Misses      41094    41074      -20     
  Partials     1314     1314

Flag	Coverage Δ
.	`23.25% <9.09%> (-0.04%)`	⬇️
.-pkg-app-pipedv1-plugin-analysis	`32.64% <ø> (ø)`
.-pkg-app-pipedv1-plugin-kubernetes	`60.24% <ø> (ø)`
.-pkg-app-pipedv1-plugin-kubernetes_multicluster	`67.48% <ø> (ø)`
.-pkg-app-pipedv1-plugin-scriptrun	`54.83% <ø> (ø)`
.-pkg-app-pipedv1-plugin-terraform	`39.49% <ø> (ø)`
.-pkg-app-pipedv1-plugin-wait	`35.51% <ø> (ø)`
.-pkg-app-pipedv1-plugin-waitapproval	`55.73% <ø> (ø)`
.-pkg-plugin-sdk	`50.37% <ø> (ø)`
.-tool-actions-gh-release	`19.23% <ø> (ø)`
.-tool-actions-plan-preview	`25.51% <ø> (ø)`
.-tool-codegen-protoc-gen-auth	`0.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

khanhtc1202 · 2025-09-03T05:05:09Z

+	if cfg.Git.ShouldConfigureSSHConfig() {
+		if _, err := git.AddSSHConfig(cfg.Git); err != nil {
+			input.Logger.Error("failed to configure ssh-config", zap.Error(err))
+			return err
+		}
+		input.Logger.Info("successfully configured ssh-config")
+	}


Should update to remove ssh key file as we do for pipedv0
ref:

pipecd/pkg/app/piped/cmd/piped/piped.go

Lines 180 to 189 in 3415269

// Configure SSH config if needed.

if cfg.Git.ShouldConfigureSSHConfig() {

tempFile, err := git.AddSSHConfig(cfg.Git)

if err != nil {

input.Logger.Error("failed to configure ssh-config", zap.Error(err))

return err

}

defer os.Remove(tempFile)

input.Logger.Info("successfully configured ssh-config")

}

Thank you, will fix it.
Note: It is supported in the PR #5769

fixed on 01304fe

khanhtc1202 · 2025-09-03T05:07:10Z

 	"text/template"

-	"github.com/pipe-cd/pipecd/pkg/config"
+	configv1 "github.com/pipe-cd/pipecd/pkg/configv1"


Suggested change

configv1 "github.com/pipe-cd/pipecd/pkg/configv1"

"github.com/pipe-cd/pipecd/pkg/configv1"

Or how about making this like we did in pipedv1 entry
ref: https://github.com/pipe-cd/pipecd/blob/master/pkg/app/pipedv1/cmd/piped/piped.go#L72

@khanhtc1202
This package is for both pipedv0 and pipedv1, so it would be nice to distinguish config and configv1 to avoid misinterpreting them.
So I think it is good as is.
WDYT?

Got your point 🙆‍♂️

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

khanhtc1202 · 2025-09-03T05:56:52Z

@@ -168,13 +168,15 @@ func (p *piped) run(ctx context.Context, input cli.Input) (runErr error) {
 	registry := registerMetrics(cfg.PipedID, cfg.ProjectID, p.launcherVersion)

 	// // Configure SSH config if needed.


Suggested change

// // Configure SSH config if needed.

// Configure SSH config if needed.

nits

fixed 05ee84e

khanhtc1202

LGTM, letf a nits 🙆‍♂️

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

khanhtc1202

LGTM, thanks 🙆‍♂️

Warashi

👍🏻

ffjlabo added 2 commits September 3, 2025 11:45

Define type alias configv1.PipedGit for config.PipedGit

6eab2b0

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

Define type alias config.PipedGit for configv1.PipedGit

3b13162

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

github-actions Bot added area/go area/pipedv1 labels Sep 3, 2025

ffjlabo changed the title ~~[WIP] Enable ssh configration for pipedv1~~ Enable ssh configration for pipedv1 Sep 3, 2025

ffjlabo marked this pull request as ready for review September 3, 2025 04:59

ffjlabo requested a review from a team as a code owner September 3, 2025 04:59

ffjlabo requested review from Warashi, khanhtc1202 and t-kikuc September 3, 2025 04:59

ffjlabo enabled auto-merge (squash) September 3, 2025 05:03

khanhtc1202 reviewed Sep 3, 2025

View reviewed changes

Remove ssh key file after pipedv1 finished

01304fe

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

khanhtc1202 reviewed Sep 3, 2025

View reviewed changes

khanhtc1202 previously approved these changes Sep 3, 2025

View reviewed changes

Fix nit

05ee84e

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

ffjlabo dismissed khanhtc1202’s stale review via 05ee84e September 3, 2025 06:42

ffjlabo requested a review from khanhtc1202 September 3, 2025 06:43

khanhtc1202 approved these changes Sep 3, 2025

View reviewed changes

Warashi approved these changes Sep 3, 2025

View reviewed changes

ffjlabo merged commit 0e9c3ff into master Sep 3, 2025
44 of 45 checks passed

ffjlabo deleted the uncomment-git-ssh-setting-for-pipedv1 branch September 3, 2025 07:01

github-actions Bot mentioned this pull request Sep 16, 2025

Update RELEASE and docs for release v0.54.0 #6228

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable ssh configration for pipedv1#6201

Enable ssh configration for pipedv1#6201
ffjlabo merged 4 commits into
masterfrom
uncomment-git-ssh-setting-for-pipedv1

ffjlabo commented Sep 3, 2025 •

edited

Loading

Uh oh!

codecov Bot commented Sep 3, 2025 •

edited

Loading

Uh oh!

khanhtc1202 Sep 3, 2025

Uh oh!

ffjlabo Sep 3, 2025

Uh oh!

ffjlabo Sep 3, 2025

Uh oh!

khanhtc1202 Sep 3, 2025 •

edited

Loading

Uh oh!

ffjlabo Sep 3, 2025

Uh oh!

khanhtc1202 Sep 3, 2025

Uh oh!

khanhtc1202 Sep 3, 2025

Uh oh!

ffjlabo Sep 3, 2025

Uh oh!

khanhtc1202 left a comment

Uh oh!

khanhtc1202 left a comment

Uh oh!

Warashi left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	// Configure SSH config if needed.
	if cfg.Git.ShouldConfigureSSHConfig() {
	tempFile, err := git.AddSSHConfig(cfg.Git)
	if err != nil {
	input.Logger.Error("failed to configure ssh-config", zap.Error(err))
	return err
	}
	defer os.Remove(tempFile)
	input.Logger.Info("successfully configured ssh-config")
	}

	configv1 "github.com/pipe-cd/pipecd/pkg/configv1"
	"github.com/pipe-cd/pipecd/pkg/configv1"

		@@ -168,13 +168,15 @@ func (p *piped) run(ctx context.Context, input cli.Input) (runErr error) {
		registry := registerMetrics(cfg.PipedID, cfg.ProjectID, p.launcherVersion)

		// // Configure SSH config if needed.

Conversation

ffjlabo commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

khanhtc1202 Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

ffjlabo Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

ffjlabo Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

khanhtc1202 Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ffjlabo Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

khanhtc1202 Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

khanhtc1202 Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

ffjlabo Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

khanhtc1202 left a comment

Choose a reason for hiding this comment

Uh oh!

khanhtc1202 left a comment

Choose a reason for hiding this comment

Uh oh!

Warashi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ffjlabo commented Sep 3, 2025 •

edited

Loading

codecov Bot commented Sep 3, 2025 •

edited

Loading

khanhtc1202 Sep 3, 2025 •

edited

Loading