Skip to content

Commit

Permalink
Move from go-yaml/yaml v2 to ghodss/yaml
Browse files Browse the repository at this point in the history
Previously we were using go-yaml/yaml for unmarshalling yaml to
api models.
There's problem with some fields, for example pod's spec.hostNetwork,
it didn't work with 'hostNetwork' in the yaml file.
The root cause were that protobuf adds json name field metadata to
the pods.pb.go generated file, but yaml.Unmarshal doesn't use that, but
defaults to "all lowercase" field name.
Switched to use ghodss/yaml which converts first yaml to JSON
and then unmarshals to the model, and that's why uses right field name.
  • Loading branch information
ernoaapa committed Dec 31, 2017
1 parent 812687d commit 5a92185
Show file tree
Hide file tree
Showing 9 changed files with 959 additions and 3 deletions.
2 changes: 1 addition & 1 deletion pkg/api/services/pods/v1/unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import (
"encoding/json"

utils "github.com/ernoaapa/eliot/pkg/utils/yaml"
"github.com/ghodss/yaml"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
yaml "gopkg.in/yaml.v2"
)

// UnmarshalYaml reads v1 Pods data in YAML format and unmarshals it to v1 api model
Expand Down
6 changes: 6 additions & 0 deletions pkg/api/services/pods/v1/unmarshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ metadata:
name: "foo"
namespace: "my-namespace"
spec:
hostNetwork: true
restartPolicy: always
containers:
- name: "foo-1"
image: "docker.io/library/hello-world:latest"
Expand All @@ -23,6 +25,8 @@ spec:

assert.Equal(t, "foo", pods[0].Metadata.Name, "Should unmarshal name")
assert.Equal(t, 2, len(pods[0].Spec.Containers), "Should have one container spec")
assert.Equal(t, "always", pods[0].Spec.RestartPolicy, "Should have host network")
assert.True(t, pods[0].Spec.HostNetwork, "Should have host network")
}

func TestUnmarshalMultiDocumentYaml(t *testing.T) {
Expand Down Expand Up @@ -55,6 +59,7 @@ func TestUnmarshalListYaml(t *testing.T) {
name: "foo"
namespace: "my-namespace"
spec:
hostNetwork: true
containers:
- name: "foo-1"
image: "docker.io/library/hello-world:latest"
Expand All @@ -73,6 +78,7 @@ func TestUnmarshalListYaml(t *testing.T) {
assert.Equal(t, 2, len(pods), "Should have pod specs")
assert.Equal(t, "foo", pods[0].Metadata.Name, "Should unmarshal name")
assert.Equal(t, 2, len(pods[0].Spec.Containers), "Should have one container spec")
assert.True(t, pods[0].Spec.HostNetwork, "Should have one container spec")
}

func TestUnmarshalListJSON(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"github.com/ernoaapa/eliot/pkg/converter"
"github.com/ernoaapa/eliot/pkg/fs"
"github.com/ernoaapa/eliot/pkg/model"
"github.com/ghodss/yaml"
"github.com/pkg/errors"
yaml "gopkg.in/yaml.v2"
)

// Config is struct for configuration for CLI client
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"log"

"github.com/ernoaapa/eliot/pkg/fs"
"github.com/ghodss/yaml"
"github.com/pkg/errors"
yaml "gopkg.in/yaml.v2"
)

// ProjectConfig represents configuration in project directory
Expand Down
1 change: 1 addition & 0 deletions vendor.conf
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,4 @@ github.com/containerd/typeurl f6943554a7e7e88b3c14aad190bf05932da84788
github.com/docker/docker v17.05.0-ce
k8s.io/kubernetes v1.9.0-alpha.2
github.com/thejerf/suture v2.0.1
github.com/ghodss/yaml v1.0.0
50 changes: 50 additions & 0 deletions vendor/github.com/ghodss/yaml/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

121 changes: 121 additions & 0 deletions vendor/github.com/ghodss/yaml/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 5a92185

Please sign in to comment.