Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified to use Git with PAT #4571

Merged
merged 45 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
0ff6e9e
fix to read PAT settings from file
Jul 30, 2023
136ba0d
piped
Jul 30, 2023
9f46a5b
include PAT information in URL
Jul 30, 2023
59e710a
fix: modification of conditional branching
sZma5a Aug 1, 2023
723ff96
fix: corrected error in error message
sZma5a Aug 1, 2023
7f8fb16
fix: integration of mask function
Aug 2, 2023
529386a
fix: make validation test
Aug 2, 2023
8edb406
fix: function name
Aug 2, 2023
249f169
fix: rename function for validation PAT
Aug 13, 2023
0e1eb27
fix: fix test code as pointed out in the review
Aug 13, 2023
7cf87be
feat: add explan
Aug 13, 2023
4e69f72
fix: change required in documentation
sZma5a Aug 27, 2023
882f671
fix: change return value
sZma5a Aug 27, 2023
4e2eb55
fix: add test case
sZma5a Aug 27, 2023
71cc28f
fix: fix test
sZma5a Nov 5, 2023
077646a
fix: PipedGit struct to use password
sZma5a Dec 17, 2023
42d59a0
fix to read PAT settings from file
Jul 30, 2023
63da02b
piped
Jul 30, 2023
9f16b6c
fix: integration of mask function
Aug 2, 2023
013c2d6
fix: make validation test
Aug 2, 2023
5540763
fix: function name
Aug 2, 2023
32691b0
fix: rename function for validation PAT
Aug 13, 2023
2d4e2a2
fix: fix test
sZma5a Nov 5, 2023
6b2bb7a
fix: PipedGit struct to use password
sZma5a Dec 17, 2023
00d7ac2
Fix Git authentication configuration
sZma5a Dec 17, 2023
e04a498
Update password authentication configuration
sZma5a Dec 17, 2023
e1b258e
Fix error variable name
sZma5a Dec 17, 2023
7ac74e9
Fix rename password
sZma5a Dec 17, 2023
43b09c5
Refactor includePasswordAuthRemote function
sZma5a Dec 17, 2023
9c7df14
Update password authentication in clone test
sZma5a Dec 17, 2023
c7f5816
fix: delete PasswordAuth
Feb 12, 2024
5a29e38
fix: remove unused PasswordAuth field and refactor password authentic…
Feb 12, 2024
46567d7
Remove unnecessary print statement in Validate function
Feb 12, 2024
080c051
fix: fix code for rebase
Feb 12, 2024
2d562e6
fix: remove unused GitPasswordAuth configuration
Feb 12, 2024
6e268e3
feat: add password decoding for password in includePasswordRemote fun…
Feb 12, 2024
04b8f02
fix: refactor Git password authentication method
Feb 12, 2024
f371e09
fix: update password encoding in TestCloneUsingPassword
Feb 12, 2024
8378040
Update docs/content/en/docs-dev/user-guide/managing-piped/configurati…
sZma5a Apr 1, 2024
d2eaab3
Update pkg/config/piped.go
sZma5a Apr 1, 2024
9130a30
[wip] delete password
Apr 18, 2024
3c9d5d2
[wip] not tested - change token to args from url
Apr 18, 2024
2f5906b
Fix commented out test case
sZma5a Apr 18, 2024
9b9da4d
Refactor authentication in git client
sZma5a Apr 18, 2024
a93d94b
feat: add password decoding function and replace Password string
sZma5a Aug 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ spec:
| hostName | string | The hostname or IP address of the remote git server. Default is the same value with Host. | No |
| sshKeyFile | string | The path to the private ssh key file. This will be used to clone the source code of the specified git repositories. | No |
| sshKeyData | string | Base64 encoded string of SSH key. | No |
| password | string | The base64 encoded password for git used while cloning above Git repository. | No |

## GitRepository

Expand Down
14 changes: 14 additions & 0 deletions pkg/app/piped/cmd/piped/piped.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,18 @@ func (p *piped) run(ctx context.Context, input cli.Input) (runErr error) {
})
}

password, err := cfg.Git.DecodedPassword()
if err != nil {
input.Logger.Error("failed to decode password", zap.Error(err))
return err
}

// Initialize git client.
gitOptions := []git.Option{
git.WithUserName(cfg.Git.Username),
git.WithEmail(cfg.Git.Email),
git.WithLogger(input.Logger),
git.WithPassword(password),
}
for _, repo := range cfg.GitHelmChartRepositories() {
if f := repo.SSHKeyFile; f != "" {
Expand Down Expand Up @@ -473,12 +480,19 @@ func (p *piped) run(ctx context.Context, input cli.Input) (runErr error) {

// Start running planpreview handler.
{
// Decode password for plan-preview feature.
password, err := cfg.Git.DecodedPassword()
if err != nil {
input.Logger.Error("failed to decode password", zap.Error(err))
return err
}
// Initialize a dedicated git client for plan-preview feature.
// Basically, this feature is an utility so it should not share any resource with the main components of piped.
gc, err := git.NewClient(
git.WithUserName(cfg.Git.Username),
git.WithEmail(cfg.Git.Email),
git.WithLogger(input.Logger),
git.WithPassword(password),
)
if err != nil {
input.Logger.Error("failed to initialize git client for plan-preview", zap.Error(err))
Expand Down
35 changes: 35 additions & 0 deletions pkg/config/piped.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ func (s *PipedSpec) Validate() error {
if s.SyncInterval < 0 {
return errors.New("syncInterval must be greater than or equal to 0")
}
if err := s.Git.Validate(); err != nil {
return err
}
for _, r := range s.ChartRepositories {
if err := r.Validate(); err != nil {
return err
Expand Down Expand Up @@ -326,6 +329,9 @@ type PipedGit struct {
SSHKeyFile string `json:"sshKeyFile,omitempty"`
// Base64 encoded string of ssh-key.
SSHKeyData string `json:"sshKeyData,omitempty"`
// Base64 encoded string of password.
// This will be used to clone the source repo with https basic auth.
Password string `json:"password,omitempty"`
}

func (g PipedGit) ShouldConfigureSSHConfig() bool {
Expand All @@ -345,6 +351,21 @@ func (g PipedGit) LoadSSHKey() ([]byte, error) {
return nil, errors.New("either sshKeyFile or sshKeyData must be set")
}

func (g *PipedGit) Validate() error {
isPassword := g.Password != ""
isSSH := g.ShouldConfigureSSHConfig()
if isSSH && isPassword {
return errors.New("cannot configure both sshKeyData or sshKeyFile and password authentication")
}
if isSSH && (g.SSHKeyData != "" && g.SSHKeyFile != "") {
return errors.New("only either sshKeyFile or sshKeyData can be set")
}
if isPassword && (g.Username == "" || g.Password == "") {
return errors.New("both username and password must be set")
}
return nil
}

func (g *PipedGit) Mask() {
if len(g.SSHConfigFilePath) != 0 {
g.SSHConfigFilePath = maskString
Expand All @@ -355,6 +376,20 @@ func (g *PipedGit) Mask() {
if len(g.SSHKeyData) != 0 {
g.SSHKeyData = maskString
}
if len(g.Password) != 0 {
g.Password = maskString
}
}

func (g *PipedGit) DecodedPassword() (string, error) {
if len(g.Password) == 0 {
return "", nil
}
decoded, err := base64.StdEncoding.DecodeString(g.Password)
if err != nil {
return "", err
}
return string(decoded), nil
}

type PipedRepository struct {
Expand Down
83 changes: 83 additions & 0 deletions pkg/config/piped_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package config

import (
"errors"
"testing"
"time"

Expand Down Expand Up @@ -607,6 +608,7 @@ func TestPipedConfigMask(t *testing.T) {
HostName: "foo",
SSHKeyFile: "foo",
SSHKeyData: "foo",
Password: "foo",
},
Repositories: []PipedRepository{
{
Expand Down Expand Up @@ -770,6 +772,7 @@ func TestPipedConfigMask(t *testing.T) {
HostName: "foo",
SSHKeyFile: maskString,
SSHKeyData: maskString,
Password: maskString,
},
Repositories: []PipedRepository{
{
Expand Down Expand Up @@ -948,6 +951,7 @@ func TestPipedSpecClone(t *testing.T) {
Username: "username",
Email: "username@email.com",
SSHKeyFile: "/etc/piped-secret/ssh-key",
Password: "Password",
},
Repositories: []PipedRepository{
{
Expand Down Expand Up @@ -1145,6 +1149,7 @@ func TestPipedSpecClone(t *testing.T) {
Username: "username",
Email: "username@email.com",
SSHKeyFile: "/etc/piped-secret/ssh-key",
Password: "Password",
},
Repositories: []PipedRepository{
{
Expand Down Expand Up @@ -1435,3 +1440,81 @@ func TestFindPlatformProvidersByLabel(t *testing.T) {
})
}
}

func TestPipeGitValidate(t *testing.T) {
t.Parallel()
testcases := []struct {
name string
git PipedGit
err error
}{
{
name: "Both SSH and Password are not valid",
git: PipedGit{
SSHKeyData: "sshkey1",
Password: "Password",
},
err: errors.New("cannot configure both sshKeyData or sshKeyFile and password authentication"),
},
{
name: "Both SSH and Password is not valid",
git: PipedGit{
SSHKeyFile: "sshkeyfile",
SSHKeyData: "sshkeydata",
Password: "Password",
},
err: errors.New("cannot configure both sshKeyData or sshKeyFile and password authentication"),
},
{
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",
},
err: errors.New("only either sshKeyFile or sshKeyData can be set"),
},
{
name: "Password is valid",
git: PipedGit{
Username: "Username",
Password: "Password",
},
err: nil,
},
{
name: "Username is empty",
git: PipedGit{
Username: "",
Password: "Password",
},
err: errors.New("both username and password must be set"),
},
{
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)
})
}
}
26 changes: 24 additions & 2 deletions pkg/git/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package git

import (
"context"
"encoding/base64"
"fmt"
"os"
"os/exec"
Expand Down Expand Up @@ -48,6 +49,7 @@ type client struct {
cacheDir string
mu sync.Mutex
repoLocks map[string]*sync.Mutex
password string

gitEnvs []string
gitEnvsByRepo map[string][]string
Expand Down Expand Up @@ -90,6 +92,14 @@ func WithEmail(e string) Option {
}
}

func WithPassword(password string) Option {
return func(c *client) {
if password != "" {
c.password = password
}
}
}

// NewClient creates a new CLient instance for cloning git repositories.
// After using Clean should be called to delete cache data.
func NewClient(opts ...Option) (Client, error) {
Expand Down Expand Up @@ -132,6 +142,14 @@ func (c *client) Clone(ctx context.Context, repoID, remote, branch, destination
)
)

authArgs := []string{}
if c.username != "" && c.password != "" {
token := fmt.Sprintf("%s:%s", c.username, c.password)
encodedToken := base64.StdEncoding.EncodeToString([]byte(token))
header := fmt.Sprintf("Authorization: Basic %s", encodedToken)
authArgs = append(authArgs, "-c", fmt.Sprintf("http.extraHeader=%s", header))
}

Comment on lines +145 to +152
Copy link
Member

@ffjlabo ffjlabo Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ffjlabo
Sorry for the delay.
Regarding git, I was able to perform a clone using PAT in my environment with the following command.

git -c http.extraHeader="Authorization: Basic {base64(username:token)}" clone --mirror https://github.com/PRIVATE_REPO ./tmp2 

In addition, the Golang code that was tested is shown below.

package main

import (
	"context"
	"fmt"

	"github.com/pipe-cd/pipecd/pkg/git"
)

func main() {
	uo := git.WithUserName("USERNAME")
	po := git.WithPassword("PAT")
	c, err := git.NewClient(uo, po)
	if err != nil {
		fmt.Println(err)
		panic(err)
	}
	ctx := context.Background()
	c.Clone(
		ctx,
		"test1",
		"https://github.com/PRIVATE_REPO",
		"main",
		"./tmp",
	)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked with the below command and succeeded in cloning the repository.

git -c http.extraHeader="Authorization: Basic $(echo -n "x-access-token:$(gh auth token)" | base64)" clone --mirror https://github.com/Warashi/PRIVATE.git ./tmp2

When I removed the -n option from echo as below, I got the same error with @ffjlabo.

git -c http.extraHeader="Authorization: Basic $(echo "x-access-token:$(gh auth token)" | base64)" clone --mirror https://github.com/Warashi/PRIVATE.git ./tmp2

So, I think that @ffjlabo appended the \n to the password before encoding it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to be late 🙏
It's my fault, as Wrashi-san says. The command worked as expected.
Thank you!

BTW, there are some fixed points in this comment, so plz check it🙏
#4571 (review)

c.lockRepo(repoID)
defer c.unlockRepo(repoID)

Expand All @@ -147,7 +165,9 @@ func (c *client) Clone(ctx context.Context, repoID, remote, branch, destination
return nil, err
}
out, err := retryCommand(3, time.Second, logger, func() ([]byte, error) {
return runGitCommand(ctx, c.gitPath, "", c.envsForRepo(remote), "clone", "--mirror", remote, repoCachePath)
args := []string{"clone", "--mirror", remote, repoCachePath}
args = append(authArgs, args...)
return runGitCommand(ctx, c.gitPath, "", c.envsForRepo(remote), args...)
})
if err != nil {
logger.Error("failed to clone from remote",
Expand All @@ -160,7 +180,9 @@ func (c *client) Clone(ctx context.Context, repoID, remote, branch, destination
// Cache hit. Do a git fetch to keep updated.
c.logger.Info(fmt.Sprintf("fetching %s to update the cache", repoID))
out, err := retryCommand(3, time.Second, c.logger, func() ([]byte, error) {
return runGitCommand(ctx, c.gitPath, repoCachePath, c.envsForRepo(remote), "fetch")
args := []string{"fetch"}
args = append(authArgs, args...)
return runGitCommand(ctx, c.gitPath, repoCachePath, c.envsForRepo(remote), args...)
})
if err != nil {
logger.Error("failed to fetch from remote",
Expand Down
Loading