Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add buildDependenciesSkipRefresh option to HelmRelease to pass --skip-refresh to helm dep bulid #9672

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
7 changes: 7 additions & 0 deletions docs-v2/content/en/schemas/v4beta12.json
Original file line number Diff line number Diff line change
Expand Up @@ -2442,6 +2442,12 @@
"x-intellij-html-description": "should build dependencies be skipped. Ignored for <code>remoteChart</code>.",
"default": "false"
},
"skipBuildDependenciesRefresh": {
"type": "boolean",
"description": "determines whether the refresh of already built dependencies should be skipped. If set to `true`, passes `--skip-refresh` flag to `helm dep build` command. Ignored when `skipBuildDependencies` is `false`.",
"x-intellij-html-description": "determines whether the refresh of already built dependencies should be skipped. If set to <code>true</code>, passes <code>--skip-refresh</code> flag to <code>helm dep build</code> command. Ignored when <code>skipBuildDependencies</code> is <code>false</code>.",
"default": "false"
},
"skipTests": {
"type": "boolean",
"description": "should ignore helm test during manifests generation.",
Expand Down Expand Up @@ -2494,6 +2500,7 @@
"wait",
"recreatePods",
"skipBuildDependencies",
"skipBuildDependenciesRefresh",
"skipTests",
"useHelmSecrets",
"repo",
Expand Down
13 changes: 11 additions & 2 deletions pkg/skaffold/render/renderer/helm/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ func (h Helm) generateHelmManifests(ctx context.Context, builds []graph.Artifact
return manifests, nil
}

func buildDepBuildArgs(skipBuildDependenciesRefresh bool) []string {
args := []string{"dep", "build"}
if skipBuildDependenciesRefresh {
args = append(args, "--skip-refresh")
}
return args
}

func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact, release latest.HelmRelease, env, additionalArgs []string) ([]byte, error) {
releaseName, err := sUtil.ExpandEnvTemplateOrFail(release.Name, nil)
if err != nil {
Expand Down Expand Up @@ -171,7 +179,7 @@ func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact,
return nil, helm.UserErr("cannot marshal overrides to create overrides values.yaml", err)
}

if err := os.WriteFile(constants.HelmOverridesFilename, overrides, 0666); err != nil {
if err := os.WriteFile(constants.HelmOverridesFilename, overrides, 0o666); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

just wondering why you changed this?

Copy link
Author

Choose a reason for hiding this comment

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

it's octal permissions, added by gofumpt automatically. I didn't intend this change as this is not the accepted tool in the repo. Reverted.

return nil, helm.UserErr(fmt.Sprintf("cannot create file %q", constants.HelmOverridesFilename), err)
}

Expand Down Expand Up @@ -208,7 +216,8 @@ func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact,
// Build Chart dependencies, but allow a user to skip it.
if !release.SkipBuildDependencies && release.ChartPath != "" {
log.Entry(ctx).Info("Building helm dependencies...")
if err := helm.ExecWithStdoutAndStderr(ctx, h, io.Discard, errBuffer, false, env, "dep", "build", release.ChartPath); err != nil {
cmdArgs := buildDepBuildArgs(release.SkipBuildDependenciesRefresh)
if err := helm.ExecWithStdoutAndStderr(ctx, h, io.Discard, errBuffer, false, env, cmdArgs...); err != nil {
log.Entry(ctx).Info(errBuffer.String())
return nil, helm.UserErr("building helm dependencies", err)
}
Expand Down
49 changes: 49 additions & 0 deletions pkg/skaffold/render/renderer/helm/helm_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
Copyright 2025 The Skaffold Authors

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package helm

import (
"testing"

"github.com/GoogleContainerTools/skaffold/v2/testutil"
)

func TestBuildDepBuildArgs(t *testing.T) {
tests := []struct {
description string
skipBuildDependenciesRefresh bool
expected []string
}{
{
description: "build args without skipBuildDependenciesRefresh",
skipBuildDependenciesRefresh: false,
expected: []string{"dep", "build"},
},
{
description: "build args with skipBuildDependenciesRefresh",
skipBuildDependenciesRefresh: true,
expected: []string{"dep", "build", "--skip-refresh"},
},
}

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
args := buildDepBuildArgs(test.skipBuildDependenciesRefresh)
t.CheckDeepEqual(test.expected, args)
})
}
}
7 changes: 5 additions & 2 deletions pkg/skaffold/schema/latest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1050,6 +1050,11 @@ type HelmRelease struct {
// Ignored for `remoteChart`.
SkipBuildDependencies bool `yaml:"skipBuildDependencies,omitempty"`

// SkipBuildDependenciesRefresh determines whether the refresh of already built dependencies should be skipped.
// If set to `true`, passes `--skip-refresh` flag to `helm dep build` command.
// Ignored when `skipBuildDependencies` is `false`.
SkipBuildDependenciesRefresh bool `yaml:"skipBuildDependenciesRefresh,omitempty"`

// SkipTests should ignore helm test during manifests generation.
// Defaults to `false`
SkipTests bool `yaml:"skipTests,omitempty"`
Expand Down Expand Up @@ -1856,7 +1861,6 @@ func (clusterDetails *ClusterDetails) UnmarshalYAML(value *yaml.Node) error {
// Unmarshal the remaining values
aux := (*ClusterDetailsForUnmarshaling)(clusterDetails)
err = yaml.Unmarshal(remaining, aux)

if err != nil {
return err
}
Expand All @@ -1883,7 +1887,6 @@ func (ka *KanikoArtifact) UnmarshalYAML(value *yaml.Node) error {
// Unmarshal the remaining values
aux := (*KanikoArtifactForUnmarshaling)(ka)
err = yaml.Unmarshal(remaining, aux)

if err != nil {
return err
}
Expand Down
Loading