Skip to content

Commit cfe0c0d

Browse files
fix: late evaluate helm version
This restores the behavior that you can execute skaffold build without needing helm.
1 parent 561ce51 commit cfe0c0d

File tree

2 files changed

+33
-18
lines changed

2 files changed

+33
-18
lines changed

pkg/skaffold/deploy/helm/helm_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1614,21 +1614,21 @@ func TestHelmRender(t *testing.T) {
16141614
description: "render with remote chart",
16151615
shouldErr: false,
16161616
commands: testutil.
1617-
CmdRunWithOutput("helm version", version31).AndRun("helm --kube-context kubecontext template skaffold-helm-remote stable/chartmuseum --repo https://charts.helm.sh/stable --kubeconfig kubeconfig"),
1617+
CmdRun("helm --kube-context kubecontext template skaffold-helm-remote stable/chartmuseum --repo https://charts.helm.sh/stable --kubeconfig kubeconfig"),
16181618
helm: testDeployRemoteChart,
16191619
},
16201620
{
16211621
description: "render with remote chart with version",
16221622
shouldErr: false,
16231623
commands: testutil.
1624-
CmdRunWithOutput("helm version", version31).AndRun("helm --kube-context kubecontext template skaffold-helm-remote stable/chartmuseum --version 1.0.0 --repo https://charts.helm.sh/stable --kubeconfig kubeconfig"),
1624+
CmdRun("helm --kube-context kubecontext template skaffold-helm-remote stable/chartmuseum --version 1.0.0 --repo https://charts.helm.sh/stable --kubeconfig kubeconfig"),
16251625
helm: testDeployRemoteChartVersion,
16261626
},
16271627
{
16281628
description: "render without building chart dependencies",
16291629
shouldErr: false,
16301630
commands: testutil.
1631-
CmdRunWithOutput("helm version", version31).AndRun("helm --kube-context kubecontext template skaffold-helm stable/chartmuseum --kubeconfig kubeconfig"),
1631+
CmdRun("helm --kube-context kubecontext template skaffold-helm stable/chartmuseum --kubeconfig kubeconfig"),
16321632
helm: testDeploySkipBuildDependencies,
16331633
},
16341634
// https://github.com/GoogleContainerTools/skaffold/issues/7595
@@ -1673,7 +1673,7 @@ func TestHelmRender(t *testing.T) {
16731673
description: "render with useHelmSecrets",
16741674
shouldErr: false,
16751675
commands: testutil.
1676-
CmdRunWithOutput("helm version", version31).AndRun("helm secrets --kube-context kubecontext template skaffold-helm examples/test --set some.key=somevalue --kubeconfig kubeconfig"),
1676+
CmdRun("helm secrets --kube-context kubecontext template skaffold-helm examples/test --set some.key=somevalue --kubeconfig kubeconfig"),
16771677
helm: testDeployChartWithUseHelmSecrets,
16781678
},
16791679
}

pkg/skaffold/render/renderer/helm/helm.go

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"io"
2525
"os"
2626
"strings"
27+
"sync"
2728

2829
"github.com/blang/semver"
2930
"gopkg.in/yaml.v3"
@@ -61,7 +62,7 @@ type Helm struct {
6162
labels map[string]string
6263
enableDebug bool
6364
overrideProtocols []string
64-
helmVersion semver.Version
65+
helmVersion func(ctx context.Context) (semver.Version, error)
6566

6667
manifestOverrides map[string]string
6768
transformAllowlist map[apimachinery.GroupKind]latest.ResourceFilter
@@ -81,22 +82,15 @@ func (h Helm) ManifestOverrides() map[string]string {
8182
}
8283

8384
func New(ctx context.Context, cfg render.Config, rCfg latest.RenderConfig, labels map[string]string, configName string, manifestOverrides map[string]string) (Helm, error) {
84-
var helmVersion semver.Version
85-
var err error
86-
if RendererHelmVersionOverride != nil {
87-
helmVersion = *RendererHelmVersionOverride
88-
} else {
89-
helmVersion, err = helm.BinVer(ctx)
90-
if err != nil {
91-
return Helm{}, helm.VersionGetErr(err)
92-
}
93-
}
94-
9585
generator := generate.NewGenerator(cfg.GetWorkingDir(), rCfg.Generate, "")
9686
transformAllowlist, transformDenylist, err := util.ConsolidateTransformConfiguration(cfg)
9787
if err != nil {
9888
return Helm{}, err
9989
}
90+
91+
helmVersionStored := semver.Version{}
92+
helmVersionMutex := sync.Mutex{}
93+
10094
return Helm{
10195
configName: configName,
10296
Generator: generator,
@@ -110,8 +104,25 @@ func New(ctx context.Context, cfg render.Config, rCfg latest.RenderConfig, label
110104
labels: labels,
111105
namespace: cfg.GetKubeNamespace(),
112106
manifestOverrides: manifestOverrides,
113-
helmVersion: helmVersion,
114107

108+
helmVersion: func(ctx context.Context) (semver.Version, error) {
109+
if RendererHelmVersionOverride != nil {
110+
return *RendererHelmVersionOverride, nil
111+
}
112+
113+
helmVersionMutex.Lock()
114+
defer helmVersionMutex.Unlock()
115+
116+
if !helmVersionStored.Equals(semver.Version{}) {
117+
return helmVersionStored, nil
118+
}
119+
helmVersion, err := helm.BinVer(ctx)
120+
if err != nil {
121+
return semver.Version{}, helm.VersionGetErr(err)
122+
}
123+
helmVersionStored = helmVersion
124+
return helmVersionStored, nil
125+
},
115126
transformAllowlist: transformAllowlist,
116127
transformDenylist: transformDenylist,
117128
}, nil
@@ -147,7 +158,11 @@ func (h Helm) generateHelmManifests(ctx context.Context, builds []graph.Artifact
147158
helmEnv = append(helmEnv, filterEnv...)
148159

149160
var cleanUpPostRenderer func()
150-
cleanUpPostRenderer, postRendererArgs, err = helm.PreparePostRenderer(ctx, h, skaffoldBinary, h.helmVersion)
161+
helmVersion, err := h.helmVersion(ctx)
162+
if err != nil {
163+
return nil, fmt.Errorf("could not get helm version: %w", err)
164+
}
165+
cleanUpPostRenderer, postRendererArgs, err = helm.PreparePostRenderer(ctx, h, skaffoldBinary, helmVersion)
151166
if err != nil {
152167
return nil, err
153168
}

0 commit comments

Comments
 (0)