From e97ca86703bbba3d96625fc3f2a5eda3bec5d628 Mon Sep 17 00:00:00 2001 From: Gabriel Cipriano Date: Fri, 17 Nov 2023 23:16:12 -0300 Subject: [PATCH 1/3] feat: validate ko main path --- internal/pipe/ko/ko.go | 19 ++++++++++++++++- internal/pipe/ko/ko_test.go | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/internal/pipe/ko/ko.go b/internal/pipe/ko/ko.go index 26e8f5a37ca..0b29271f0fc 100644 --- a/internal/pipe/ko/ko.go +++ b/internal/pipe/ko/ko.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "path/filepath" + "regexp" "strconv" "strings" "sync" @@ -48,7 +49,8 @@ var ( azureKeychain, ) - errNoRepository = errors.New("ko: missing repository: please set either the repository field or a $KO_DOCKER_REPO environment variable") + errNoRepository = errors.New("ko: missing repository: please set either the repository field or a $KO_DOCKER_REPO environment variable") + errInvalidMainPath = errors.New("ko: invalid Main path: ko.main (or build.main if ko.main is not set) should be a relative path.") ) // Pipe that build OCI compliant images with ko. @@ -93,6 +95,10 @@ func (Pipe) Default(ctx *context.Context) error { ko.Main = build.Main } + if err := validateMainPath(ko.Main); err != nil { + return err + } + if ko.WorkingDir == "" { ko.WorkingDir = build.Dir } @@ -420,3 +426,14 @@ func getTimeFromTemplate(ctx *context.Context, t string) (*v1.Time, error) { } return &v1.Time{Time: time.Unix(seconds, 0)}, nil } + +func validateMainPath(path string) error { + if matched, _ := regexp.MatchString(`^\.?(\.\/[^\/]?.*)?$`, path); !matched { + return errInvalidMainPath + } + // paths sure can have dots in them, but if the path ends in .go, it's propably a file that one misundertood as a valid value + if strings.HasSuffix(path, ".go") { + return errInvalidMainPath + } + return nil +} diff --git a/internal/pipe/ko/ko_test.go b/internal/pipe/ko/ko_test.go index 43485abcc03..5a51f6e086e 100644 --- a/internal/pipe/ko/ko_test.go +++ b/internal/pipe/ko/ko_test.go @@ -340,6 +340,48 @@ func TestPublishPipeSuccess(t *testing.T) { } } +func TestKoValidateMainPathIssue4383(t *testing.T) { + someKoConfig := []config.Ko{ + { + ID: "default", + Build: "foo", + }, + } + + ctx := testctx.NewWithCfg(config.Project{ + Builds: []config.Build{ + { + ID: "foo", + Main: "./...", + }, + }, + Kos: someKoConfig, + }) + require.NoError(t, Pipe{}.Default(ctx)) + + ctx = testctx.NewWithCfg(config.Project{ + Builds: []config.Build{ + { + ID: "foo", + Main: "/some/non/relative/path", + }, + }, + Kos: someKoConfig, + }) + require.EqualError(t, Pipe{}.Default(ctx), errInvalidMainPath.Error()) + + ctx = testctx.NewWithCfg(config.Project{ + Builds: []config.Build{ + { + ID: "foo", + Main: "./src/main.go", + }, + }, + Kos: someKoConfig, + }) + require.EqualError(t, Pipe{}.Default(ctx), errInvalidMainPath.Error()) +} + func TestPublishPipeError(t *testing.T) { makeCtx := func() *context.Context { return testctx.NewWithCfg(config.Project{ From b57e0d138a4d29b3bc64c59337af4a886c795369 Mon Sep 17 00:00:00 2001 From: Gabriel Cipriano Date: Sat, 18 Nov 2023 20:45:13 -0300 Subject: [PATCH 2/3] test: more validateMainPath tests --- internal/pipe/ko/ko.go | 4 +++ internal/pipe/ko/ko_test.go | 54 ++++++++++++++++++++++--------------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/internal/pipe/ko/ko.go b/internal/pipe/ko/ko.go index 0b29271f0fc..de96202559a 100644 --- a/internal/pipe/ko/ko.go +++ b/internal/pipe/ko/ko.go @@ -428,6 +428,10 @@ func getTimeFromTemplate(ctx *context.Context, t string) (*v1.Time, error) { } func validateMainPath(path string) error { + // if the path is empty, it's probably fine as ko will use the default value + if path == "" { + return nil + } if matched, _ := regexp.MatchString(`^\.?(\.\/[^\/]?.*)?$`, path); !matched { return errInvalidMainPath } diff --git a/internal/pipe/ko/ko_test.go b/internal/pipe/ko/ko_test.go index 5a51f6e086e..4ed31953bdc 100644 --- a/internal/pipe/ko/ko_test.go +++ b/internal/pipe/ko/ko_test.go @@ -340,46 +340,56 @@ func TestPublishPipeSuccess(t *testing.T) { } } -func TestKoValidateMainPathIssue4383(t *testing.T) { - someKoConfig := []config.Ko{ - { - ID: "default", - Build: "foo", - }, - } - - ctx := testctx.NewWithCfg(config.Project{ +func TestKoValidateMainPathIssue4382(t *testing.T) { + // testing the validation of the main path directly to cover many cases + require.NoError(t, validateMainPath("")) + require.NoError(t, validateMainPath(".")) + require.NoError(t, validateMainPath("./...")) + require.NoError(t, validateMainPath("./app")) + require.NoError(t, validateMainPath("../../../...")) + require.NoError(t, validateMainPath("../../app/")) + require.NoError(t, validateMainPath("./testdata/app/main")) + require.NoError(t, validateMainPath("./testdata/app/folder.with.dots")) + + require.ErrorIs(t, validateMainPath("app/"), errInvalidMainPath) + require.ErrorIs(t, validateMainPath("/src/"), errInvalidMainPath) + require.ErrorIs(t, validateMainPath("/src/app"), errInvalidMainPath) + require.ErrorIs(t, validateMainPath("./testdata/app/main.go"), errInvalidMainPath) + + // testing with real context + ctxOk := testctx.NewWithCfg(config.Project{ Builds: []config.Build{ { ID: "foo", Main: "./...", }, }, - Kos: someKoConfig, + Kos: []config.Ko{ + { + ID: "default", + Build: "foo", + Repository: "fakerepo", + }, + }, }) - require.NoError(t, Pipe{}.Default(ctx)) + require.NoError(t, Pipe{}.Default(ctxOk)) - ctx = testctx.NewWithCfg(config.Project{ + ctxWithInvalidMainPath := testctx.NewWithCfg(config.Project{ Builds: []config.Build{ { ID: "foo", Main: "/some/non/relative/path", }, }, - Kos: someKoConfig, - }) - require.EqualError(t, Pipe{}.Default(ctx), errInvalidMainPath.Error()) - - ctx = testctx.NewWithCfg(config.Project{ - Builds: []config.Build{ + Kos: []config.Ko{ { - ID: "foo", - Main: "./src/main.go", + ID: "default", + Build: "foo", + Repository: "fakerepo", }, }, - Kos: someKoConfig, }) - require.EqualError(t, Pipe{}.Default(ctx), errInvalidMainPath.Error()) + require.ErrorIs(t, Pipe{}.Default(ctxWithInvalidMainPath), errInvalidMainPath) } func TestPublishPipeError(t *testing.T) { From 7a2bf7561717bdde6854c3c71bf4c5ea7e21f429 Mon Sep 17 00:00:00 2001 From: Gabriel Cipriano Date: Sat, 18 Nov 2023 20:53:35 -0300 Subject: [PATCH 3/3] docs: add obs about relative path in ko main --- www/docs/customization/ko.md | 1 + 1 file changed, 1 insertion(+) diff --git a/www/docs/customization/ko.md b/www/docs/customization/ko.md index 30c1108f46c..edd03a8b01a 100644 --- a/www/docs/customization/ko.md +++ b/www/docs/customization/ko.md @@ -25,6 +25,7 @@ kos: build: build-id # Main path to build. + # It must be a relative path # # Default: build.main main: ./cmd/...