Modified to use Git with PAT

[sZma5a]

What this PR does / why we need it: Modified to use Personal Access Token since currently only SSH can control the Git repository.

Which issue(s) this PR fixes:

Fixes #4106

Does this PR introduce a user-facing change?: Yes

• How are users affected by this change: Be able to use Personal Access Token setting like this:

apiVersion: pipecd.dev/v1beta1
kind: Piped
spec:
  git:
     personalAccessToken:
        userName: <user-name>
        userToken: <user-token>

• Is this breaking change: No
• How to migrate (if breaking change):

[sZma5a]

@kentakozuka

Sorry...
I accidentally closed a previously created PR, but was unable to reopen it, so I created a side...

#4534

[codecov]

Codecov Report

Attention: Patch coverage is 54.16667% with 22 lines in your changes missing coverage. Please review.

Project coverage is 28.93%. Comparing base (14eb473) to head (87be877).
Report is 37 commits behind head on master.

❗ Current head 87be877 differs from pull request most recent head 1291bab

Please upload reports for the commit 1291bab to get more accurate results.

Files	Patch %	Lines
pkg/git/client.go	40.00%	12 Missing and 3 partials ⚠️
pkg/app/piped/cmd/piped/piped.go	0.00%	4 Missing ⚠️
pkg/config/piped.go	84.21%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4571      +/-   ##
==========================================
- Coverage   29.23%   28.93%   -0.30%     
==========================================
  Files         318      317       -1     
  Lines       40597    40413     -184     
==========================================
- Hits        11870    11695     -175     
+ Misses      27787    27786       -1     
+ Partials      940      932       -8     

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

[kentakozuka]

Thanks! Left a comment. PTAL 👀

[kentakozuka]

IMHO, the test above misses some cases.
Can you copy and paste the test bellow, fix TODOs, and run it?

func TestPipeGitValidate(t *testing.T) {
	t.Parallel()
	testcases := []struct {
		name string
		git  PipedGit
		err  error
	}{
		{
			name: "both SSH and PAT are valid",
			git: PipedGit{
				SSHKeyData: "sshkey1",
				PersonalAccessToken: PipedGitPersonalAccessToken{
					UserName:  "UserName",
					UserToken: "UserToken",
				},
			},
			err: errors.New("cannot configure both sshKeyData or sshKeyFile and personalAccessToken"),
		},
		{
			name: "Both SSH and PAT is not valid",
			git: PipedGit{
				SSHKeyFile: "sshkeyfile",
				SSHKeyData: "sshkeydata",
				PersonalAccessToken: PipedGitPersonalAccessToken{
					UserName:  "",
					UserToken: "UserToken",
				},
			},
			// TODO: should return error
		},
		{
			name: "SSH key data is not empty",
			git: PipedGit{
				SSHKeyData: "sshkey2",
			},
			err: nil,
		},
		{
			name: "SSH key file is not empty",
			git: PipedGit{
				SSHKeyFile: "sshkey2",
			},
			err: nil,
		},
		{
			name: "Both SSH file and data is not empty",
			git: PipedGit{
				SSHKeyData: "sshkeydata",
				SSHKeyFile: "sshkeyfile",
			},
			// TODO: should return error
		},
		{
			name: "PAT is valid",
			git: PipedGit{
				PersonalAccessToken: PipedGitPersonalAccessToken{
					UserName:  "UserName",
					UserToken: "UserToken",
				},
			},
			err: nil,
		},
		{
			name: "PAT username is empty",
			git: PipedGit{
				PersonalAccessToken: PipedGitPersonalAccessToken{
					UserName:  "UserName",
					UserToken: "",
				},
			},
			// TODO: should return error
		},
		{
			name: "PAT token is empty",
			git: PipedGit{
				PersonalAccessToken: PipedGitPersonalAccessToken{
					UserName:  "",
					UserToken: "UserToken",
				},
			},
			// TODO: should return error
		},
		{
			name: "Git config is empty",
			git:  PipedGit{},
			err:  nil,
		},
	}
	for _, tc := range testcases {
		tc := tc
		t.Run(tc.git.SSHKeyData, func(t *testing.T) {
			t.Parallel()
			err := tc.git.Validate()
			assert.Equal(t, tc.err, err)
		})
	}
}

[kentakozuka]

Just realized that;
PipeCD supports not only GitHub but also Git services(GitLab, self-host Git server, etc.).
So, it should be how this configuration can be used for other Git services.

How about simply adding password instead of personalAccessToken?
WDYT?

[sZma5a]

Are you suggesting replacing the Personal Access Token with a password?
Using a password might lead to confusion, so maybe we could stick with some form of access token instead?

[sZma5a]

@kentakozuka

[kentakozuka]

You are right. It can be confusing for GitHub users. But, IMHO, using the word password should be better because Personal access tokens can be used in GitHub (and GitLab as well), but it is not required for being a Git server. PipeCD supports Git (not GitHub) so it should respect Git terminology.

[sZma5a]

Since 'Password' overlaps with the variable name, 'PersonalAccessToken' renamed to 'PasswordAuth' and 'userToken' to 'Password'.

[sZma5a]

@kentakozuka
Sorry I forgot to add a mentions...

[ffjlabo]

@sZma5a @kentakozuka
To clarify this discussion, I want to organize your opinions and give my opinion 👀
Feel free to tell me if there is a misunderstanding :)

The point is whether to define a variable only for PAT or not.
For me, each opinion of yours seems to be like below 👀

[@kentakozuka 's opinion]
GitHub's PAT behaves like a password on the git. It's just a password on the git.

[@sZma5a 's opinion]
PAT is a different role from a password. So It's confusing to define it as a Password.

I think both opinions are important points 👍
So I will try to give my opinion after considering those opinions!

[IMO]

• Define the variable as a Password (spec.git.password on piped config) because PAT is just a password on git. (ref: https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens#using-a-personal-access-token-on-the-command-line)
• The Password is like a base64 encoded value to avoid the security risk. (the same as sshKeyData)
• Add documents and comments to explain that we can use both PAT and a password for spec.git.password.
  • The example is "the password for git. You can also use GitHub's Personal Access Token."

Maybe this PR is like a supporting password for git on piped :)

WDYT?

[sZma5a]

Understood, thank you very much!

[sZma5a]

@ffjlabo @kentakozuka
Sorry it took so long to fix.
I have modified the PAT settings to use a password, and updated it to work with the existing Git username!
How does this look?

[github-actions]

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[sZma5a]

@Warashi
Sorry be late.
I fixed your comment.
I would appreciate it if you could review this.
#4571 (review)

[Warashi]

I checked the behavior, and this worked as expected!
Thank you. LGTM.

[ffjlabo]

Thank you for the great contribution 🚀

[sZma5a]

Thank you very much for following me!