diff --git a/integration/fix_test.go b/integration/fix_test.go index 6b49ba9efde..43d05b8c7cd 100644 --- a/integration/fix_test.go +++ b/integration/fix_test.go @@ -51,6 +51,14 @@ func TestFixStdout(t *testing.T) { skaffold.Run().WithConfig("-").InDir("testdata/fix").InNs(ns.Name).WithStdin(out).RunOrFail(t) } +func TestFixV2Beta29ToV3Alpha1PatchProfiles(t *testing.T) { + MarkIntegrationTest(t, CanRunWithoutGcp) + + out, err := skaffold.Fix().InDir("testdata/fix-v2beta29").RunWithCombinedOutput(t) + testutil.CheckError(t, false, err) + testutil.CheckContains(t, "/manifests/helm/releases/0/setValueTemplates/", string(out)) +} + func TestFixOutputFile(t *testing.T) { // TODO: Fix https://github.com/GoogleContainerTools/skaffold/issues/7033. t.Skipf("Fix https://github.com/GoogleContainerTools/skaffold/issues/7033") diff --git a/integration/testdata/fix-v2beta29/skaffold.yaml b/integration/testdata/fix-v2beta29/skaffold.yaml new file mode 100644 index 00000000000..4d54d946f48 --- /dev/null +++ b/integration/testdata/fix-v2beta29/skaffold.yaml @@ -0,0 +1,24 @@ +apiVersion: skaffold/v2beta29 +kind: Config +build: + artifacts: + - image: skaffold-helm +deploy: + helm: + releases: + - name: skaffold-helm + chartPath: charts + namespace: repro-ns + recreatePods: false + useHelmSecrets: false + wait: false + artifactOverrides: + image: skaffold-helm + imageStrategy: + helm: {} +profiles: + - name: skaffold-helm-v2 + patches: + - op: replace + path: /deploy/helm/releases/0/artifactOverrides/image + value: skaffold-helm-v2 \ No newline at end of file diff --git a/pkg/skaffold/schema/v2beta29/upgrade.go b/pkg/skaffold/schema/v2beta29/upgrade.go index 1bda6f15b0d..84376881648 100644 --- a/pkg/skaffold/schema/v2beta29/upgrade.go +++ b/pkg/skaffold/schema/v2beta29/upgrade.go @@ -20,9 +20,12 @@ import ( "context" "encoding/json" "fmt" + "regexp" + "strconv" "strings" "github.com/pkg/errors" + "gopkg.in/yaml.v2" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output/log" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/util" @@ -30,6 +33,8 @@ import ( pkgutil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util" ) +var artifactOverridesRegexp = regexp.MustCompile("/deploy/helm/releases/[0-9]+/artifactOverrides/image") + var migrations = map[string]string{ "/deploy/kubectl": "/manifests/rawYaml", "/deploy/kustomize/paths": "/manifests/kustomize/paths", @@ -45,28 +50,57 @@ func (c *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) { pkgutil.CloneThroughJSON(c, &newConfig) newConfig.APIVersion = next.Version - err := util.UpgradePipelines(c, &newConfig, upgradeOnePipeline) + var nProfiles []next.Profile + // seed with existing Profiles + if c.Profiles != nil { + pkgutil.CloneThroughJSON(newConfig.Profiles, &nProfiles) + } + + oldPatchesMap := map[string][]JSONPatch{} + newPatchesMap := map[string][]next.JSONPatch{} + for i, p := range c.Profiles { + oldPatchesMap[p.Name], newPatchesMap[p.Name] = p.Patches, nProfiles[i].Patches + } + onePipelineUpgrader := NewOnePipelineUpgrader(oldPatchesMap, newPatchesMap) + + err := util.UpgradePipelines(c, &newConfig, onePipelineUpgrader.upgradeOnePipeline) if err != nil { return &newConfig, err } - var newProfiles []next.Profile // seed with existing Profiles if c.Profiles != nil { pkgutil.CloneThroughJSON(newConfig.Profiles, &newProfiles) } + for i := range newProfiles { + if _, ok := newPatchesMap[newProfiles[i].Name]; ok { + newProfiles[i].Patches = newPatchesMap[newProfiles[i].Name] + } + } + // Update profiles patches for i, p := range c.Profiles { upgradePatches(p.Patches, newProfiles[i].Patches) } newConfig.Profiles = newProfiles - return &newConfig, nil } -func upgradeOnePipeline(oldPipeline, newPipeline interface{}) error { +type OnePipelineUpgrader struct { + oldPatchesMap map[string][]JSONPatch + newPatchesMap map[string][]next.JSONPatch +} + +func NewOnePipelineUpgrader(oldPatchesMap map[string][]JSONPatch, newPatchesMap map[string][]next.JSONPatch) OnePipelineUpgrader { + return OnePipelineUpgrader{ + oldPatchesMap: oldPatchesMap, + newPatchesMap: newPatchesMap, + } +} + +func (opu *OnePipelineUpgrader) upgradeOnePipeline(oldPipeline, newPipeline interface{}) error { oldPL := oldPipeline.(*Pipeline) newPL := newPipeline.(*next.Pipeline) @@ -151,12 +185,112 @@ func upgradeOnePipeline(oldPipeline, newPipeline interface{}) error { newPL.Deploy.LegacyHelmDeploy = &next.LegacyHelmDeploy{} newPL.Deploy.LegacyHelmDeploy.LifecycleHooks = newHelm.LifecycleHooks } + + err := upgradeArtifactOverridesPatches(opu.oldPatchesMap, opu.newPatchesMap, oldPL) + if err != nil { + return fmt.Errorf("error converting helm artifactOverrides during version upgrade: %w", err) + } + + return nil +} + +func upgradeArtifactOverridesPatches(oldsMap map[string][]JSONPatch, newsMap map[string][]next.JSONPatch, oldPL *Pipeline) error { + for k := range oldsMap { + for i, old := range oldsMap[k] { + if artifactOverridesRegexp.Match([]byte(old.Path)) { + idx, err := strconv.Atoi(strings.Split(old.Path, "/")[4]) + if err != nil { + return err + } + newPathModified := strings.ReplaceAll(strings.ReplaceAll(old.Path, "/deploy/helm", "/manifests/helm"), + "artifactOverrides", "setValueTemplates") + if oldPL.Deploy.HelmDeploy != nil { + // replace commonly used image name chars that are illegal helm template chars "/" & "-" with "_" + validV := pkgutil.SanitizeHelmTemplateValue(old.Value.Node.Value().(string)) + n := &util.YamlpatchNode{} + + if oldPL.Deploy.HelmDeploy.Releases[idx].ImageStrategy.HelmConventionConfig != nil { + err = yaml.Unmarshal([]byte(fmt.Sprintf("\"{{.IMAGE_TAG_%s}}\"", validV)), &n) + if err != nil { + return err + } + + newsMap[k] = append(newsMap[k], next.JSONPatch{ + Op: newsMap[k][i].Op, + Path: newPathModified + ".tag", + From: newsMap[k][i].From, + Value: n, + }) + + if oldPL.Deploy.HelmDeploy.Releases[i].ImageStrategy.HelmConventionConfig.ExplicitRegistry { + // is 'helm' imageStrategy + explicitRegistry + n := &util.YamlpatchNode{} + err = yaml.Unmarshal([]byte(fmt.Sprintf("\"{{.IMAGE_DOMAIN_%s}}\"", validV)), &n) + if err != nil { + return err + } + + newsMap[k] = append(newsMap[k], next.JSONPatch{ + Op: newsMap[k][i].Op, + Path: newPathModified + ".registry", + From: newsMap[k][i].From, + Value: n, + }) + + n = &util.YamlpatchNode{} + err := yaml.Unmarshal([]byte(fmt.Sprintf("\"{{.IMAGE_REPO_NO_DOMAIN_%s}}\"", validV)), &n) + if err != nil { + return err + } + + newsMap[k][i] = next.JSONPatch{ + Op: newsMap[k][i].Op, + Path: newPathModified + ".repository", + From: newsMap[k][i].From, + Value: n, + } + } else { + // is 'helm' imageStrategy + n = &util.YamlpatchNode{} + err := yaml.Unmarshal([]byte(fmt.Sprintf("\"{{.IMAGE_REPO_%s}}\"", validV)), &n) + if err != nil { + return err + } + + newsMap[k][i] = next.JSONPatch{ + Op: newsMap[k][i].Op, + Path: newPathModified + ".repository", + From: newsMap[k][i].From, + Value: n, + } + } + } else { + // is 'fqn' imageStrategy + // replace commonly used image name chars that are illegal helm template chars "/" & "-" with "_" + newValue := fmt.Sprintf("\"{{.IMAGE_FULLY_QUALIFIED_%s}}\"", validV) + + n := &util.YamlpatchNode{} + err := yaml.Unmarshal([]byte(newValue), &n) + if err != nil { + return err + } + newsMap[k][i].Value = n + newsMap[k][i].Path = newPathModified + } + } + } + } + } return nil } func upgradePatches(olds []JSONPatch, news []next.JSONPatch) { for i, old := range olds { for str, repStr := range migrations { + if artifactOverridesRegexp.Match([]byte(old.Path)) { + // this is handled by upgradeArtifactOverridesPatches + continue + } if strings.Contains(old.Path, str) { news[i].Path = strings.ReplaceAll(old.Path, str, repStr) }