Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -59,11 +59,14 @@ spec:

| Field | Type | Description | Required |
|-|-|-|-|
| type | string | The repository type. Currently, HTTP and GIT are supported. Default is HTTP. | No |
| name | string | The name of the Helm chart repository. Note that is not a Git repository but a [Helm chart repository](https://helm.sh/docs/topics/chart_repository/). | Yes |
| address | string | The address to the Helm chart repository. | Yes |
| username | string | Username used for the repository backed by HTTP basic authentication. | No |
| password | string | Password used for the repository backed by HTTP basic authentication. | No |
| insecure | bool | Whether to skip TLS certificate checks for the repository or not. | No |
| gitRemote | string | Remote address of the Git repository used to clone Helm charts. | No |
| sshKeyFile | string | The path to the private ssh key file used while cloning Helm charts from above Git repository. | No |

## CloudProvider

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

// Add configured Helm chart repositories.
if len(cfg.ChartRepositories) > 0 {
if repos := cfg.HTTPHelmChartRepositories(); len(repos) > 0 {
reg := toolregistry.DefaultRegistry()
if err := chartrepo.Add(ctx, cfg.ChartRepositories, reg, input.Logger); err != nil {
if err := chartrepo.Add(ctx, repos, reg, input.Logger); err != nil {
input.Logger.Error("failed to add configured chart repositories", zap.Error(err))
return err
}
if len(cfg.ChartRepositories) > 0 {
if err := chartrepo.Update(ctx, reg, input.Logger); err != nil {
input.Logger.Error("failed to update Helm chart repositories", zap.Error(err))
return err
}
if err := chartrepo.Update(ctx, reg, input.Logger); err != nil {
input.Logger.Error("failed to update Helm chart repositories", zap.Error(err))
return err
}
}

Expand Down Expand Up @@ -229,11 +227,19 @@ func (p *piped) run(ctx context.Context, input cli.Input) (runErr error) {
}

// Initialize git client.
gitClient, err := git.NewClient(
gitOptions := []git.Option{
git.WithUserName(cfg.Git.Username),
git.WithEmail(cfg.Git.Email),
git.WithLogger(input.Logger),
)
}
for _, repo := range cfg.GitHelmChartRepositories() {
if f := repo.SSHKeyFile; f != "" {
// Configure git client to use the specified SSH key while fetching private Helm charts.
env := fmt.Sprintf("GIT_SSH_COMMAND=ssh -i %s -o StrictHostKeyChecking=no -F /dev/null", f)
gitOptions = append(gitOptions, git.WithGitEnvForRepo(repo.GitRemote, env))
}
}
gitClient, err := git.NewClient(gitOptions...)
if err != nil {
input.Logger.Error("failed to initialize git client", zap.Error(err))
return err
Expand Down
73 changes: 73 additions & 0 deletions pkg/config/piped.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ func (s *PipedSpec) Validate() error {
if s.SyncInterval < 0 {
return errors.New("syncInterval must be greater than or equal to 0")
}
for _, r := range s.ChartRepositories {
if err := r.Validate(); err != nil {
return err
}
}
if s.SecretManagement != nil {
if err := s.SecretManagement.Validate(); err != nil {
return err
Expand Down Expand Up @@ -245,7 +250,19 @@ type PipedRepository struct {
Branch string `json:"branch"`
}

type HelmChartRepositoryType string

const (
HTTPHelmChartRepository HelmChartRepositoryType = "HTTP"
GITHelmChartRepository HelmChartRepositoryType = "GIT"
)

type HelmChartRepository struct {
// The repository type. Currently, HTTP and GIT are supported.
// Default is HTTP.
Type HelmChartRepositoryType `json:"type" default:"HTTP"`

// Configuration for HTTP type.
// The name of the Helm chart repository.
Name string `json:"name"`
// The address to the Helm chart repository.
Expand All @@ -256,6 +273,62 @@ type HelmChartRepository struct {
Password string `json:"password"`
// Whether to skip TLS certificate checks for the repository or not.
Insecure bool `json:"insecure"`

// Configuration for GIT type.
// Remote address of the Git repository used to clone Helm charts.
// e.g. [email protected]:org/repo.git
GitRemote string `json:"gitRemote"`
// The path to the private ssh key file used while cloning Helm charts from above Git repository.
SSHKeyFile string `json:"sshKeyFile"`
}

func (r *HelmChartRepository) IsHTTPRepository() bool {
return r.Type == HTTPHelmChartRepository
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return r.Type == HTTPHelmChartRepository
return strings.ToUpper(r.Type) == HTTPHelmChartRepository

nits, users may insert HTTP, http or Http as well. Or we may need to check for the type field in the validation function at L293.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that we should allow that. Like, KubernetesApp not K8sApp or kubernetesApp.

Copy link
Member

@nakabonne nakabonne Nov 29, 2021

Choose a reason for hiding this comment

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

I also think we should distinguish between lowercase and uppercase.

Copy link
Member

@khanhtc1202 khanhtc1202 Nov 29, 2021

Choose a reason for hiding this comment

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

You're right, in that case, we may need to add validate r.Type to the validate function at line L293, and the current error message (at L311) return Git instead of GIT
https://github.com/pipe-cd/pipe/pull/2863/files#diff-49258a851400c4bc54c40491b440d7edf1dd45eda52c9eccd3f5371af15e8409R311

Copy link
Member

Choose a reason for hiding this comment

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

Yep, that should be:

return fmt.Errorf("either %s repository or %s repository must be configured", HTTPHelmChartRepository, GITHelmChartRepository)

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch!
Fixed it.

}

func (r *HelmChartRepository) IsGitRepository() bool {
return r.Type == GITHelmChartRepository
}

func (r *HelmChartRepository) Validate() error {
if r.IsHTTPRepository() {
if r.Name == "" {
return errors.New("name must be set")
}
if r.Address == "" {
return errors.New("address must be set")
}
return nil
}

if r.IsGitRepository() {
if r.GitRemote == "" {
return errors.New("gitRemote must be set")
}
return nil
}

return fmt.Errorf("either %s repository or %s repository must be configured", HTTPHelmChartRepository, GITHelmChartRepository)
}

func (s *PipedSpec) HTTPHelmChartRepositories() []HelmChartRepository {
repos := make([]HelmChartRepository, 0, len(s.ChartRepositories))
for _, r := range s.ChartRepositories {
if r.IsHTTPRepository() {
repos = append(repos, r)
}
}
return repos
}

func (s *PipedSpec) GitHelmChartRepositories() []HelmChartRepository {
repos := make([]HelmChartRepository, 0, len(s.ChartRepositories))
for _, r := range s.ChartRepositories {
if r.IsGitRepository() {
repos = append(repos, r)
}
}
return repos
}

type PipedCloudProvider struct {
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/piped_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,12 @@ func TestPipedConfig(t *testing.T) {
},
ChartRepositories: []HelmChartRepository{
{
Type: HTTPHelmChartRepository,
Name: "fantastic-charts",
Address: "https://fantastic-charts.storage.googleapis.com",
},
{
Type: HTTPHelmChartRepository,
Name: "private-charts",
Address: "https://private-charts.com",
Username: "basic-username",
Expand Down