diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index 4250253b523..5248864474f 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -277,23 +277,10 @@ func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r lates for k, v := range params { var value string - if cfg := r.ImageStrategy.HelmImageConfig.HelmConventionConfig; cfg != nil { - dockerRef, err := docker.ParseReference(v.Tag) - if err != nil { - return nil, errors.Wrapf(err, "cannot parse the image reference %s", v.Tag) - } - - if cfg.ExplicitRegistry { - if dockerRef.Domain == "" { - return nil, errors.Wrapf(err, "image reference %s has no domain", v.Tag) - } - - value = fmt.Sprintf("%[1]s.registry=%s,%[1]s.repository=%s,%[1]s.tag=%s", k, dockerRef.Domain, dockerRef.Path, v.Tag) - } else { - value = fmt.Sprintf("%[1]s.repository=%s,%[1]s.tag=%s", k, dockerRef.BaseName, v.Tag) - } - } else { - value = fmt.Sprintf("%s=%s", k, v.Tag) + cfg := r.ImageStrategy.HelmImageConfig.HelmConventionConfig + value, err = getImageSetValueFromHelmStrategy(cfg, k, v.Tag) + if err != nil { + return nil, err } valuesSet[v.Tag] = true @@ -417,6 +404,34 @@ func (h *HelmDeployer) getReleaseInfo(ctx context.Context, release string) (*buf return bufio.NewReader(&releaseInfo), nil } +func getImageSetValueFromHelmStrategy(cfg *latest.HelmConventionConfig, valueName string, tag string) (string, error) { + if cfg != nil { + dockerRef, err := docker.ParseReference(tag) + if err != nil { + return "", errors.Wrapf(err, "cannot parse the image reference %s", tag) + } + + if cfg.ExplicitRegistry { + if dockerRef.Domain == "" { + return "", errors.New(fmt.Sprintf("image reference %s has no domain", tag)) + } + return fmt.Sprintf( + "%[1]s.registry=%[2]s,%[1]s.repository=%[3]s,%[1]s.tag=%[4]s", + valueName, + dockerRef.Domain, + dockerRef.Path, + dockerRef.Tag, + ), nil + } + return fmt.Sprintf( + "%[1]s.repository=%[2]s,%[1]s.tag=%[3]s", + valueName, dockerRef.BaseName, + dockerRef.Tag, + ), nil + } + return fmt.Sprintf("%s=%s", valueName, tag), nil +} + // Retrieve info about all releases using helm get // Skaffold labels will be applied to each deployed k8s object // Since helm isn't always consistent with retrieving results, don't return errors here diff --git a/pkg/skaffold/deploy/helm_test.go b/pkg/skaffold/deploy/helm_test.go index b28737d2da3..ca94a7d4baa 100644 --- a/pkg/skaffold/deploy/helm_test.go +++ b/pkg/skaffold/deploy/helm_test.go @@ -541,7 +541,7 @@ func (m *MockHelm) RunCmd(c *exec.Cmd) error { case "get": return m.getResult case "install": - if m.upgradeMatcher != nil && !m.installMatcher(c) { + if m.installMatcher != nil && !m.installMatcher(c) { m.t.Errorf("install matcher failed to match commands: %+v", c.Args) } return m.installResult @@ -674,6 +674,69 @@ func TestHelmDependencies(t *testing.T) { } } +func TestGetImageSetValueFromHelmStrategy(t *testing.T) { + tests := []struct { + description string + valueName string + tag string + expected string + strategy *latest.HelmConventionConfig + shouldErr bool + }{ + { + description: "Helm set values with no convention config", + valueName: "image", + tag: "skaffold-helm:1.0.0", + expected: "image=skaffold-helm:1.0.0", + strategy: nil, + shouldErr: false, + }, + { + description: "Helm set values with helm conventions", + valueName: "image", + tag: "skaffold-helm:1.0.0", + expected: "image.repository=skaffold-helm,image.tag=1.0.0", + strategy: &latest.HelmConventionConfig{}, + shouldErr: false, + }, + { + description: "Helm set values with helm conventions and explicit registry value", + valueName: "image", + tag: "docker.io/skaffold-helm:1.0.0", + expected: "image.registry=docker.io,image.repository=skaffold-helm,image.tag=1.0.0", + strategy: &latest.HelmConventionConfig{ + ExplicitRegistry: true, + }, + shouldErr: false, + }, + { + description: "Invalid tag with helm conventions", + valueName: "image", + tag: "skaffold-helm:1.0.0,0", + expected: "", + strategy: &latest.HelmConventionConfig{}, + shouldErr: true, + }, + { + description: "Helm set values with helm conventions and explicit registry value, but missing in tag", + valueName: "image", + tag: "skaffold-helm:1.0.0", + expected: "", + strategy: &latest.HelmConventionConfig{ + ExplicitRegistry: true, + }, + shouldErr: true, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + values, err := getImageSetValueFromHelmStrategy(test.strategy, test.valueName, test.tag) + t.CheckError(test.shouldErr, err) + t.CheckDeepEqual(test.expected, values) + }) + } +} + func TestExpandPaths(t *testing.T) { homedir.DisableCache = true // for testing only