From a8ed05be9699ced39ae16c0f3c8ef9e861dee7cd Mon Sep 17 00:00:00 2001 From: fabiankramm Date: Wed, 4 Mar 2020 14:32:09 +0100 Subject: [PATCH] fix issue with wrongly cached image tags (#998) --- pkg/devspace/deploy/deployer/helm/deploy.go | 53 +---------- .../deploy/deployer/helm/deploy_test.go | 76 ---------------- .../deploy/deployer/kubectl/kubectl.go | 62 +------------ pkg/devspace/deploy/deployer/util/replace.go | 75 ++++++++++++++++ .../deploy/deployer/util/replace_test.go | 89 +++++++++++++++++++ 5 files changed, 168 insertions(+), 187 deletions(-) create mode 100644 pkg/devspace/deploy/deployer/util/replace.go create mode 100644 pkg/devspace/deploy/deployer/util/replace_test.go diff --git a/pkg/devspace/deploy/deployer/helm/deploy.go b/pkg/devspace/deploy/deployer/helm/deploy.go index bd28b1e23c..44005ee454 100644 --- a/pkg/devspace/deploy/deployer/helm/deploy.go +++ b/pkg/devspace/deploy/deployer/helm/deploy.go @@ -9,11 +9,9 @@ import ( yaml "gopkg.in/yaml.v2" "github.com/devspace-cloud/devspace/pkg/devspace/config/generated" - "github.com/devspace-cloud/devspace/pkg/devspace/config/versions/latest" "github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/helm/merge" - "github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/kubectl/walk" + "github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/util" "github.com/devspace-cloud/devspace/pkg/devspace/helm" - "github.com/devspace-cloud/devspace/pkg/devspace/registry" hashpkg "github.com/devspace-cloud/devspace/pkg/util/hash" "github.com/devspace-cloud/devspace/pkg/util/yamlutil" "github.com/mgutz/ansi" @@ -151,7 +149,7 @@ func (d *DeployConfig) internalDeploy(cache *generated.CacheConfig, forceDeploy // Add devspace specific values if d.DeploymentConfig.Helm.ReplaceImageTags == nil || *d.DeploymentConfig.Helm.ReplaceImageTags == true { // Replace image names - shouldRedeploy := replaceContainerNames(overwriteValues, cache, d.config.Images, builtImages) + shouldRedeploy := util.ReplaceImageNames(overwriteValues, cache, d.config.Images, builtImages, nil) if forceDeploy == false && shouldRedeploy { forceDeploy = true } @@ -191,50 +189,3 @@ func (d *DeployConfig) internalDeploy(cache *generated.CacheConfig, forceDeploy return true, nil } - -func replaceContainerNames(overwriteValues map[interface{}]interface{}, cache *generated.CacheConfig, imagesConf map[string]*latest.ImageConfig, builtImages map[string]string) bool { - shouldRedeploy := false - - match := func(path, key, value string) bool { - image, err := registry.GetStrippedDockerImageName(value) - if err != nil { - return false - } - - // Search for image name - for _, imageCache := range cache.Images { - if imageCache.ImageName == image && imageCache.Tag != "" { - if builtImages != nil { - if _, ok := builtImages[image]; ok { - shouldRedeploy = true - } - } - - return true - } - } - - return false - } - - replace := func(path, value string) (interface{}, error) { - image, err := registry.GetStrippedDockerImageName(value) - if err != nil { - return false, nil - } - - // Search for image name - for _, imageCache := range cache.Images { - if imageCache.ImageName == image { - return image + ":" + imageCache.Tag, nil - } - } - - return value, nil - } - - // We ignore the error here because our replace function never throws an error - _ = walk.Walk(overwriteValues, match, replace) - - return shouldRedeploy -} diff --git a/pkg/devspace/deploy/deployer/helm/deploy_test.go b/pkg/devspace/deploy/deployer/helm/deploy_test.go index ee366bdd67..db2f797f90 100644 --- a/pkg/devspace/deploy/deployer/helm/deploy_test.go +++ b/pkg/devspace/deploy/deployer/helm/deploy_test.go @@ -124,79 +124,3 @@ func TestDeploy(t *testing.T) { assert.Equal(t, string(cacheAsYaml), string(expectationAsYaml), "Unexpected cache in testCase %s", testCase.name) } } - -type replaceContainerNamesTestCase struct { - name string - - overwriteValues map[interface{}]interface{} - cache *generated.CacheConfig - imagesConf map[string]*latest.ImageConfig - builtImages map[string]string - - expectedShouldRedeploy bool - expectedOverwriteValues map[interface{}]interface{} -} - -func TestReplaceContainerNames(t *testing.T) { - testCases := []replaceContainerNamesTestCase{ - replaceContainerNamesTestCase{ - name: "invalid image name", - overwriteValues: map[interface{}]interface{}{ - "": "", - }, - cache: &generated.CacheConfig{ - Images: map[string]*generated.ImageCache{ - "": &generated.ImageCache{}, - }, - }, - expectedOverwriteValues: map[interface{}]interface{}{ - "": "", - }, - }, - replaceContainerNamesTestCase{ - name: "Image not in cache", - overwriteValues: map[interface{}]interface{}{ - "": "myimage", - }, - cache: &generated.CacheConfig{ - Images: map[string]*generated.ImageCache{}, - }, - expectedOverwriteValues: map[interface{}]interface{}{ - "": "myimage", - }, - }, - replaceContainerNamesTestCase{ - name: "Image in cache", - overwriteValues: map[interface{}]interface{}{ - "": "myimage", - }, - cache: &generated.CacheConfig{ - Images: map[string]*generated.ImageCache{ - "": &generated.ImageCache{ - ImageName: "myimage", - Tag: "someTag", - }, - }, - }, - builtImages: map[string]string{ - "myimage": "", - }, - expectedShouldRedeploy: true, - expectedOverwriteValues: map[interface{}]interface{}{ - "": "myimage:someTag", - }, - }, - } - - for _, testCase := range testCases { - shouldRedeploy := replaceContainerNames(testCase.overwriteValues, testCase.cache, testCase.imagesConf, testCase.builtImages) - - assert.Equal(t, shouldRedeploy, testCase.expectedShouldRedeploy, "Unexpected deployed-bool in testCase %s", testCase.name) - - ovAsYaml, err := yaml.Marshal(testCase.overwriteValues) - assert.NilError(t, err, "Error marshaling overwriteValues in testCase %s", testCase.name) - expectationAsYaml, err := yaml.Marshal(testCase.expectedOverwriteValues) - assert.NilError(t, err, "Error marshaling expectation in testCase %s", testCase.name) - assert.Equal(t, string(ovAsYaml), string(expectationAsYaml), "Unexpected overwriteValues in testCase %s", testCase.name) - } -} diff --git a/pkg/devspace/deploy/deployer/kubectl/kubectl.go b/pkg/devspace/deploy/deployer/kubectl/kubectl.go index 9e49969591..ab365dd131 100644 --- a/pkg/devspace/deploy/deployer/kubectl/kubectl.go +++ b/pkg/devspace/deploy/deployer/kubectl/kubectl.go @@ -11,9 +11,8 @@ import ( "github.com/devspace-cloud/devspace/pkg/devspace/config/generated" "github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer" - "github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/kubectl/walk" + "github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/util" "github.com/devspace-cloud/devspace/pkg/devspace/kubectl" - "github.com/devspace-cloud/devspace/pkg/devspace/registry" "github.com/devspace-cloud/devspace/pkg/devspace/config/versions/latest" "github.com/devspace-cloud/devspace/pkg/util/hash" @@ -252,7 +251,7 @@ func (d *DeployConfig) getReplacedManifest(manifest string, cache *generated.Cac if len(cache.Images) > 0 { if d.DeploymentConfig.Kubectl.ReplaceImageTags == nil || *d.DeploymentConfig.Kubectl.ReplaceImageTags == true { - shouldRedeploy = replaceManifest(manifestYaml, cache, d.config.Images, builtImages) || shouldRedeploy + shouldRedeploy = util.ReplaceImageNames(manifestYaml, cache, d.config.Images, builtImages, map[string]bool{"image": true}) || shouldRedeploy } } @@ -319,60 +318,3 @@ func (d *DeployConfig) dryRun(manifest string) ([]byte, error) { return output, nil } - -func replaceManifest(manifest map[interface{}]interface{}, cache *generated.CacheConfig, imagesConf map[string]*latest.ImageConfig, builtImages map[string]string) bool { - shouldRedeploy := false - - match := func(path, key, value string) bool { - if key == "image" { - image, err := registry.GetStrippedDockerImageName(value) - if err != nil { - return false - } - - // Search for image name - for _, imageCache := range cache.Images { - found := false - for _, imageConfig := range imagesConf { - if imageConfig.Image == image { - found = true - break - } - } - - if found && imageCache.ImageName == image && imageCache.Tag != "" { - if builtImages != nil { - if _, ok := builtImages[image]; ok { - shouldRedeploy = true - } - } - - return true - } - } - } - - return false - } - - replace := func(path, value string) (interface{}, error) { - image, err := registry.GetStrippedDockerImageName(value) - if err != nil { - return false, nil - } - - // Search for image name - for _, imageCache := range cache.Images { - if imageCache.ImageName == image { - return image + ":" + imageCache.Tag, nil - } - } - - return value, nil - } - - // We ignore the error here because the replace function can never throw an error - _ = walk.Walk(manifest, match, replace) - - return shouldRedeploy -} diff --git a/pkg/devspace/deploy/deployer/util/replace.go b/pkg/devspace/deploy/deployer/util/replace.go new file mode 100644 index 0000000000..71389ccac8 --- /dev/null +++ b/pkg/devspace/deploy/deployer/util/replace.go @@ -0,0 +1,75 @@ +package util + +import ( + "github.com/devspace-cloud/devspace/pkg/devspace/config/generated" + "github.com/devspace-cloud/devspace/pkg/devspace/config/versions/latest" + "github.com/devspace-cloud/devspace/pkg/devspace/deploy/deployer/kubectl/walk" + "github.com/devspace-cloud/devspace/pkg/devspace/registry" +) + +// ReplaceImageNames replaces images within a certain manifest with the correct tags from the cache +func ReplaceImageNames(manifest map[interface{}]interface{}, cache *generated.CacheConfig, imagesConf map[string]*latest.ImageConfig, builtImages map[string]string, keys map[string]bool) bool { + if imagesConf == nil { + imagesConf = map[string]*latest.ImageConfig{} + } + if keys == nil { + keys = map[string]bool{} + } + + // strip out images from cache that are not in the imagesconf anymore + for name := range cache.Images { + if _, ok := imagesConf[name]; !ok { + delete(cache.Images, name) + } + } + + shouldRedeploy := false + + match := func(path, key, value string) bool { + if len(keys) > 0 && keys[key] == false { + return false + } + + // Strip tag from image + image, err := registry.GetStrippedDockerImageName(value) + if err != nil { + return false + } + + // Search for image name + for _, imageCache := range cache.Images { + if imageCache.ImageName == image && imageCache.Tag != "" { + if builtImages != nil { + if _, ok := builtImages[image]; ok { + shouldRedeploy = true + } + } + + return true + } + } + + return false + } + + replace := func(path, value string) (interface{}, error) { + image, err := registry.GetStrippedDockerImageName(value) + if err != nil { + return false, nil + } + + // Search for image name + for _, imageCache := range cache.Images { + if imageCache.ImageName == image { + return image + ":" + imageCache.Tag, nil + } + } + + return value, nil + } + + // We ignore the error here because the replace function can never throw an error + _ = walk.Walk(manifest, match, replace) + + return shouldRedeploy +} diff --git a/pkg/devspace/deploy/deployer/util/replace_test.go b/pkg/devspace/deploy/deployer/util/replace_test.go new file mode 100644 index 0000000000..52bbae02c9 --- /dev/null +++ b/pkg/devspace/deploy/deployer/util/replace_test.go @@ -0,0 +1,89 @@ +package util + +import ( + "testing" + + "github.com/devspace-cloud/devspace/pkg/devspace/config/generated" + "github.com/devspace-cloud/devspace/pkg/devspace/config/versions/latest" + "gopkg.in/yaml.v2" + "gotest.tools/assert" +) + +type replaceContainerNamesTestCase struct { + name string + + overwriteValues map[interface{}]interface{} + cache *generated.CacheConfig + imagesConf map[string]*latest.ImageConfig + builtImages map[string]string + + expectedShouldRedeploy bool + expectedOverwriteValues map[interface{}]interface{} +} + +func TestReplaceContainerNames(t *testing.T) { + testCases := []replaceContainerNamesTestCase{ + replaceContainerNamesTestCase{ + name: "invalid image name", + overwriteValues: map[interface{}]interface{}{ + "": "", + }, + cache: &generated.CacheConfig{ + Images: map[string]*generated.ImageCache{ + "": &generated.ImageCache{}, + }, + }, + expectedOverwriteValues: map[interface{}]interface{}{ + "": "", + }, + }, + replaceContainerNamesTestCase{ + name: "Image not in cache", + overwriteValues: map[interface{}]interface{}{ + "": "myimage", + }, + cache: &generated.CacheConfig{ + Images: map[string]*generated.ImageCache{}, + }, + expectedOverwriteValues: map[interface{}]interface{}{ + "": "myimage", + }, + }, + replaceContainerNamesTestCase{ + name: "Image in cache", + overwriteValues: map[interface{}]interface{}{ + "": "myimage", + }, + imagesConf: map[string]*latest.ImageConfig{ + "test": &latest.ImageConfig{}, + }, + cache: &generated.CacheConfig{ + Images: map[string]*generated.ImageCache{ + "test": &generated.ImageCache{ + ImageName: "myimage", + Tag: "someTag", + }, + }, + }, + builtImages: map[string]string{ + "myimage": "", + }, + expectedShouldRedeploy: true, + expectedOverwriteValues: map[interface{}]interface{}{ + "": "myimage:someTag", + }, + }, + } + + for _, testCase := range testCases { + shouldRedeploy := ReplaceImageNames(testCase.overwriteValues, testCase.cache, testCase.imagesConf, testCase.builtImages, nil) + + assert.Equal(t, shouldRedeploy, testCase.expectedShouldRedeploy, "Unexpected deployed-bool in testCase %s", testCase.name) + + ovAsYaml, err := yaml.Marshal(testCase.overwriteValues) + assert.NilError(t, err, "Error marshaling overwriteValues in testCase %s", testCase.name) + expectationAsYaml, err := yaml.Marshal(testCase.expectedOverwriteValues) + assert.NilError(t, err, "Error marshaling expectation in testCase %s", testCase.name) + assert.Equal(t, string(ovAsYaml), string(expectationAsYaml), "Unexpected overwriteValues in testCase %s", testCase.name) + } +}