Skip to content

Commit 729eb2f

Browse files
committed
Use UnmarshalStrict for all YAML parsing
Centralize yaml unmarshalling to k8s.io/enhancements/yaml and add an UnmarshalStrict() function implemented using gopkg.in/yaml.v3 Somehow all of gopkg.in/yaml.v2, gopkg.in/yaml.v3 and sigs.k8s.io/yaml were being used across the code. Hopefully this unifies and lets us choose just one. Update KEPs to fix unmarshalling failures. The one controversial change would be to support the KEP that added "deprecated" and "removed" milestones. I opted to add those to the API, but am open to suggestion on how that planned lifecycle should be encoded in the KEP instead.
1 parent 0ec64c8 commit 729eb2f

File tree

9 files changed

+67
-21
lines changed

9 files changed

+67
-21
lines changed

api/approval.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ import (
2323

2424
"github.com/go-playground/validator/v10"
2525
"github.com/pkg/errors"
26-
"gopkg.in/yaml.v3"
26+
27+
"k8s.io/enhancements/pkg/yaml"
2728
)
2829

2930
type PRRApprovals []*PRRApproval
@@ -108,7 +109,7 @@ func (p *PRRHandler) Parse(in io.Reader) (*PRRApproval, error) {
108109
return approval, errors.Wrap(err, "reading file")
109110
}
110111

111-
if err := yaml.Unmarshal(body.Bytes(), &approval); err != nil {
112+
if err := yaml.UnmarshalStrict(body.Bytes(), &approval); err != nil {
112113
p.Errors = append(p.Errors, errors.Wrap(err, "error unmarshalling YAML"))
113114
return approval, errors.Wrap(err, "unmarshalling YAML")
114115
}

api/groups.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626

2727
"github.com/pkg/errors"
2828

29-
"sigs.k8s.io/yaml"
29+
"k8s.io/enhancements/pkg/yaml"
3030
)
3131

3232
// GroupFetcher is responsible for getting informationg about groups in the
@@ -147,9 +147,10 @@ func (f *RemoteGroupFetcher) FetchPRRApprovers() ([]string, error) {
147147
}
148148

149149
config := &struct {
150-
Data map[string][]string `json:"aliases,omitempty"`
150+
Data map[string][]string `json:"aliases,omitempty" yaml:"aliases,omitempty"`
151151
}{}
152-
if err := yaml.Unmarshal(body, config); err != nil {
152+
153+
if err := yaml.UnmarshalStrict(body, config); err != nil {
153154
return nil, errors.Wrap(err, "unmarshalling owners aliases")
154155
}
155156

api/proposal.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import (
2626

2727
"github.com/go-playground/validator/v10"
2828
"github.com/pkg/errors"
29-
"gopkg.in/yaml.v3"
29+
30+
"k8s.io/enhancements/pkg/yaml"
3031
)
3132

3233
var ValidStages = []string{
@@ -119,7 +120,7 @@ func (k *KEPHandler) Parse(in io.Reader) (*Proposal, error) {
119120
kep.Contents = ""
120121
}
121122

122-
if err := yaml.Unmarshal(metadata, &kep); err != nil {
123+
if err := yaml.UnmarshalStrict(metadata, &kep); err != nil {
123124
k.Errors = append(k.Errors, errors.Wrap(err, "error unmarshalling YAML"))
124125
return kep, errors.Wrap(err, "unmarshalling YAML")
125126
}
@@ -194,9 +195,11 @@ func (k *KEPHandler) Validate(p *Proposal) []error {
194195
}
195196

196197
type Milestone struct {
197-
Alpha string `json:"alpha" yaml:"alpha"`
198-
Beta string `json:"beta" yaml:"beta"`
199-
Stable string `json:"stable" yaml:"stable"`
198+
Alpha string `json:"alpha" yaml:"alpha"`
199+
Beta string `json:"beta" yaml:"beta"`
200+
Stable string `json:"stable" yaml:"stable"`
201+
Deprecated string `json:"deprecated" yaml:"deprecated,omitempty"`
202+
Removed string `json:"removed" yaml:"removed,omitempty"`
200203
}
201204

202205
type FeatureGate struct {

keps/sig-network/2595-expanded-dns-config/kep.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ latest-milestone: "v1.22"
3131
milestone:
3232
alpha: "v1.22"
3333
beta: "x.y"
34-
GA: "x.y"
34+
stable: "x.y"
3535

3636
# The following PRR answers are required at alpha release
3737
# List the feature gate name and the components for which it must be enabled

keps/sig-node/2727-grpc-probe/kep.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ participating-sigs:
99
- sig-network
1010
status: implementable
1111
creation-date: 2020-04-04
12-
modified-date: 2021-05-12
12+
last-updated: 2021-05-12
1313
reviewers:
1414
- "@thockin"
1515
- "@mrunalp"

pkg/repo/repo.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,13 @@ import (
3030
"github.com/pkg/errors"
3131
"github.com/sirupsen/logrus"
3232
"golang.org/x/oauth2"
33-
"gopkg.in/yaml.v3"
3433

35-
"k8s.io/enhancements/api"
36-
"k8s.io/enhancements/pkg/kepval"
3734
krgh "k8s.io/release/pkg/github"
3835
"k8s.io/test-infra/prow/git"
36+
37+
"k8s.io/enhancements/api"
38+
"k8s.io/enhancements/pkg/kepval"
39+
"k8s.io/enhancements/pkg/yaml"
3940
)
4041

4142
const (
@@ -439,7 +440,7 @@ func (r *Repo) loadKEPFromYaml(kepPath string) (*api.Proposal, error) {
439440
}
440441

441442
var p api.Proposal
442-
err = yaml.Unmarshal(b, &p)
443+
err = yaml.UnmarshalStrict(b, &p)
443444
if err != nil {
444445
return nil, fmt.Errorf("unable to load KEP metadata: %s", err)
445446
}

pkg/repo/repo_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ import (
2424
"testing"
2525

2626
"github.com/stretchr/testify/require"
27-
"gopkg.in/yaml.v3"
27+
"k8s.io/release/pkg/log"
2828

2929
"k8s.io/enhancements/api"
3030
"k8s.io/enhancements/pkg/repo"
31-
"k8s.io/release/pkg/log"
31+
"k8s.io/enhancements/pkg/yaml"
3232
)
3333

3434
var fixture = struct {
@@ -91,7 +91,7 @@ func TestProposalValidate(t *testing.T) {
9191
require.NoError(t, err)
9292

9393
var p api.Proposal
94-
err = yaml.Unmarshal(b, &p)
94+
err = yaml.UnmarshalStrict(b, &p)
9595
require.NoError(t, err)
9696

9797
errs := parser.Validate(&p)

pkg/repo/write_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ import (
2727

2828
"k8s.io/enhancements/api"
2929
"k8s.io/enhancements/pkg/repo"
30-
"sigs.k8s.io/yaml"
30+
31+
"k8s.io/enhancements/pkg/yaml"
3132
)
3233

3334
// TODO: Consider using afero to mock the filesystem here
@@ -126,7 +127,7 @@ func TestWriteKep(t *testing.T) {
126127
require.NoError(t, err)
127128

128129
var p api.Proposal
129-
err = yaml.Unmarshal(b, &p)
130+
err = yaml.UnmarshalStrict(b, &p)
130131
require.NoError(t, err)
131132

132133
p.OwningSIG = tc.sig

pkg/yaml/yaml.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
Copyright 2021 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package yaml
18+
19+
import (
20+
"bytes"
21+
22+
"gopkg.in/yaml.v3"
23+
)
24+
25+
// UnmarshalStrict unmarshals the contents of body into the given interface,
26+
// and returns an error if any duplicate or unknown fields are encountered
27+
func UnmarshalStrict(body []byte, v interface{}) error {
28+
r := bytes.NewReader(body)
29+
d := yaml.NewDecoder(r)
30+
d.KnownFields(true)
31+
err := d.Decode(v)
32+
return err
33+
}
34+
35+
// Marshal returns a byte array containing a YAML representation of the
36+
// given interface, and a non-nil error if there was an error
37+
func Marhsal(v interface{}) ([]byte, error) {
38+
return yaml.Marshal(v)
39+
}

0 commit comments

Comments
 (0)