From 31f812c48edf220eb71949bbc8755f624be23cd3 Mon Sep 17 00:00:00 2001 From: Halvard Skogsrud Date: Thu, 19 Aug 2021 17:38:42 +1000 Subject: [PATCH] Set build config via BuildOptions Enables programmatically overriding build configs when ko is embedded in another tool. Related: #340, #419 --- pkg/commands/options/build.go | 4 ++ pkg/commands/resolver.go | 5 +- pkg/commands/resolver_test.go | 111 +++++++++++++++++++++++----------- 3 files changed, 84 insertions(+), 36 deletions(-) diff --git a/pkg/commands/options/build.go b/pkg/commands/options/build.go index 20e32cde3f..5533655ff9 100644 --- a/pkg/commands/options/build.go +++ b/pkg/commands/options/build.go @@ -17,6 +17,7 @@ limitations under the License. package options import ( + "github.com/google/ko/pkg/build" "github.com/spf13/cobra" ) @@ -39,6 +40,9 @@ type BuildOptions struct { UserAgent string InsecureRegistry bool + + // BuildConfigs enables programmatic overriding of build config set in `.ko.yaml`. + BuildConfigs map[string]build.Config } func AddBuildOptions(cmd *cobra.Command, bo *BuildOptions) { diff --git a/pkg/commands/resolver.go b/pkg/commands/resolver.go index b2e95635f3..7f6ad811e6 100644 --- a/pkg/commands/resolver.go +++ b/pkg/commands/resolver.go @@ -106,7 +106,10 @@ func gobuildOptions(bo *options.BuildOptions) ([]build.Option, error) { opts = append(opts, build.WithLabel(parts[0], parts[1])) } - if len(buildConfigs) > 0 { + // prefer buildConfigs from BuildOptions + if bo.BuildConfigs != nil { + opts = append(opts, build.WithConfig(bo.BuildConfigs)) + } else if len(buildConfigs) > 0 { opts = append(opts, build.WithConfig(buildConfigs)) } diff --git a/pkg/commands/resolver_test.go b/pkg/commands/resolver_test.go index 53a4e67a59..5df5c91da1 100644 --- a/pkg/commands/resolver_test.go +++ b/pkg/commands/resolver_test.go @@ -156,24 +156,64 @@ kind: Bar } } -func TestNewBuilderCanBuild(t *testing.T) { - ctx := context.Background() - bo := &options.BuildOptions{ - ConcurrentBuilds: 1, - } - builder, err := NewBuilder(ctx, bo) - if err != nil { - t.Fatalf("NewBuilder(): %v", err) - } - res, err := builder.Build(ctx, "ko://github.com/google/ko/test") - if err != nil { - t.Fatalf("builder.Build(): %v", err) +// TODO This test accesses the network and is slow. Implement a dry-run mode? +func TestNewBuilder(t *testing.T) { + tests := []struct { + description string + importpath string + bo *options.BuildOptions + wantQualifiedImportpath string + shouldBuildError bool + }{ + { + description: "test app with already qualified import path", + importpath: "ko://github.com/google/ko/test", + bo: &options.BuildOptions{ + ConcurrentBuilds: 1, + }, + wantQualifiedImportpath: "ko://github.com/google/ko/test", + shouldBuildError: false, + }, + { + description: "programmatic build config", + importpath: "./test", + bo: &options.BuildOptions{ + ConcurrentBuilds: 1, + BuildConfigs: map[string]build.Config{ + "github.com/google/ko/test": { + ID: "id-can-be-anything", + Flags: []string{"-invalid-flag-should-cause-error"}, + }, + }, + WorkingDirectory: "../..", + }, + wantQualifiedImportpath: "ko://github.com/google/ko/test", + shouldBuildError: true, + }, } - gotDigest, err := res.Digest() - if err != nil { - t.Fatalf("res.Digest(): %v", err) + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + ctx := context.Background() + builder, err := NewBuilder(ctx, test.bo) + if err != nil { + t.Fatalf("NewBuilder(): %v", err) + } + qualifiedImportpath, err := builder.QualifyImport(test.importpath) + if err != nil { + t.Fatalf("builder.QualifyImport(%s): %v", test.importpath, err) + } + if qualifiedImportpath != test.wantQualifiedImportpath { + t.Fatalf("incorrect qualified import path, got %s, wanted %s", qualifiedImportpath, test.wantQualifiedImportpath) + } + _, err = builder.Build(ctx, qualifiedImportpath) + if err != nil && !test.shouldBuildError { + t.Fatalf("builder.Build(): %v", err) + } + if err == nil && test.shouldBuildError { + t.Fatalf("expected error got nil") + } + }) } - fmt.Println(gotDigest.String()) } func TestNewPublisherCanPublish(t *testing.T) { @@ -224,26 +264,27 @@ func TestNewPublisherCanPublish(t *testing.T) { }, } for _, test := range tests { - publisher, err := NewPublisher(test.po) - if err != nil { - t.Fatalf("%s: NewPublisher(): %v", test.description, err) - } - defer publisher.Close() - ref, err := publisher.Publish(context.Background(), empty.Image, build.StrictScheme+importpath) - if test.shouldError { - if err == nil || !strings.HasSuffix(err.Error(), test.wantError.Error()) { - t.Errorf("%s: got error %v, wanted %v", test.description, err, test.wantError) + t.Run(test.description, func(t *testing.T) { + publisher, err := NewPublisher(test.po) + if err != nil { + t.Fatalf("NewPublisher(): %v", err) } - continue - } - if err != nil { - t.Fatalf("%s: publisher.Publish(): %v", test.description, err) - } - fmt.Printf("ref: %+v\n", ref) - gotImageName := ref.Context().Name() - if gotImageName != test.wantImageName { - t.Errorf("%s: got %s, wanted %s", test.description, gotImageName, test.wantImageName) - } + defer publisher.Close() + ref, err := publisher.Publish(context.Background(), empty.Image, build.StrictScheme+importpath) + if test.shouldError { + if err == nil || !strings.HasSuffix(err.Error(), test.wantError.Error()) { + t.Errorf("%s: got error %v, wanted %v", test.description, err, test.wantError) + } + return + } + if err != nil { + t.Fatalf("publisher.Publish(): %v", err) + } + gotImageName := ref.Context().Name() + if gotImageName != test.wantImageName { + t.Errorf("got %s, wanted %s", gotImageName, test.wantImageName) + } + }) } }