Skip to content

Commit

Permalink
Modified to use Git with PAT (#4571)
Browse files Browse the repository at this point in the history
* fix to read PAT settings from file

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* piped

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* include  PAT information in URL

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: modification of conditional branching

Co-authored-by: sivchari <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: corrected error in error message

Co-authored-by: sivchari <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: integration of mask function

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: make validation test

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: function name

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: rename function for validation PAT

Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: fix test code as pointed out in the review

Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* feat: add explan
for git personal access token in document

Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: change required in documentation

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: change return value

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: add test case

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: fix test

Signed-off-by: swallow <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: PipedGit struct to use password
authentication instead of personal access token

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix to read PAT settings from file

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* piped

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: integration of mask function

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: make validation test

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: function name

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: rename function for validation PAT

Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: fix test

Signed-off-by: swallow <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: PipedGit struct to use password
authentication instead of personal access token

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* Fix Git authentication configuration

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* Update password authentication configuration

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* Fix error variable name

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* Fix rename password

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* Refactor includePasswordAuthRemote function

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* Update password authentication in clone test

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: delete PasswordAuth

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: remove unused PasswordAuth field and refactor password authentication in git client

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* Remove unnecessary print statement in Validate function

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: fix code for rebase

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* fix: remove unused GitPasswordAuth configuration

Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* feat: add password decoding for password in includePasswordRemote function

Signed-off-by: sZma5a <[email protected]>

* fix: refactor Git password authentication method

Signed-off-by: sZma5a <[email protected]>

* fix: update password encoding in TestCloneUsingPassword

Signed-off-by: sZma5a <[email protected]>

* Update docs/content/en/docs-dev/user-guide/managing-piped/configuration-reference.md

Co-authored-by: Yoshiki Fujikane <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* Update pkg/config/piped.go

Co-authored-by: Yoshiki Fujikane <[email protected]>
Signed-off-by: sZma5a <[email protected]>

* [wip] delete password

Signed-off-by: sZma5a <[email protected]>

* [wip] not tested - change token to args from url

Signed-off-by: sZma5a <[email protected]>

* Fix commented out test case

Signed-off-by: sZma5a <[email protected]>

* Refactor authentication in git client

Signed-off-by: sZma5a <[email protected]>

* feat: add password decoding function and replace Password string

Signed-off-by: sZma5a <[email protected]>

---------

Signed-off-by: sZma5a <[email protected]>
Signed-off-by: 鈴木 優耀 <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: sZma5a <[email protected]>
Signed-off-by: swallow <[email protected]>
Co-authored-by: sZma5a <[email protected]>
Co-authored-by: sivchari <[email protected]>
Co-authored-by: 鈴木 優耀 <[email protected]>
Co-authored-by: Your Name <[email protected]>
Co-authored-by: Yoshiki Fujikane <[email protected]>
Signed-off-by: pipecd-bot <[email protected]>
  • Loading branch information
6 people authored and pipecd-bot committed Sep 5, 2024
1 parent 8c47ec6 commit 59bf54e
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 2 deletions.
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 @@ -278,11 +278,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 @@ -462,12 +469,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: "[email protected]",
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: "[email protected]",
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))
}

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

0 comments on commit 59bf54e

Please sign in to comment.