Deprecate cr index "charts-repo" flag. Instead read index.yaml from g…

…it repository (#144)

* read index.yaml from git repository

So far the index.yaml was loaded directly from the chart repository.
In case of GitHub pages that's https://<user or org>.github.io/<repo>

The problem with this approach is that GitHub pages use a Content
Delivery Network. So the index.yaml files fetched from there might be
outdated as there is a delay between a git push to the gh-pages branch
until the content is visible.

This delay might lead to the situation that one downloads an old index
file from and updated that one instead of the most recent index.yaml.

For the grafana repository that resulted in already released charts
being removed again.
- grafana/helm-charts#783
- grafana/helm-charts#796

This PR resolves that issue by reading the index.yaml from the gh_pages
branch of the repository. The branch needs to be up-to-date anyhow if
you want to create a PR or directly push the changes.

It should also be fine for the chart-releaser-action as it uses
a fetch-depth: 0 to clone the whole repository.

```
jobs:
  release:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v2
        with:
          fetch-depth: 0
```

Fixes: #143

Signed-off-by: Torsten Walter <torsten.walter@syncier.com>

* fix error handling

Signed-off-by: Torsten Walter <torsten.walter@syncier.com>

* re-add charts-repo flag to avoid breaking changes

- message is printed when using the flag to let users know that it's
  deprecated
- update all docs in README

Signed-off-by: Torsten Walter <torsten.walter@syncier.com>

* Update cr/cmd/index.go

Co-authored-by: Reinhard Nägele <unguiculus@gmail.com>
Signed-off-by: Torsten Walter <mail@torstenwalter.de>

* fix import ordering

Signed-off-by: Torsten Walter <torsten.walter@syncier.com>

Co-authored-by: Reinhard Nägele <unguiculus@gmail.com>

README.md

@@ -47,10 +47,11 @@ Usage:
   cr [command]
 
 Available Commands:
+  completion  generate the autocompletion script for the specified shell
   help        Help about any command
   index       Update Helm repo index.yaml for the given GitHub repo
-  upload      Upload Helm chart packages to GitHub Releases
   package     Package Helm charts
+  upload      Upload Helm chart packages to GitHub Releases
   version     Print version information
 
 Flags:
@@ -101,15 +102,18 @@ Usage:
   cr index [flags]
 
 Flags:
-  -c, --charts-repo string             The URL to the charts repository
   -b, --git-base-url string            GitHub Base URL (only needed for private GitHub) (default "https://api.github.com/")
   -r, --git-repo string                GitHub repository
   -u, --git-upload-url string          GitHub Upload URL (only needed for private GitHub) (default "https://uploads.github.com/")
   -h, --help                           help for index
   -i, --index-path string              Path to index file (default ".cr-index/index.yaml")
   -o, --owner string                   GitHub username or organization
   -p, --package-path string            Path to directory with chart packages (default ".cr-release-packages")
+      --pages-branch string            The GitHub pages branch (default "gh-pages")
+      --pr                             Create a pull request for index.yaml against the GitHub Pages branch (must not be set if --push is set)
+      --push                           Push index.yaml to the GitHub Pages branch (must not be set if --pr is set)
       --release-name-template string   Go template for computing release names, using chart metadata (default "{{ .Name }}-{{ .Version }}")
+      --remote string                  The Git remote used when creating a local worktree for the GitHub Pages branch (default "origin")
   -t, --token string                   GitHub Auth Token (only needed for private repos)
 
 Global Flags:

cr/cmd/index.go

@@ -15,6 +15,9 @@
 package cmd
 
 import (
+	"fmt"
+	"os"
+
 	"github.com/helm/chart-releaser/pkg/config"
 	"github.com/helm/chart-releaser/pkg/git"
 	"github.com/helm/chart-releaser/pkg/github"
@@ -35,6 +38,15 @@ given GitHub repository's releases.
 		if err != nil {
 			return err
 		}
+
+		if len(config.ChartsRepo) > 0 {
+			fmt.Fprintf(os.Stderr, "ATTENTION: Flag --charts-repo is deprecated. It does not have any effect.\n"+
+				"The index.yaml is read from the '%s' branch instead.\n"+
+				"Loading index.yaml directly from the charts repository lead to problems as there is a delay between\n"+
+				"pushing to the GitHub pages branch until things appear online.\n"+
+				"The flag will be removed with the next major release.", config.PagesBranch)
+		}
+
 		ghc := github.NewClient(config.Owner, config.GitRepo, config.Token, config.GitBaseURL, config.GitUploadURL)
 		releaser := releaser.NewReleaser(config, ghc, &git.Git{})
 		_, err = releaser.UpdateIndexFile()
@@ -43,7 +55,7 @@ given GitHub repository's releases.
 }
 
 func getRequiredIndexArgs() []string {
-	return []string{"owner", "git-repo", "charts-repo"}
+	return []string{"owner", "git-repo"}
 }
 
 func init() {
@@ -52,6 +64,7 @@ func init() {
 	flags.StringP("owner", "o", "", "GitHub username or organization")
 	flags.StringP("git-repo", "r", "", "GitHub repository")
 	flags.StringP("charts-repo", "c", "", "The URL to the charts repository")
+	_ = flags.MarkHidden("charts-repo")
 	flags.StringP("index-path", "i", ".cr-index/index.yaml", "Path to index file")
 	flags.StringP("package-path", "p", ".cr-release-packages", "Path to directory with chart packages")
 	flags.StringP("token", "t", "", "GitHub Auth Token (only needed for private repos)")

pkg/releaser/releaser.go

@@ -78,18 +78,16 @@ func (c *DefaultHttpClient) Get(url string) (resp *http.Response, err error) {
 }
 
 type Releaser struct {
-	config     *config.Options
-	github     GitHub
-	httpClient HttpClient
-	git        Git
+	config *config.Options
+	github GitHub
+	git    Git
 }
 
 func NewReleaser(config *config.Options, github GitHub, git Git) *Releaser {
 	return &Releaser{
-		config:     config,
-		github:     github,
-		httpClient: &DefaultHttpClient{},
-		git:        git,
+		config: config,
+		github: github,
+		git:    git,
 	}
 }
 
@@ -107,35 +105,25 @@ func (r *Releaser) UpdateIndexFile() (bool, error) {
 		}
 	}
 
-	var indexFile *repo.IndexFile
-
-	resp, err := r.httpClient.Get(fmt.Sprintf("%s/index.yaml", r.config.ChartsRepo))
+	fmt.Printf("Loading index file from git repository %s\n", r.config.IndexPath)
+	worktree, err := r.git.AddWorktree("", r.config.Remote+"/"+r.config.PagesBranch)
 	if err != nil {
 		return false, err
 	}
+	defer r.git.RemoveWorktree("", worktree) // nolint: errcheck
+	indexYamlPath := filepath.Join(worktree, "index.yaml")
 
-	defer resp.Body.Close()
-
-	if resp.StatusCode == http.StatusOK {
-		out, err := os.Create(r.config.IndexPath)
-		if err != nil {
-			return false, err
-		}
-		defer out.Close()
-
-		_, err = io.Copy(out, resp.Body)
-		if err != nil {
-			return false, err
-		}
-
-		fmt.Printf("Using existing index at %s\n", r.config.IndexPath)
-		indexFile, err = repo.LoadIndexFile(r.config.IndexPath)
+	var indexFile *repo.IndexFile
+	_, err = os.Stat(indexYamlPath)
+	if err == nil {
+		indexFile, err = repo.LoadIndexFile(indexYamlPath)
 		if err != nil {
 			return false, err
 		}
-	} else {
-		fmt.Printf("UpdateIndexFile new index at %s\n", r.config.IndexPath)
+	} else if errors.Is(err, os.ErrNotExist) {
 		indexFile = repo.NewIndexFile()
+	} else {
+		return false, err
 	}
 
 	// We have to explicitly glob for *.tgz files only. If GPG signing is enabled,
@@ -203,13 +191,6 @@ func (r *Releaser) UpdateIndexFile() (bool, error) {
 		return true, nil
 	}
 
-	worktree, err := r.git.AddWorktree("", r.config.Remote+"/"+r.config.PagesBranch)
-	if err != nil {
-		return false, err
-	}
-	defer r.git.RemoveWorktree("", worktree) // nolint: errcheck
-
-	indexYamlPath := filepath.Join(worktree, "index.yaml")
 	if err := copyFile(r.config.IndexPath, indexYamlPath); err != nil {
 		return false, err
 	}

pkg/releaser/releaser_test.go

@@ -15,11 +15,9 @@
 package releaser
 
 import (
-	"bufio"
 	"context"
 	"fmt"
 	"io/ioutil"
-	"net/http"
 	"os"
 	"path/filepath"
 	"testing"
@@ -38,19 +36,40 @@ type FakeGitHub struct {
 	release *github.Release
 }
 
-type MockClient struct {
-	statusCode int
-	file       string
+type FakeGit struct {
+	indexFile string
 }
 
-func (m *MockClient) Get(url string) (*http.Response, error) {
-	if m.statusCode == http.StatusOK {
-		file, _ := os.Open(m.file)
-		reader := bufio.NewReader(file)
-		return &http.Response{StatusCode: http.StatusOK, Body: ioutil.NopCloser(reader)}, nil
-	} else {
-		return &http.Response{StatusCode: http.StatusNotFound, Body: ioutil.NopCloser(nil)}, nil
+func (f *FakeGit) AddWorktree(workingDir string, committish string) (string, error) {
+	dir, err := ioutil.TempDir("", "chart-releaser-")
+	if err != nil {
+		return "", err
 	}
+	if len(f.indexFile) == 0 {
+		return dir, nil
+	}
+
+	return dir, copyFile(f.indexFile, filepath.Join(dir, "index.yaml"))
+}
+
+func (f *FakeGit) RemoveWorktree(workingDir string, path string) error {
+	return nil
+}
+
+func (f *FakeGit) Add(workingDir string, args ...string) error {
+	panic("implement me")
+}
+
+func (f *FakeGit) Commit(workingDir string, message string) error {
+	panic("implement me")
+}
+
+func (f *FakeGit) Push(workingDir string, args ...string) error {
+	panic("implement me")
+}
+
+func (f *FakeGit) GetPushURL(remote string, token string) (string, error) {
+	panic("implement me")
 }
 
 func (f *FakeGitHub) CreateRelease(ctx context.Context, input *github.Release) error {
@@ -97,8 +116,8 @@ func TestReleaser_UpdateIndexFile(t *testing.T) {
 					IndexPath:   "testdata/index/index.yaml",
 					PackagePath: "testdata/release-packages",
 				},
-				github:     fakeGitHub,
-				httpClient: &MockClient{http.StatusOK, "testdata/repo/index.yaml"},
+				github: fakeGitHub,
+				git:    &FakeGit{"testdata/repo/index.yaml"},
 			},
 		},
 		{
@@ -109,8 +128,8 @@ func TestReleaser_UpdateIndexFile(t *testing.T) {
 					IndexPath:   filepath.Join(indexDir, "index.yaml"),
 					PackagePath: "testdata/release-packages",
 				},
-				github:     fakeGitHub,
-				httpClient: &MockClient{http.StatusNotFound, ""},
+				github: fakeGitHub,
+				git:    &FakeGit{""},
 			},
 		},
 	}
@@ -151,8 +170,8 @@ func TestReleaser_UpdateIndexFileGenerated(t *testing.T) {
 					IndexPath:   filepath.Join(indexDir, "index.yaml"),
 					PackagePath: "testdata/release-packages",
 				},
-				github:     fakeGitHub,
-				httpClient: &MockClient{http.StatusOK, "testdata/empty-repo/index.yaml"},
+				github: fakeGitHub,
+				git:    &FakeGit{indexFile: "testdata/empty-repo/index.yaml"},
 			},
 		},
 	}
@@ -166,6 +185,7 @@ func TestReleaser_UpdateIndexFileGenerated(t *testing.T) {
 			newIndexFile, _ := repo.LoadIndexFile(tt.releaser.config.IndexPath)
 			newGenerated := newIndexFile.Generated
 			assert.True(t, newGenerated.After(generated))
+			assert.Equal(t, 2, len(newIndexFile.Entries))
 		})
 	}
 }