carvel-dev / vendir

Easy way to vendor portions of git repos, github releases, helm charts, docker image contents, etc. declaratively
https://carvel.dev/vendir
Apache License 2.0
276 stars 49 forks source link

Fix parsing empty yaml documents #228

Closed Zebradil closed 1 year ago

Zebradil commented 1 year ago

Fixes #201

Zebradil commented 1 year ago

@joaopapereira

Do you mind adding a test to ensure that this feature is working?

Disclaimer: I'm not a golang developer.

I was thinking about writing a test before implementing the fix, but I couldn't figure out how to do that nicely and without restructuring the existing code. So maybe I can get some advice here.

This is the function which would be tested:

func parseResources(paths []string, resourceFunc func([]byte) error) error

It accepts a list of file paths or - for stdin and a callback function and returns an error.

In a test, I can create a temporary file with content suitable for the test, pass the path to the function together with a callback function and check if parseResources returns an error.

This doesn't seem to be elegant but should work. Or maybe there are other ideas?

neil-hickey commented 1 year ago

👋 Hello, I wrote a quick test you can copy verbatim or not and use as inspiration to help move this issue along:

New file pkg/vendir/config/config_test.go

func TestEmptyLineStartsConfig(t *testing.T) {
    t.Run("whitespace in config is ignored", func(t *testing.T) {
        tempConfigPath := filepath.Join(t.TempDir(), "config.yml")
        configWithWhitespace := []byte(`
---
apiVersion: v1
kind: Secret
metadata:
  name: vendir-secret
data:
  username: dXNlcg==
  password: MTIzNDU2`)

        require.NoError(t, os.WriteFile(tempConfigPath, configWithWhitespace, 0666))

        _, _, _, err := config.NewConfigFromFiles([]string{tempConfigPath})
        require.NoError(t, err)
    })
}
Zebradil commented 1 year ago

@neil-hickey awesome, thanks!

I've added the test.

It fails on the develop branch:

--- FAIL: TestEmptyLineStartsConfig (0.00s)
    --- FAIL: TestEmptyLineStartsConfig/whitespace_in_config_is_ignored (0.00s)
        config_test.go:27:
                Error Trace:    /home/zebradil/development/forks/vendir/pkg/vendir/config/config_test.go:27
                Error:          Received unexpected error:
                                Parsing resource config '/tmp/TestEmptyLineStartsConfigwhitespace_in_config_is_ignored185744596/001/config.yml': Unknown apiVersion '' or kind '' for resource
                Test:           TestEmptyLineStartsConfig/whitespace_in_config_is_ignored
FAIL
FAIL    command-line-arguments  0.004s
FAIL

It passes on the feature branch:

ok      command-line-arguments  0.004s