From 2f241824ff3a2664a987fe742aed2f0b56aeb9ab Mon Sep 17 00:00:00 2001 From: Fabian Lopez Date: Fri, 3 Dec 2021 10:07:37 -0500 Subject: [PATCH] feat: add flag to push image at the end of a successful build (#681) * add flag to push image at the end of a successful build * ensure that push flag is binded to viper * add unit test to build command * change build test name * add registry to build test * fix e2e test problem * fix compile error * add push fail test case * avoid pusher instanciation in absence of push flag --- client.go | 22 ++++---- cmd/build.go | 31 +++++++++++- cmd/build_test.go | 126 ++++++++++++++++++++++++++++++++++++++++++++++ cmd/root.go | 2 +- 4 files changed, 169 insertions(+), 12 deletions(-) diff --git a/client.go b/client.go index a02754f6..21b77987 100644 --- a/client.go +++ b/client.go @@ -593,15 +593,7 @@ func (c *Client) Deploy(ctx context.Context, path string) (err error) { return ErrNotBuilt } - // Push the image for the named service to the configured registry - imageDigest, err := c.pusher.Push(ctx, f) - if err != nil { - return - } - - // Record the Image Digest pushed. - f.ImageDigest = imageDigest - if err = f.Write(); err != nil { + if err = c.Push(ctx, &f); err != nil { return } @@ -715,6 +707,18 @@ func (c *Client) Emit(ctx context.Context, endpoint string) error { return c.emitter.Emit(ctx, endpoint) } +// Push the image for the named service to the configured registry +func (c *Client) Push(ctx context.Context, f *Function) (err error) { + imageDigest, err := c.pusher.Push(ctx, *f) + if err != nil { + return + } + + // Record the Image Digest pushed. + f.ImageDigest = imageDigest + return f.Write() +} + // DEFAULTS // --------- diff --git a/cmd/build.go b/cmd/build.go index a60231d7..c20d206d 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -11,6 +11,7 @@ import ( fn "knative.dev/kn-plugin-func" "knative.dev/kn-plugin-func/buildpacks" + "knative.dev/kn-plugin-func/docker" "knative.dev/kn-plugin-func/progress" ) @@ -24,11 +25,31 @@ func newBuildClient(cfg buildConfig) (*fn.Client, error) { builder := buildpacks.NewBuilder() listener := progress.New() + pusherOption := fn.WithPusher(nil) + if cfg.Push { + credentialsProvider := docker.NewCredentialsProvider( + newCredentialsCallback(), + docker.CheckAuth, + newChooseHelperCallback(), + ) + pusher, err := docker.NewPusher( + docker.WithCredentialsProvider(credentialsProvider), + docker.WithProgressListener(listener), + ) + + if err != nil { + return nil, err + } + pusher.Verbose = cfg.Verbose + pusherOption = fn.WithPusher(pusher) + } builder.Verbose = cfg.Verbose listener.Verbose = cfg.Verbose return fn.New( fn.WithBuilder(builder), + // fn.WithPusher(pusher), + pusherOption, fn.WithProgressListener(listener), fn.WithRegistry(cfg.Registry), // for image name when --image not provided fn.WithVerbose(cfg.Verbose)), nil @@ -64,7 +85,7 @@ kn func build kn func build --builder cnbs/sample-builder:bionic `, SuggestFor: []string{"biuld", "buidl", "built"}, - PreRunE: bindEnv("image", "path", "builder", "registry", "confirm"), + PreRunE: bindEnv("image", "path", "builder", "registry", "confirm", "push"), } cmd.Flags().StringP("builder", "b", "", "Buildpack builder, either an as a an image name or a mapping name.\nSpecified value is stored in func.yaml for subsequent builds.") @@ -72,6 +93,7 @@ kn func build --builder cnbs/sample-builder:bionic cmd.Flags().StringP("image", "i", "", "Full image name in the form [registry]/[namespace]/[name]:[tag] (optional). This option takes precedence over --registry (Env: $FUNC_IMAGE)") cmd.Flags().StringP("path", "p", cwd(), "Path to the project directory (Env: $FUNC_PATH)") cmd.Flags().StringP("registry", "r", "", "Registry + namespace part of the image to build, ex 'quay.io/myuser'. The full image name is automatically determined based on the local directory name. If not provided the registry will be taken from func.yaml (Env: $FUNC_REGISTRY)") + cmd.Flags().BoolP("push", "u", false, "Attempt to push the function image after being successfully built") if err := cmd.RegisterFlagCompletionFunc("builder", CompleteBuilderList); err != nil { fmt.Println("internal: error while calling RegisterFlagCompletionFunc: ", err) @@ -170,7 +192,11 @@ func runBuild(cmd *cobra.Command, _ []string, clientFn buildClientFn) (err error return err } - return client.Build(cmd.Context(), config.Path) + err = client.Build(cmd.Context(), config.Path) + if err == nil && config.Push { + err = client.Push(cmd.Context(), &function) + } + return } type buildConfig struct { @@ -206,6 +232,7 @@ func newBuildConfig() buildConfig { Verbose: viper.GetBool("verbose"), // defined on root Confirm: viper.GetBool("confirm"), Builder: viper.GetString("builder"), + Push: viper.GetBool("push"), } } diff --git a/cmd/build_test.go b/cmd/build_test.go index 199a46a2..56b733dc 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -1,9 +1,13 @@ package cmd import ( + "context" + "fmt" "io/ioutil" + "os" "testing" + "github.com/ory/viper" fn "knative.dev/kn-plugin-func" "knative.dev/kn-plugin-func/mock" ) @@ -49,3 +53,125 @@ created: 2021-01-01T00:00:00+00:00 t.Fatal("Expected error") } } + +func Test_runBuild(t *testing.T) { + tests := []struct { + name string + pushFlag bool + fileContents string + shouldBuild bool + shouldPush bool + wantErr bool + }{ + { + name: "push flag triggers push after build", + pushFlag: true, + fileContents: `name: test-func +runtime: go +created: 2009-11-10 23:00:00`, + shouldBuild: true, + shouldPush: true, + }, + { + name: "push flag with failing push", + pushFlag: true, + fileContents: `name: test-func +runtime: go +created: 2009-11-10 23:00:00`, + shouldBuild: true, + shouldPush: true, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockPusher := mock.NewPusher() + failPusher := &mock.Pusher{ + PushFn: func(f fn.Function) (string, error) { + return "", fmt.Errorf("push failed") + }, + } + mockBuilder := mock.NewBuilder() + cmd := NewBuildCmd(func(bc buildConfig) (*fn.Client, error) { + pusher := mockPusher + if tt.wantErr { + pusher = failPusher + } + return fn.New( + fn.WithBuilder(mockBuilder), + fn.WithPusher(pusher), + ), nil + }) + + tempDir, err := os.MkdirTemp("", "func-tests") + if err != nil { + t.Fatalf("temp dir couldn't be created %v", err) + } + t.Log("tempDir created:", tempDir) + t.Cleanup(func() { + os.RemoveAll(tempDir) + }) + + fullPath := tempDir + "/func.yaml" + tempFile, err := os.Create(fullPath) + if err != nil { + t.Fatalf("temp file couldn't be created %v", err) + } + _, err = tempFile.WriteString(tt.fileContents) + if err != nil { + t.Fatalf("file content was not written %v", err) + } + + cmd.SetArgs([]string{"--path=" + tempDir}) + viper.SetDefault("push", tt.pushFlag) + viper.SetDefault("registry", "docker.io/tigerteam") + + err = cmd.Execute() + if tt.wantErr != (err != nil) { + t.Errorf("Wanted error %v but actually got %v", tt.wantErr, err) + } + + if mockBuilder.BuildInvoked != tt.shouldBuild { + t.Errorf("Build execution expected: %v but was actually %v", tt.shouldBuild, mockBuilder.BuildInvoked) + } + + if tt.shouldPush != (mockPusher.PushInvoked || failPusher.PushInvoked) { + t.Errorf("Push execution expected: %v but was actually mockPusher invoked: %v failPusher invoked %v", tt.shouldPush, mockPusher.PushInvoked, failPusher.PushInvoked) + } + }) + } +} + +func Test_newBuildClient(t *testing.T) { + tests := []struct { + name string + cfg buildConfig + succeed bool + }{ + { + name: "push flag set to false avoids pusher instanciation", + cfg: buildConfig{ + Push: false, + }, + succeed: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, err := newBuildClient(tt.cfg) + if err != nil { + t.Error(err) + } + defer func() { + if r := recover(); r != nil && tt.succeed { + t.Errorf("expected function call to succeed %v, got actually %v", tt.succeed, r) + } + }() + err = client.Push(context.TODO(), &fn.Function{}) + if err != nil { + t.Error(err) + } + }) + } +} diff --git a/cmd/root.go b/cmd/root.go index 8257d971..74ddec34 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -156,7 +156,7 @@ func cwd() (cwd string) { // bindFunc which conforms to the cobra PreRunE method signature type bindFunc func(*cobra.Command, []string) error -// bindEnv returns a bindFunc that binds env vars to the namd flags. +// bindEnv returns a bindFunc that binds env vars to the named flags. func bindEnv(flags ...string) bindFunc { return func(cmd *cobra.Command, args []string) (err error) { for _, flag := range flags {