Skip to content

Commit

Permalink
Log a warning when loading bundle manifests with multiple documents
Browse files Browse the repository at this point in the history
decodeFileFS reads only the first YAML document in a manifest and
ignores the rest of the documents in that file. This commit makes
it so that it will log a warning when more than one document is
encountered in a single manifest file.

Inspired by this annoying bug: https://gitlab.com/nvidia/kubernetes/gpu-operator/-/merge_requests/254
  • Loading branch information
omertuc committed Jul 12, 2021
1 parent 4ac6a2e commit 072ef74
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 8 deletions.
14 changes: 12 additions & 2 deletions pkg/registry/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/yaml"
"github.com/sirupsen/logrus"
)

// DecodeUnstructured decodes a raw stream into a an
Expand Down Expand Up @@ -44,7 +45,7 @@ func DecodePackageManifest(reader io.Reader) (manifest *PackageManifest, err err
return
}

func decodeFileFS(root fs.FS, path string, into interface{}) error {
func decodeFileFS(root fs.FS, path string, into interface{}, log *logrus.Entry) error {
fileReader, err := root.Open(path)
if err != nil {
return fmt.Errorf("unable to read file %s: %s", path, err)
Expand All @@ -53,5 +54,14 @@ func decodeFileFS(root fs.FS, path string, into interface{}) error {

decoder := yaml.NewYAMLOrJSONDecoder(fileReader, 30)

return decoder.Decode(into)
errRet := decoder.Decode(into)

// Look for and warn about extra documents
extraDocument := &unstructured.Unstructured{}
err = decoder.Decode(extraDocument)
if err == nil && log != nil {
log.Warnf("found more than one document inside %s, using only the first one", path)
}

return errRet
}
4 changes: 2 additions & 2 deletions pkg/registry/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ func TestDecodeFileFS(t *testing.T) {
}

var nilPtr *foo
require.NoError(t, decodeFileFS(root, "foo.yaml", nilPtr))
require.NoError(t, decodeFileFS(root, "foo.yaml", nilPtr, nil))
require.Nil(t, nilPtr)

ptr := &foo{}
require.NoError(t, decodeFileFS(root, "foo.yaml", ptr))
require.NoError(t, decodeFileFS(root, "foo.yaml", ptr, nil))
require.NotNil(t, ptr)
require.Equal(t, "baz", ptr.Bar)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/registry/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (b *bundleParser) addManifests(manifests fs.FS, bundle *Bundle) error {
}

obj := &unstructured.Unstructured{}
if err = decodeFileFS(manifests, name, obj); err != nil {
if err = decodeFileFS(manifests, name, obj, b.log); err != nil {
b.log.Warnf("failed to decode: %s", err)
continue
}
Expand Down Expand Up @@ -128,23 +128,23 @@ func (b *bundleParser) addMetadata(metadata fs.FS, bundle *Bundle) error {
name := f.Name()
if af == nil {
decoded := AnnotationsFile{}
if err = decodeFileFS(metadata, name, &decoded); err == nil {
if err = decodeFileFS(metadata, name, &decoded, b.log); err == nil {
if decoded != (AnnotationsFile{}) {
af = &decoded
}
}
}
if df == nil {
decoded := DependenciesFile{}
if err = decodeFileFS(metadata, name, &decoded); err == nil {
if err = decodeFileFS(metadata, name, &decoded, b.log); err == nil {
if len(decoded.Dependencies) > 0 {
df = &decoded
}
}
}
if pf == nil {
decoded := PropertiesFile{}
if err = decodeFileFS(metadata, name, &decoded); err == nil {
if err = decodeFileFS(metadata, name, &decoded, b.log); err == nil {
if len(decoded.Properties) > 0 {
pf = &decoded
}
Expand Down

0 comments on commit 072ef74

Please sign in to comment.