Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

fixes #658 #659

Merged
merged 1 commit into from Mar 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/bundle/bundle.go
Expand Up @@ -82,14 +82,14 @@ type ImagePlatform struct {

// Image describes a container image in the bundle
type Image struct {
BaseImage
BaseImage `mapstructure:",squash"`
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch here!
I do have two questions regarding this change:

  • any particular reason you didn't include the json tag as well?
  • we haven't used squash for fields so far -- could you, please, include a couple of sentences why we need to add it, and why it's inconsistent with all other fields in the bundle?

Thanks!

Copy link
Contributor Author

@sbawaska sbawaska Mar 12, 2019

Choose a reason for hiding this comment

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

any particular reason you didn't include the json tag as well?

using squash causes the other structure to basically be inlined, as a result the json tag from the BaseImage struct will be used.

we haven't used squash for fields so far -- could you, please, include a couple of sentences why we need to add it, and why it's inconsistent with all other fields in the bundle?

As described in the bug #658, not having the squash tag prevents the Image from marshalling the fields of the BaseImage struct.

Description string `json:"description" mapstructure:"description"` //TODO: change? see where it's being used? change to description?
Refs []LocationRef `json:"refs" mapstructure:"refs"`
}

// InvocationImage contains the image type and location for the installation of a bundle
type InvocationImage struct {
BaseImage
BaseImage `mapstructure:",squash"`
}

// Location provides the location where a value should be written in
Expand Down
9 changes: 9 additions & 0 deletions pkg/duffle/manifest/manifest_test.go
Expand Up @@ -54,6 +54,15 @@ func TestLoad(t *testing.T) {
t.Errorf("expected a component named cnab but got %v", m.InvocationImages)
}

if len(m.Images) != 1 {
t.Errorf("exp 1 but was \"%v\"", len(m.Images))
}

img := m.Images["istio"]
if !strings.EqualFold(img.ImageType, "docker") {
t.Errorf("exp docker but was \"%v\"", img.ImageType)
}

if len(m.Parameters) != 1 {
t.Fatalf("expected 1 parameter but got %d", len(m.Parameters))
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/duffle/manifest/testdata/duffle.json
Expand Up @@ -9,6 +9,14 @@
}
}
},
"images": {
"istio": {
"description": "istio images",
"imageType": "docker",
"image": "docker.io/istio/citadel:1.0.2",
"digest": "sha256:ca4050c9fed3a2ddcaef32140686613c4110ed728f53262d0a23a7e17da73111"
}
},
"parameters": {
"foo": {
"type": "string"
Expand Down
6 changes: 6 additions & 0 deletions pkg/duffle/manifest/testdata/duffle.toml
Expand Up @@ -4,6 +4,12 @@ name = "testbundle"
name = "cnab"
builder = "docker"
configuration = { registry = "microsoft" }
[images]
[images.istio]
description = "istio images"
imageType = "docker"
image = "docker.io/istio/citadel:1.0.2"
digest = "sha256:ca4050c9fed3a2ddcaef32140686613c4110ed728f53262d0a23a7e17da73111"
[parameters.foo]
type = "string"

Expand Down
6 changes: 6 additions & 0 deletions pkg/duffle/manifest/testdata/duffle.yaml
Expand Up @@ -5,6 +5,12 @@ invocationImages:
builder: docker
configuration:
registry: microsoft
images:
istio:
description: "istio images"
imageType: "docker"
image: "docker.io/istio/citadel:1.0.2"
digest: "sha256:ca4050c9fed3a2ddcaef32140686613c4110ed728f53262d0a23a7e17da73111"
parameters:
foo:
type: string
Expand Down