diff --git a/pkg/registry/decode.go b/pkg/registry/decode.go index 392d5dd88..0a9587d09 100644 --- a/pkg/registry/decode.go +++ b/pkg/registry/decode.go @@ -6,6 +6,7 @@ import ( "io" "io/fs" + "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/yaml" ) @@ -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) @@ -53,5 +54,16 @@ func decodeFileFS(root fs.FS, path string, into interface{}) error { decoder := yaml.NewYAMLOrJSONDecoder(fileReader, 30) - return decoder.Decode(into) + errRet := decoder.Decode(into) + + if errRet == nil { + // Look for and warn about extra documents + extraDocument := &map[string]interface{}{} + 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 } diff --git a/pkg/registry/decode_test.go b/pkg/registry/decode_test.go index 86a7f0d72..02b26de5a 100644 --- a/pkg/registry/decode_test.go +++ b/pkg/registry/decode_test.go @@ -6,6 +6,8 @@ import ( "testing" "testing/fstest" + "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -97,17 +99,34 @@ func TestDecodeFileFS(t *testing.T) { } root := fstest.MapFS{ - "foo.yaml": &fstest.MapFile{Data: []byte("bar: baz")}, + "foo.yaml": &fstest.MapFile{Data: []byte("bar: baz")}, + "multi.yaml": &fstest.MapFile{Data: []byte("bar: baz\n---\nfoo: bar")}, } + logger, logHook := test.NewNullLogger() + entry := logger.WithFields(nil) + var nilPtr *foo - require.NoError(t, decodeFileFS(root, "foo.yaml", nilPtr)) + require.NoError(t, decodeFileFS(root, "foo.yaml", nilPtr, entry)) require.Nil(t, nilPtr) + require.Equal(t, 0, len(logHook.Entries)) + logHook.Reset() ptr := &foo{} - require.NoError(t, decodeFileFS(root, "foo.yaml", ptr)) + require.NoError(t, decodeFileFS(root, "foo.yaml", ptr, entry)) + require.NotNil(t, ptr) + require.Equal(t, "baz", ptr.Bar) + require.Equal(t, 0, len(logHook.Entries)) + logHook.Reset() + + ptr = &foo{} + require.NoError(t, decodeFileFS(root, "multi.yaml", ptr, entry)) require.NotNil(t, ptr) require.Equal(t, "baz", ptr.Bar) + require.Equal(t, 1, len(logHook.Entries)) + require.Equal(t, logrus.WarnLevel, logHook.LastEntry().Level) + require.Equal(t, "found more than one document inside multi.yaml, using only the first one", logHook.LastEntry().Message) + logHook.Reset() } func loadFile(t *testing.T, path string) io.Reader { diff --git a/pkg/registry/parse.go b/pkg/registry/parse.go index 8dc079dd5..4b13ef767 100644 --- a/pkg/registry/parse.go +++ b/pkg/registry/parse.go @@ -73,7 +73,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 } @@ -127,7 +127,7 @@ 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 } @@ -135,7 +135,7 @@ func (b *bundleParser) addMetadata(metadata fs.FS, bundle *Bundle) error { } 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 } @@ -143,7 +143,7 @@ func (b *bundleParser) addMetadata(metadata fs.FS, bundle *Bundle) error { } 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 } diff --git a/vendor/github.com/sirupsen/logrus/hooks/test/test.go b/vendor/github.com/sirupsen/logrus/hooks/test/test.go new file mode 100644 index 000000000..b16d06654 --- /dev/null +++ b/vendor/github.com/sirupsen/logrus/hooks/test/test.go @@ -0,0 +1,91 @@ +// The Test package is used for testing logrus. +// It provides a simple hooks which register logged messages. +package test + +import ( + "io/ioutil" + "sync" + + "github.com/sirupsen/logrus" +) + +// Hook is a hook designed for dealing with logs in test scenarios. +type Hook struct { + // Entries is an array of all entries that have been received by this hook. + // For safe access, use the AllEntries() method, rather than reading this + // value directly. + Entries []logrus.Entry + mu sync.RWMutex +} + +// NewGlobal installs a test hook for the global logger. +func NewGlobal() *Hook { + + hook := new(Hook) + logrus.AddHook(hook) + + return hook + +} + +// NewLocal installs a test hook for a given local logger. +func NewLocal(logger *logrus.Logger) *Hook { + + hook := new(Hook) + logger.Hooks.Add(hook) + + return hook + +} + +// NewNullLogger creates a discarding logger and installs the test hook. +func NewNullLogger() (*logrus.Logger, *Hook) { + + logger := logrus.New() + logger.Out = ioutil.Discard + + return logger, NewLocal(logger) + +} + +func (t *Hook) Fire(e *logrus.Entry) error { + t.mu.Lock() + defer t.mu.Unlock() + t.Entries = append(t.Entries, *e) + return nil +} + +func (t *Hook) Levels() []logrus.Level { + return logrus.AllLevels +} + +// LastEntry returns the last entry that was logged or nil. +func (t *Hook) LastEntry() *logrus.Entry { + t.mu.RLock() + defer t.mu.RUnlock() + i := len(t.Entries) - 1 + if i < 0 { + return nil + } + return &t.Entries[i] +} + +// AllEntries returns all entries that were logged. +func (t *Hook) AllEntries() []*logrus.Entry { + t.mu.RLock() + defer t.mu.RUnlock() + // Make a copy so the returned value won't race with future log requests + entries := make([]*logrus.Entry, len(t.Entries)) + for i := 0; i < len(t.Entries); i++ { + // Make a copy, for safety + entries[i] = &t.Entries[i] + } + return entries +} + +// Reset removes all Entries from this test hook. +func (t *Hook) Reset() { + t.mu.Lock() + defer t.mu.Unlock() + t.Entries = make([]logrus.Entry, 0) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index eaf6f18ad..cdeb24143 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -444,6 +444,7 @@ github.com/russross/blackfriday # github.com/sirupsen/logrus v1.8.1 ## explicit github.com/sirupsen/logrus +github.com/sirupsen/logrus/hooks/test # github.com/spf13/afero v1.6.0 ## explicit # github.com/spf13/cobra v1.1.3