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 are missing coverage. Please review.

Project coverage is 28.93%. Comparing base (14eb473) to head (87be877).

❗ Current head 87be877 differs from pull request most recent head 1291bab. Consider uploading 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.

[ffjlabo]

@sZma5a Thank you for the fix! I'm checking 👍

[ffjlabo]

At first, thank you for investigating and the fix 🙏 I don't know such an option! It's so helpful 👍
noted: https://git-scm.com/docs/git-config#Documentation/git-config.txt-httpextraHeader

I tried the expected command with a private repo git -c http.extraHeader="Authorization: Basic <base64 encoded value of username:password>" clone https://github.com/ffjlabo-playground/git-test.git on mac.
I encountered the behaviors written below. Does below also reproduce on your machine? Could you try it?

• It shows the prompts for username and password like this↓

% git -c http.extraHeader="Authorization: Basic <base64 encoded value of username:password>" clone https://github.com/ffjlabo-playground/git-test.git
Cloning into 'git-test'...
Username for 'https://github.com':

• When I input the username and password as empty, then It shows the auth error like this↓

% git -c http.extraHeader="Authorization: Basic <base64 encoded value of username:password>" clone https://github.com/ffjlabo-playground/git-test.git
Cloning into 'git-test'...
Username for 'https://github.com':
Password for 'https://github.com':
remote: Support for password authentication was removed on August 13, 2021.
remote: Please see https://docs.github.com/get-started/getting-started-with-git/about-remote-repositories#cloning-with-https-urls for information on currently recommended modes of authentication.
fatal: Authentication failed for 'https://github.com/ffjlabo-playground/git-test.git/'

[sZma5a]

Sorry...
It worked when we looked into it before, but we will look into it again and verify.

[ffjlabo]

Thank you! Don't worry! I think this is a challenging. I'm so helpful.

[ffjlabo]

Hi @sZma5a ! Do you have any updates on the above? If you want some helps, feel free to ping me 👍