Adds feature to fetch release values and secret values from remote #47

dol · 2022-04-19T15:27:17Z

The releases and environment section allow for values files on the local
disk.
This enhancement allows for referencing remote (go-getter) files to be
fetched, cached and referenced.

Example:

repositories:
  - name: "incubator"
    url: "https://charts.helm.sh/incubator/"

releases:
  - name: "test"
    namespace: "test"
    chart: "incubator/raw"
    version: "0.1.0"
    missingFileHandler: Debug
    values:
      # Reference a remote source file
      - "git::https://github.com/helm/charts.git@raw/ci/values.yaml"

In addition when fetching a remote git source with a ssh key the ssh key
will not be part of the caching folder name. This avoids two problems:

Don't leak sensitive information in the name of the caching folder
Base64 encoded SSH keys are very long. On some file systems the max
lenght of the directory name is hit when using the full base64
information in the path name.

The sshkey informations are reducted. Because of this fixed string
there is a change of colloding cache names. The likelihood of this
collision is very low. The git repo and git reference need to be the
same, but the sshkey can change. This will result in the same source to
be checkout out and referenced.

dol · 2022-04-20T06:58:55Z

pkg/remote/remote.go

@@ -186,7 +187,11 @@ func (r *Remote) Fetch(goGetterSrc string, cacheDirOpt ...string) (string, error
 	replacer := strings.NewReplacer(":", "", "//", "_", "/", "_", ".", "_")
 	dirKey := replacer.Replace(srcDir)
 	if len(query) > 0 {
-		paramsKey := strings.Replace(query, "&", "_", -1)
+		q, _ := neturl.ParseQuery(query)


According to https://github.com/hashicorp/go-getter/blob/4553965d9c4a8d99bd0d381c1180c08e07eff5fd/README.md#protocol-specific-options-1 there are multiple supported protocols like git, http, s3 and gcs. All this protocols can include some sensitive information in any URL part.
Instead of redacting ever possible query parameter the whole URL could be hashed with sha256. This will fix the two problems mentioned in this pull request.

Don't leak sensitive information

Avoid hitting the max length of a folder name

kuzaxak · 2022-04-20T12:04:12Z

pkg/state/storage.go

@@ -30,7 +31,17 @@ func NewStorage(forFile string, logger *zap.SugaredLogger, glob func(string) ([]
 func (st *Storage) resolveFile(missingFileHandler *string, tpe, path string) ([]string, bool, error) {


This enhancement allows for referencing remote (go-getter) files to be
fetched, cached and referenced.

Loading env values from remote already supported, link. Here is a code that is responsible for that.

Does it mean that you propose to trait any file link as a potentially remote file? In that I think LoadEnvironmentValues function should be adjusted as well.

Thank you for pointing out. The PR solves the same issues ( roboll/helmfile#1838 / roboll/helmfile#1499 ) as your RP #42. The title of the PR was misleading and I changed it from environment to release. I also added a basic example to illustrate the enhancement.

The difference of the two implementations is, that this PR also fetches secret values files. Since your PR only patches generateVanillaValuesFiles.
Maybe we find a middle ground and we can combine the two options.

Yes, thanks for the additional input!

Yeah, what I'm pointing is that in your case resolving happens twice. Maybe it is better to patch vanilla values and secrets fletcher separately. Not the base function.

Or, as a common solution better than repeating solution 3 times it makes sense to remove env vars loader and just use your patch everywhere.

in your case resolving happens twice

If we're going to support this remote file loading universally within helmlfile, it might make sense to patch resolveFile this way. Let's merge this and see how it goes.

I'm unsure if I'm aware of all the consequences of doing it though. I'd appreciate it if you could submit follow-up PRs to de-duplicate the remote file fetching steps if that makes sense 🙇

stale · 2022-05-11T11:17:23Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2022-05-30T17:08:31Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

The releases and environment section allow for values files on the local disk. This enhancement allows for referencing remote (go-getter) files to be fetched, cached and referenced. In addition when fetching a remote git source with a ssh key the ssh key will not be part of the caching folder name. This avoids two problems: 1. Don't leak sensitive information in the name of the caching folder 2. Base64 encoded SSH keys are very long. On some file systems the max lenght of the directory name is hit when using the full base64 information in the path name. The sshkey informations are reducted. Because of this fixed string there is a change of colloding cache names. The likelihood of this collision is very low. The git repo and git reference need to be the same, but the sshkey can change. This will result in the same source to be checkout out and referenced. Signed-off-by: Lüchinger Dominic <dev@snowgarden.ch>

pkg/state/storage.go

mumoshu

LGTM. Thanks for your contribution @dol ☺️

dol requested review from mumoshu and itscaro as code owners April 19, 2022 15:27

dol commented Apr 20, 2022

View reviewed changes

kuzaxak reviewed Apr 20, 2022

View reviewed changes

dol changed the title ~~Adds feature to fetch environment values from remote~~ Adds feature to fetch release values and secret values from remote Apr 25, 2022

stale bot added wontfix This will not be worked on and removed wontfix This will not be worked on labels May 11, 2022

roanosullivan mentioned this pull request May 24, 2022

Proposal: add option for remote values files located in other repos roboll/helmfile#469

Open

stale bot added the wontfix This will not be worked on label May 30, 2022

stale bot removed the wontfix This will not be worked on label May 31, 2022

mumoshu reviewed Jun 5, 2022

View reviewed changes

pkg/state/storage.go Outdated Show resolved Hide resolved

Update pkg/state/storage.go

6c1aa7c

mumoshu approved these changes Jun 5, 2022

View reviewed changes

mumoshu merged commit 789af92 into helmfile:main Jun 5, 2022

github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds feature to fetch release values and secret values from remote #47

Adds feature to fetch release values and secret values from remote #47

dol commented Apr 19, 2022 •

edited

dol Apr 20, 2022

kuzaxak Apr 20, 2022

dol Apr 25, 2022

kuzaxak Apr 27, 2022

mumoshu Jun 5, 2022 •

edited

stale bot commented May 11, 2022

stale bot commented May 30, 2022

mumoshu left a comment

		@@ -30,7 +31,17 @@ func NewStorage(forFile string, logger *zap.SugaredLogger, glob func(string) ([]
		func (st Storage) resolveFile(missingFileHandler string, tpe, path string) ([]string, bool, error) {

Adds feature to fetch release values and secret values from remote #47

Adds feature to fetch release values and secret values from remote #47

Conversation

dol commented Apr 19, 2022 • edited

dol Apr 20, 2022

Choose a reason for hiding this comment

kuzaxak Apr 20, 2022

Choose a reason for hiding this comment

dol Apr 25, 2022

Choose a reason for hiding this comment

kuzaxak Apr 27, 2022

Choose a reason for hiding this comment

mumoshu Jun 5, 2022 • edited

Choose a reason for hiding this comment

stale bot commented May 11, 2022

stale bot commented May 30, 2022

mumoshu left a comment

Choose a reason for hiding this comment

dol commented Apr 19, 2022 •

edited

mumoshu Jun 5, 2022 •

edited