Skip to content

Commit

Permalink
fix: helmDefaults.kubeContext ignored in helm diff of `helmfile app…
Browse files Browse the repository at this point in the history
…ly` (#682)

The root cause of this bug was due to that `--kube-context` and `kubeContext` had been treated specifically in code. So on the way I have made it consistent with other per-release settings - by adding `kubeContext` for each release and treating `helmDefaults.kubeContext` as just the default value for per-release setting.

Fixes #674
  • Loading branch information
mumoshu authored Jun 12, 2019
1 parent 1da3488 commit 2e38f42
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 19 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ releases:
tlsCert: "path/to/cert.pem"
# path to TLS key file (default "$HELM_HOME/key.pem")
tlsKey: "path/to/key.pem"
# --kube-context to be passed to helm commands
# CAUTION: this doesn't work as expected for `tilerless: true`.
# See https://github.com/roboll/helmfile/issues/642
kubeContext: kube-context

# Local chart example
- name: grafana # name of this release
Expand Down
4 changes: 0 additions & 4 deletions pkg/argparser/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,6 @@ func GetArgs(args string, state *state.HelmState) []string {
}
}

if state.HelmDefaults.KubeContext != "" {
argsMap.SetArg("--kube-context", state.HelmDefaults.KubeContext, false)
}

var argArr []string

for _, flag := range argsMap.flags {
Expand Down
4 changes: 2 additions & 2 deletions pkg/argparser/args_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func TestGetArgs(t *testing.T) {
testState := &state.HelmState{HelmDefaults: Helmdefaults}
receivedArgs := GetArgs(args, testState)

expectedOutput := "--timeout=3600 --set app1.bootstrap=true --set app2.bootstrap=false --tiller-namespace ns --recreate-pods --force --kube-context=test"
expectedOutput := "--timeout=3600 --set app1.bootstrap=true --set app2.bootstrap=false --tiller-namespace ns --recreate-pods --force"

if compareArgs(expectedOutput, receivedArgs) == false {
t.Errorf("expected %s, got %s", expectedOutput, strings.Join(receivedArgs, " "))
Expand All @@ -27,7 +27,7 @@ func Test2(t *testing.T) {
testState := &state.HelmState{HelmDefaults: Helmdefaults}
receivedArgs := GetArgs(args, testState)

expectedOutput := "--timeout=3600 --set app1.bootstrap=true --set app2.bootstrap=false,app3.bootstrap=true --tiller-namespace ns --recreate-pods --force --kube-context=test"
expectedOutput := "--timeout=3600 --set app1.bootstrap=true --set app2.bootstrap=false,app3.bootstrap=true --tiller-namespace ns --recreate-pods --force"

if compareArgs(expectedOutput, receivedArgs) == false {
t.Errorf("expected %s, got %s", expectedOutput, strings.Join(receivedArgs, " "))
Expand Down
2 changes: 1 addition & 1 deletion pkg/state/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func (st *HelmState) loadEnvValues(name string, ctxEnv *environment.Environment,
// installed on the cluster, just for decrypting secrets!
// Related: https://github.com/futuresimple/helm-secrets/issues/83
release := &ReleaseSpec{}
flags := st.appendTillerFlags([]string{}, release)
flags := st.appendConnectionFlags([]string{}, release)
decFile, err := helm.DecryptSecret(st.createHelmContext(release, 0), path, flags...)
if err != nil {
return nil, err
Expand Down
31 changes: 20 additions & 11 deletions pkg/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ type ReleaseSpec struct {
TillerNamespace string `yaml:"tillerNamespace"`
Tillerless *bool `yaml:"tillerless"`

KubeContext string `yaml:"kubeContext"`

TLS *bool `yaml:"tls"`
TLSCACert string `yaml:"tlsCACert"`
TLSKey string `yaml:"tlsKey"`
Expand Down Expand Up @@ -318,7 +320,7 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu
}

func (st *HelmState) isReleaseInstalled(context helmexec.HelmContext, helm helmexec.Interface, release ReleaseSpec) (bool, error) {
out, err := helm.List(context, "^"+release.Name+"$", st.tillerFlags(&release)...)
out, err := helm.List(context, "^"+release.Name+"$", st.connectionFlags(&release)...)
if err != nil {
return false, err
} else if out != "" {
Expand Down Expand Up @@ -438,7 +440,7 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme

func (st *HelmState) getDeployedVersion(context helmexec.HelmContext, helm helmexec.Interface, release *ReleaseSpec) (string, error) {
//retrieve the version
if out, err := helm.List(context, "^"+release.Name+"$", st.tillerFlags(release)...); err == nil {
if out, err := helm.List(context, "^"+release.Name+"$", st.connectionFlags(release)...); err == nil {
chartName := filepath.Base(release.Chart)
//the regexp without escapes : .*\s.*\s.*\s.*\schartName-(.*?)\s
pat := regexp.MustCompile(".*\\s.*\\s.*\\s.*\\s" + chartName + "-(.*?)\\s")
Expand Down Expand Up @@ -835,7 +837,7 @@ func (st *HelmState) ReleaseStatuses(helm helmexec.Interface, workerLimit int) [
}

flags := []string{}
flags = st.appendTillerFlags(flags, &release)
flags = st.appendConnectionFlags(flags, &release)

return helm.ReleaseStatus(st.createHelmContext(&release, workerIndex), release.Name, flags...)
})
Expand All @@ -852,7 +854,7 @@ func (st *HelmState) DeleteReleases(affectedReleases *AffectedReleases, helm hel
if purge {
flags = append(flags, "--purge")
}
flags = st.appendTillerFlags(flags, &release)
flags = st.appendConnectionFlags(flags, &release)
context := st.createHelmContext(&release, workerIndex)

installed, err := st.isReleaseInstalled(context, helm, release)
Expand Down Expand Up @@ -884,7 +886,7 @@ func (st *HelmState) TestReleases(helm helmexec.Interface, cleanup bool, timeout
flags = append(flags, "--cleanup")
}
flags = append(flags, "--timeout", strconv.Itoa(timeout))
flags = st.appendTillerFlags(flags, &release)
flags = st.appendConnectionFlags(flags, &release)

return helm.TestRelease(st.createHelmContext(&release, workerIndex), release.Name, flags...)
})
Expand Down Expand Up @@ -1116,15 +1118,16 @@ func findChartDirectory(topLevelDir string) (string, error) {
return topLevelDir, errors.New("No Chart.yaml found")
}

func (st *HelmState) appendTillerFlags(flags []string, release *ReleaseSpec) []string {
adds := st.tillerFlags(release)
// appendConnectionFlags append all the helm command-line flags related to K8s API and Tiller connection including the kubecontext
func (st *HelmState) appendConnectionFlags(flags []string, release *ReleaseSpec) []string {
adds := st.connectionFlags(release)
for _, a := range adds {
flags = append(flags, a)
}
return flags
}

func (st *HelmState) tillerFlags(release *ReleaseSpec) []string {
func (st *HelmState) connectionFlags(release *ReleaseSpec) []string {
flags := []string{}
tillerless := st.HelmDefaults.Tillerless
if release.Tillerless != nil {
Expand Down Expand Up @@ -1158,6 +1161,12 @@ func (st *HelmState) tillerFlags(release *ReleaseSpec) []string {
} else if st.HelmDefaults.TLSCACert != "" {
flags = append(flags, "--tls-ca-cert", st.HelmDefaults.TLSCACert)
}

if release.KubeContext != "" {
flags = append(flags, "--kube-context", release.KubeContext)
} else if st.HelmDefaults.KubeContext != "" {
flags = append(flags, "--kube-context", st.HelmDefaults.KubeContext)
}
}

return flags
Expand Down Expand Up @@ -1201,7 +1210,7 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
flags = append(flags, "--atomic")
}

flags = st.appendTillerFlags(flags, release)
flags = st.appendConnectionFlags(flags, release)

var err error
flags, err = st.appendHelmXFlags(flags, release)
Expand Down Expand Up @@ -1244,7 +1253,7 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec,
flags = append(flags, "--devel")
}

flags = st.appendTillerFlags(flags, release)
flags = st.appendConnectionFlags(flags, release)

var err error
flags, err = st.appendHelmXFlags(flags, release)
Expand Down Expand Up @@ -1417,7 +1426,7 @@ func (st *HelmState) namespaceAndValuesFlags(helm helmexec.Interface, release *R
}
path := paths[0]

decryptFlags := st.appendTillerFlags([]string{}, release)
decryptFlags := st.appendConnectionFlags([]string{}, release)
valfile, err := helm.DecryptSecret(st.createHelmContext(release, workerIndex), path, decryptFlags...)
if err != nil {
return nil, err
Expand Down
39 changes: 38 additions & 1 deletion pkg/state/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1917,6 +1917,8 @@ func TestHelmState_Delete(t *testing.T) {
purge bool
flags string
tillerNamespace string
kubeContext string
defKubeContext string
}{
{
name: "desired and installed (purge=false)",
Expand Down Expand Up @@ -2008,6 +2010,37 @@ func TestHelmState_Delete(t *testing.T) {
flags: "--tiller-namespacetillerns",
deleted: []mockRelease{{"releaseA", []string{"--purge", "--tiller-namespace", "tillerns"}}},
},
{
name: "with kubecontext",
wantErr: false,
desired: nil,
installed: true,
purge: true,
kubeContext: "ctx",
flags: "--kube-contextctx",
deleted: []mockRelease{{"releaseA", []string{"--purge", "--kube-context", "ctx"}}},
},
{
name: "with default kubecontext",
wantErr: false,
desired: nil,
installed: true,
purge: true,
defKubeContext: "defctx",
flags: "--kube-contextdefctx",
deleted: []mockRelease{{"releaseA", []string{"--purge", "--kube-context", "defctx"}}},
},
{
name: "with non-default and default kubecontexts",
wantErr: false,
desired: nil,
installed: true,
purge: true,
kubeContext: "ctx",
defKubeContext: "defctx",
flags: "--kube-contextctx",
deleted: []mockRelease{{"releaseA", []string{"--purge", "--kube-context", "ctx"}}},
},
}
for i := range tests {
tt := tests[i]
Expand All @@ -2020,11 +2053,15 @@ func TestHelmState_Delete(t *testing.T) {
Name: name,
Installed: tt.desired,
TillerNamespace: tt.tillerNamespace,
KubeContext: tt.kubeContext,
}
releases := []ReleaseSpec{
release,
}
state := &HelmState{
HelmDefaults: HelmSpec{
KubeContext: tt.defKubeContext,
},
Releases: releases,
logger: logger,
}
Expand All @@ -2043,7 +2080,7 @@ func TestHelmState_Delete(t *testing.T) {
return
}
} else if !(reflect.DeepEqual(tt.deleted, helm.deleted) && (len(affectedReleases.Deleted) == len(tt.deleted))) {
t.Errorf("unexpected deletions happened: expected %v, got %v", &affectedReleases.Deleted, tt.deleted)
t.Errorf("unexpected deletions happened: expected %v, got %v", tt.deleted, &affectedReleases.Deleted)
}
}
t.Run(tt.name, f)
Expand Down

0 comments on commit 2e38f42

Please sign in to comment.