Skip to content

Commit

Permalink
correctly parses docker properties with inheritted manifest
Browse files Browse the repository at this point in the history
[#97697116]

Signed-off-by: Nick Wei <nwei@pivotal.io>
  • Loading branch information
mariash authored and Nick Wei committed Sep 15, 2017
1 parent a231636 commit e9e61f1
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 10 deletions.
17 changes: 7 additions & 10 deletions cf/manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -541,25 +541,22 @@ func parseDocker(input generic.Map, errs *[]error) models.ManifestDocker {
return models.ManifestDocker{}
}

docker, ok := input.Get("docker").(map[interface{}]interface{})
if !ok {
return models.ManifestDocker{}
}
dockerMap := generic.NewMap(input.Get("docker"))

imageValue := ""
image, imageExists := docker["image"]
if imageExists {
imageValue, ok = image.(string)
if dockerMap.Has("image") {
var ok bool
imageValue, ok = dockerMap.Get("image").(string)
if !ok {
*errs = append(*errs, fmt.Errorf(T("'docker.image' must be a string")))
return models.ManifestDocker{}
}
}

usernameValue := ""
username, usernameExists := docker["username"]
if usernameExists {
usernameValue, ok = username.(string)
if dockerMap.Has("username") {
var ok bool
usernameValue, ok = dockerMap.Get("username").(string)
if !ok {
*errs = append(*errs, fmt.Errorf(T("'docker.username' must be a string")))
return models.ManifestDocker{}
Expand Down
22 changes: 22 additions & 0 deletions cf/manifest/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,28 @@ var _ = Describe("Manifests", func() {
})
})

Context("when the 'docker' property is specified but contains no properties", func() {
var manifest *manifest.Manifest

BeforeEach(func() {
manifest = NewManifest("/some/path/manifest.yml", generic.NewMap(map[interface{}]interface{}{
"applications": []interface{}{
generic.NewMap(map[interface{}]interface{}{
"docker": map[interface{}]interface{}{},
}),
},
}))
})

It("returns a parsed manifest that does not contain any docker fields", func() {
apps, err := manifest.Applications()
Expect(err).NotTo(HaveOccurred())
Expect(apps).To(HaveLen(1))
Expect(apps[0].DockerImage).To(BeNil())
Expect(apps[0].DockerUsername).To(BeNil())
})
})

Context("when the 'docker' property is specified", func() {
var manifest *manifest.Manifest

Expand Down
40 changes: 40 additions & 0 deletions integration/isolated/manifest_inheritance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ func pushHelloWorldAppWithManifests(manifests []string) {
})
}

func pushDockerWithManifests(manifests []string) {
helpers.WithHelloWorldApp(func(appDir string) {
pushPath := filepath.Join(appDir, "manifest-0.yml")
for i, manifest := range manifests {
manifestPath := filepath.Join(appDir, fmt.Sprintf("manifest-%d.yml", i))
manifest = strings.Replace(manifest, "inherit: {some-parent}", fmt.Sprintf("inherit: manifest-%d.yml", i+1), 1)
manifest = strings.Replace(manifest, "path: {some-dir}", fmt.Sprintf("path: %s", appDir), -1)
err := ioutil.WriteFile(manifestPath, []byte(manifest), 0666)
Expect(err).ToNot(HaveOccurred())
}
Eventually(helpers.CustomCF(helpers.CFEnv{WorkingDirectory: appDir}, "push", "-f", pushPath)).Should(Exit())
})
}

// CASE 1: child manifest (no inheritance)
//
// APPLICATION params:
Expand Down Expand Up @@ -628,6 +642,32 @@ FOO: child-global
})
})

Context("when the child has docker image properties; and the parent has global docker image properties", func() {
BeforeEach(func() {
pushDockerWithManifests([]string{
fmt.Sprintf(`
---
inherit: {some-parent}
applications:
- name: %s
docker:
image: some-image
`, app1Name),
fmt.Sprintf(`
---
docker:
image: some-parent-docker-image
`),
})
})

It("pushes with child docker image properties taking precedence;", func() {
session := helpers.CF("app", app1Name)
Eventually(session.Out).Should(Say(`docker image:\s*some-image`))
Eventually(session).Should(Exit(0))
})
})

Context("when the child has applications and global properties; and the parent has global properties", func() {
BeforeEach(func() {
pushHelloWorldAppWithManifests([]string{
Expand Down

0 comments on commit e9e61f1

Please sign in to comment.