From 34f237e367edab44360f71636504803a532fe92d Mon Sep 17 00:00:00 2001 From: gauron99 Date: Fri, 21 Jun 2024 09:46:34 +0200 Subject: [PATCH 01/16] init fix Signed-off-by: gauron99 --- cmd/deploy.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/deploy.go b/cmd/deploy.go index 4ce4a29c14..3c125ac637 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -677,6 +677,11 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) { return } + // TODO: gauron99 + // if _, err = isTagged(c.Image); err != nil { + // return + // } + // --build can be "auto"|true|false if c.Build != "auto" { if _, err := strconv.ParseBool(c.Build); err != nil { From a0a00e3ba9951c9b20f581350dbdb49e7e051ef0 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Sun, 23 Jun 2024 22:33:53 +0200 Subject: [PATCH 02/16] dont override direct deploy tag, more tests Signed-off-by: gauron99 --- cmd/deploy.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 3c125ac637..0c3d305b5e 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -336,6 +336,7 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return } } + fmt.Printf("DEPLOY IMAGE BEFORE WRITE: %v\n", f.Deploy.Image) // Write if err = f.Write(); err != nil { @@ -677,10 +678,9 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) { return } - // TODO: gauron99 - // if _, err = isTagged(c.Image); err != nil { - // return - // } + if _, err = isTagged(c.Image); err != nil { + return + } // --build can be "auto"|true|false if c.Build != "auto" { From 21e1b5603fbc40525ac2c57ef99e92c4f5dbdbe4 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Sun, 23 Jun 2024 22:35:17 +0200 Subject: [PATCH 03/16] fix Signed-off-by: gauron99 --- cmd/deploy.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 0c3d305b5e..0fa8f9d6be 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -336,7 +336,6 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return } } - fmt.Printf("DEPLOY IMAGE BEFORE WRITE: %v\n", f.Deploy.Image) // Write if err = f.Write(); err != nil { From 6d8b1fe2508b0ef027d2481b2049a62e04b73c44 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Sun, 23 Jun 2024 22:45:26 +0200 Subject: [PATCH 04/16] dont validate with tagged image, fix comment Signed-off-by: gauron99 --- cmd/deploy.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 0fa8f9d6be..4ce4a29c14 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -677,10 +677,6 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) { return } - if _, err = isTagged(c.Image); err != nil { - return - } - // --build can be "auto"|true|false if c.Build != "auto" { if _, err := strconv.ParseBool(c.Build); err != nil { From 0ddc30a1c14578906aa1f85d5f5baab3ec5e40a9 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Fri, 28 Jun 2024 11:52:14 +0200 Subject: [PATCH 05/16] init run fix for --image Signed-off-by: gauron99 --- cmd/run.go | 8 ++++ cmd/run_test.go | 106 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/cmd/run.go b/cmd/run.go index f99874aac9..e7bfd7b414 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -172,6 +172,14 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, clientOptions...) defer done() + f, digested, err := processImageName(f, cfg.Image) + if err != nil { + return + } + if digested { + f.Deploy.Image = cfg.Image + } + // Build // // If requesting to run via the container, build the container if it is diff --git a/cmd/run_test.go b/cmd/run_test.go index b655186870..40bce98117 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -174,3 +174,109 @@ func TestRun_Run(t *testing.T) { }) } } + +// TestRun_Images ensures that runnning 'func run' with --image +// (and additional flags) works as intended +func TestRun_Images(t *testing.T) { + tests := []struct { + name string + args []string + buildInvoked bool + runInvoked bool + + runError error + buildError error + }{ + { + name: "image with digest", + args: []string{"--image", "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"}, + runInvoked: true, + buildInvoked: false, + runError: nil, + buildError: nil, + }, + { + name: "image with tag direct deploy", + args: []string{"--image", "exampleimage:latest", "--build=false"}, + runInvoked: true, + buildInvoked: false, + runError: nil, + buildError: nil, + }, + { + name: "image without tag direct deploy", + args: []string{"--image", "exampleimage", "--build=false"}, + runInvoked: true, + buildInvoked: false, + runError: nil, + buildError: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := FromTempDirectory(t) + runner := mock.NewRunner() + + if tt.runError != nil { + runner.RunFn = func(context.Context, fn.Function, time.Duration) (*fn.Job, error) { return nil, tt.runError } + } + + builder := mock.NewBuilder() + if tt.buildError != nil { + builder.BuildFn = func(f fn.Function) error { return tt.buildError } + } + + // using a command whose client will be populated with mock + // builder and mock runner, each of which may be set to error if the + // test has an error defined. + cmd := NewRunCmd(NewTestClient( + fn.WithRunner(runner), + fn.WithBuilder(builder), + fn.WithRegistry("ghcr.com/reg"), + )) + cmd.SetArgs(tt.args) // Do not use test command args + + // set test case's function instance + _, err := fn.New().Init(fn.Function{Root: root, Runtime: "go"}) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + runErrCh := make(chan error, 1) + go func() { + t0 := tt // capture tt into closure + _, err := cmd.ExecuteContextC(ctx) + if err != nil && t0.buildError != nil { + // This is an expected error, so simply continue execution ignoring + // the error (send nil on the channel to release the parent routine + runErrCh <- nil + return + } else if err != nil { + runErrCh <- err // error not expected + return + } + + // No errors, but an error was expected: + if t0.buildError != nil { + runErrCh <- fmt.Errorf("Expected error: %v but got %v\n", t0.buildError, err) + } + + // Ensure invocations match expectations + if builder.BuildInvoked != tt.buildInvoked { + runErrCh <- fmt.Errorf("Function was expected to build is: %v but build execution was: %v", tt.buildInvoked, builder.BuildInvoked) + } + if runner.RunInvoked != tt.runInvoked { + runErrCh <- fmt.Errorf("Function was expected to run is: %v but run execution was: %v", tt.runInvoked, runner.RunInvoked) + } + + close(runErrCh) // release the waiting parent process + }() + cancel() // trigger the return of cmd.ExecuteContextC in the routine + <-ctx.Done() + if err := <-runErrCh; err != nil { // wait for completion of assertions + t.Fatal(err) + } + }) + } +} From 25dba57fb091a4b4456a330daada4fa6a0f0ea99 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Fri, 26 Jul 2024 13:04:24 +0200 Subject: [PATCH 06/16] init Signed-off-by: gauron99 --- cmd/deploy.go | 3 ++- cmd/run.go | 34 +++++++++++++++++++++++++++------- cmd/run_test.go | 13 ++++--------- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 4ce4a29c14..0c3f69307c 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -775,7 +775,8 @@ func printDeployMessages(out io.Writer, f fn.Function) { } } -// isUndigested returns true if provided image string 'v' has valid tag and false if +// isUndigested returns true if provided image string 'v' is valid ( by +// by acceptable standards -- tagged, untagged -> assuming :latest) and false if // not. It is lenient in validating - does not always throw an error, just // returning false in some scenarios. func isUndigested(v string) (validTag bool, err error) { diff --git a/cmd/run.go b/cmd/run.go index e7bfd7b414..b807c2a92b 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -172,14 +172,23 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, clientOptions...) defer done() - f, digested, err := processImageName(f, cfg.Image) + // check for digested image first + var digested bool + digested, err = isDigested(cfg.Image) if err != nil { - return - } - if digested { - f.Deploy.Image = cfg.Image + return err } + // if !digested { + // valid, err := isUndigested(cfg.Image) + // if err != nil { + // return err + // } + // if valid { + // f.Run.Image = cfg.Image //assign image to be potentially run + // } + // } + // Build // // If requesting to run via the container, build the container if it is @@ -189,8 +198,19 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { if err != nil { return err } - if f, _, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { - return err + + if digested { + // run cmd takes f.Build.Image - see newContainerConfig in docker/runner.go + // it doesnt get saved, just runtime image + f.Build.Image = cfg.Image + } else { + if f, _, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { + return err + } + } + } else { // dont run digested image without a container + if digested { + return fmt.Errorf("cannot use digested image with --container=false") } } diff --git a/cmd/run_test.go b/cmd/run_test.go index 40bce98117..ef82c4af12 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -192,24 +192,19 @@ func TestRun_Images(t *testing.T) { args: []string{"--image", "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"}, runInvoked: true, buildInvoked: false, - runError: nil, - buildError: nil, }, { name: "image with tag direct deploy", args: []string{"--image", "exampleimage:latest", "--build=false"}, runInvoked: true, buildInvoked: false, - runError: nil, - buildError: nil, }, { - name: "image without tag direct deploy", - args: []string{"--image", "exampleimage", "--build=false"}, - runInvoked: true, + name: "digested image without container should fail", + args: []string{"--container=false", "--image", "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"}, + runInvoked: false, buildInvoked: false, - runError: nil, - buildError: nil, + buildError: fmt.Errorf("cannot use digested image with --container=false"), }, } From a5617d408faf303f40eed62f834a9be38388262e Mon Sep 17 00:00:00 2001 From: gauron99 Date: Mon, 5 Aug 2024 12:52:03 +0200 Subject: [PATCH 07/16] int test, add valid untdigested images to run Signed-off-by: gauron99 --- cmd/deploy.go | 4 ++ cmd/run.go | 27 +++++----- cmd/run_test.go | 8 ++- pkg/docker/runner_int_test.go | 92 +++++++++++++++++++++++++++++++++-- 4 files changed, 114 insertions(+), 17 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 0c3f69307c..6f067adb40 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -780,6 +780,10 @@ func printDeployMessages(out io.Writer, f fn.Function) { // not. It is lenient in validating - does not always throw an error, just // returning false in some scenarios. func isUndigested(v string) (validTag bool, err error) { + if v == "" { + // specif image was not given + return + } if strings.Contains(v, "@") { // digest has been processed separately return diff --git a/cmd/run.go b/cmd/run.go index b807c2a92b..fb04a06c5d 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -172,22 +172,24 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, clientOptions...) defer done() + var ( + digested bool + validUndigested bool + justBuilt bool + ) + // check for digested image first - var digested bool digested, err = isDigested(cfg.Image) if err != nil { return err } - // if !digested { - // valid, err := isUndigested(cfg.Image) - // if err != nil { - // return err - // } - // if valid { - // f.Run.Image = cfg.Image //assign image to be potentially run - // } - // } + if !digested { + validUndigested, err = isUndigested(cfg.Image) + if err != nil { + return err + } + } // Build // @@ -204,9 +206,12 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { // it doesnt get saved, just runtime image f.Build.Image = cfg.Image } else { - if f, _, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { + if f, justBuilt, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { return err } + if !justBuilt && validUndigested { + f.Build.Image = cfg.Image + } } } else { // dont run digested image without a container if digested { diff --git a/cmd/run_test.go b/cmd/run_test.go index ef82c4af12..5ebdc80234 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -195,7 +195,7 @@ func TestRun_Images(t *testing.T) { }, { name: "image with tag direct deploy", - args: []string{"--image", "exampleimage:latest", "--build=false"}, + args: []string{"--image", "username/exampleimage:latest", "--build=false"}, runInvoked: true, buildInvoked: false, }, @@ -206,6 +206,12 @@ func TestRun_Images(t *testing.T) { buildInvoked: false, buildError: fmt.Errorf("cannot use digested image with --container=false"), }, + { + name: "image should build even with tagged image given", + args: []string{"--image", "username/exampleimage:latest"}, + runInvoked: true, + buildInvoked: true, + }, } for _, tt := range tests { diff --git a/pkg/docker/runner_int_test.go b/pkg/docker/runner_int_test.go index f159d21890..09fdd79bd9 100644 --- a/pkg/docker/runner_int_test.go +++ b/pkg/docker/runner_int_test.go @@ -24,21 +24,24 @@ import ( const displayEventImg = "gcr.io/knative-releases/knative.dev/eventing/cmd/event_display@sha256:610234e4319b767b187398085971d881956da660a4e0fab65a763e0f81881d82" +// public image from repo (author: github.com/gauron99) +const testImageWithDigest = "index.docker.io/4141gauron3268/teste-builder@sha256:4cf9eddf34f14cc274364a4ae60274301385d470de1fb91cbc6fec1227daa739" + func TestRun(t *testing.T) { root, cleanup := Mktemp(t) defer cleanup() ctx, cancel := context.WithTimeout(context.Background(), time.Minute*10) t.Cleanup(cancel) - - prePullTestImages(t) + image := displayEventImg + prePullTestImages(t, image) // No need to check for port 8080 since the runner should automatically // choose an open port, with 8080 only being the preferred first choice. // Initialize a new function (creates all artifacts on disk necessary // to perform actions such as running) - f := fn.Function{Runtime: "go", Root: root, Image: displayEventImg} + f := fn.Function{Runtime: "go", Root: root, Image: image} client := fn.New() f, err := client.Init(f) @@ -95,13 +98,92 @@ func TestRun(t *testing.T) { } } -func prePullTestImages(t *testing.T) { +// TestRunDigested ensures that passing a digested image to the runner will deploy +// that image instead of the previously built one. This test is depended on the +// specific image since its verifying the function's output. +func TestRunDigested(t *testing.T) { + root, cleanup := Mktemp(t) + defer cleanup() + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute*10) + t.Cleanup(cancel) + + image := testImageWithDigest + prePullTestImages(t, image) + + f := fn.Function{Runtime: "go", Root: root} + + client := fn.New() + f, err := client.Init(f) + if err != nil { + t.Fatal(err) + } + + // prebuild default image + f, err = client.Build(ctx, f) + + // simulate passing image from --image flag since client.Run just sets + // a timeout and simply calls runner.Run. + f.Build.Image = image + + // Run the function using a docker runner + var out, errOut bytes.Buffer + runner := docker.NewRunner(true, &out, &errOut) + j, err := runner.Run(ctx, f, fn.DefaultStartTimeout) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = j.Stop() }) + time.Sleep(time.Second * 5) + + var ( + id = "runner-my-id" + src = "runner-my-src" + typ = "runner-my-type" + ) + + event := cloudevents.NewEvent() + event.SetID(id) + event.SetSource(src) + event.SetType(typ) + + c, err := cloudevents.NewClientHTTP(cloudevents.WithTarget("http://localhost:" + j.Port)) + if err != nil { + t.Fatal(err) + } + + var httpErr *http.Result + res := c.Send(ctx, event) + if ok := errors.As(res, &httpErr); ok { + if httpErr.StatusCode < 200 || httpErr.StatusCode > 299 { + t.Fatal("non 2XX code") + } + } else { + t.Error("expected http.Result type") + } + time.Sleep(time.Second * 5) + + t.Log("out: ", out.String()) + t.Log("errOut: ", errOut.String()) + + outStr := out.String() + + if !(strings.Contains(outStr, id) && strings.Contains(outStr, src) && strings.Contains(outStr, typ)) { + t.Error("output doesn't contain invocation info") + } + + if !(strings.Contains(outStr, "testing the waters - serverside")) { + t.Error("output doesn't contain expected text") + } +} + +func prePullTestImages(t *testing.T, img string) { t.Helper() c, _, err := docker.NewClient(dockerClient.DefaultDockerHost) if err != nil { t.Fatal(err) } - resp, err := c.ImagePull(context.Background(), displayEventImg, types.ImagePullOptions{}) + resp, err := c.ImagePull(context.Background(), img, types.ImagePullOptions{}) if err != nil { t.Fatal(err) } From 0dfb1782bffe6c8f238d60bdeb3852da501483e7 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Mon, 5 Aug 2024 17:44:56 +0200 Subject: [PATCH 08/16] check images passed to runner for func run command Signed-off-by: gauron99 --- cmd/deploy.go | 2 + cmd/run_test.go | 105 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/cmd/deploy.go b/cmd/deploy.go index 6f067adb40..72eedf77fe 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -370,6 +370,8 @@ func build(cmd *cobra.Command, flag string, f fn.Function, client *fn.Client, bu if f, err = client.Build(cmd.Context(), f, buildOptions...); err != nil { return f, false, err } + } else if !build { + return f, false, nil } else if _, err = strconv.ParseBool(flag); err != nil { return f, false, fmt.Errorf("--build ($FUNC_BUILD) %q not recognized. Should be 'auto' or a truthy value such as 'true', 'false', '0', or '1'.", flag) diff --git a/cmd/run_test.go b/cmd/run_test.go index 5ebdc80234..236aff7057 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -281,3 +281,108 @@ func TestRun_Images(t *testing.T) { }) } } + +// TestRun_CorrectImage enusures that correct image gets passed through to the +// runner. +func TestRun_CorrectImage(t *testing.T) { + tests := []struct { + name string + image string + args []string + buildInvoked bool + expectError bool + }{ + { + name: "image with digest, auto build", + args: []string{"--image", "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"}, + image: "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", + buildInvoked: false, + }, + { + name: "image with tag direct deploy", + args: []string{"--image", "username/exampleimage:latest", "--build=false"}, + image: "username/exampleimage:latest", + buildInvoked: false, + }, + { + name: "digested image without container should fail", + args: []string{"--container=false", "--image", "exampleimage@sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"}, + image: "", + buildInvoked: false, + expectError: true, + }, + { + name: "image should build even with tagged image given", + args: []string{"--image", "username/exampleimage:latest"}, + image: "username/exampleimage:latest", + buildInvoked: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := FromTempDirectory(t) + runner := mock.NewRunner() + + runner.RunFn = func(_ context.Context, f fn.Function, _ time.Duration) (*fn.Job, error) { + // TODO: add if for empty image? -- should fail beforehand + if f.Build.Image != tt.image { + return nil, fmt.Errorf("Expected image: %v but got: %v", tt.image, f.Build.Image) + } + errs := make(chan error, 1) + stop := func() error { return nil } + return fn.NewJob(f, "127.0.0.1", "8080", errs, stop, false) + } + + builder := mock.NewBuilder() + if tt.expectError { + builder.BuildFn = func(f fn.Function) error { return fmt.Errorf("expected error") } + } + + cmd := NewRunCmd(NewTestClient( + fn.WithRunner(runner), + fn.WithBuilder(builder), + fn.WithRegistry("ghcr.com/reg"), + )) + cmd.SetArgs(tt.args) + + // set test case's function instance + _, err := fn.New().Init(fn.Function{Root: root, Runtime: "go"}) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + runErrCh := make(chan error, 1) + go func() { + t0 := tt // capture tt into closure + _, err := cmd.ExecuteContextC(ctx) + if err != nil && t0.expectError { + // This is an expected error, so simply continue execution ignoring + // the error (send nil on the channel to release the parent routine + runErrCh <- nil + return + } else if err != nil { + runErrCh <- err // error not expected + return + } + + // No errors, but an error was expected: + if t0.expectError { + runErrCh <- fmt.Errorf("Expected error but got '%v'\n", err) + } + + // Ensure invocations match expectations + if builder.BuildInvoked != tt.buildInvoked { + runErrCh <- fmt.Errorf("Function was expected to build is: %v but build execution was: %v", tt.buildInvoked, builder.BuildInvoked) + } + + close(runErrCh) // release the waiting parent process + }() + cancel() // trigger the return of cmd.ExecuteContextC in the routine + <-ctx.Done() + if err := <-runErrCh; err != nil { // wait for completion of assertions + t.Fatal(err) + } + }) + } +} From ea396393912f831306822ee24614c3e78a7e3f2b Mon Sep 17 00:00:00 2001 From: gauron99 Date: Tue, 6 Aug 2024 17:28:17 +0200 Subject: [PATCH 09/16] fix build/deploy image passing bug add test Signed-off-by: gauron99 --- cmd/build.go | 2 +- cmd/deploy.go | 13 +++++------ cmd/deploy_test.go | 49 +++++++++++++++++++++++++++++++++++++++++ pkg/functions/client.go | 16 +++++++++----- pkg/oci/pusher_test.go | 4 ++-- 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/cmd/build.go b/cmd/build.go index 96f0cf60a8..aab47aa27c 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -192,7 +192,7 @@ func runBuild(cmd *cobra.Command, _ []string, newClient ClientFactory) (err erro return } if cfg.Push { - if f, err = client.Push(cmd.Context(), f); err != nil { + if f, _, err = client.Push(cmd.Context(), f); err != nil { return } } diff --git a/cmd/deploy.go b/cmd/deploy.go index 72eedf77fe..0cdd463877 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -307,7 +307,7 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { } var justBuilt bool - + var justPushed bool // If user provided --image with digest, they are requesting that specific // image to be used which means building phase should be skipped and image // should be deployed as is @@ -319,19 +319,18 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return } if cfg.Push { - if f, err = client.Push(cmd.Context(), f); err != nil { + if f, justPushed, err = client.Push(cmd.Context(), f); err != nil { return } } // TODO: gauron99 - temporary fix for undigested image direct deploy (w/out // build) I think we will be able to remove this after we clean up the // building process - move the setting of built image in building phase? - if justBuilt && f.Build.Image != "" { + if (justBuilt || justPushed) && f.Build.Image != "" { // f.Build.Image is set in Push for now, just set it as a deployed image f.Deploy.Image = f.Build.Image } } - if f, err = client.Deploy(cmd.Context(), f, fn.WithDeploySkipBuildCheck(cfg.Build == "false")); err != nil { return } @@ -370,11 +369,11 @@ func build(cmd *cobra.Command, flag string, f fn.Function, client *fn.Client, bu if f, err = client.Build(cmd.Context(), f, buildOptions...); err != nil { return f, false, err } - } else if !build { - return f, false, nil } else if _, err = strconv.ParseBool(flag); err != nil { return f, false, fmt.Errorf("--build ($FUNC_BUILD) %q not recognized. Should be 'auto' or a truthy value such as 'true', 'false', '0', or '1'.", flag) - + } else if !build { + fmt.Printf("didnt currently build anything but no errors either\n") + return f, false, nil } return f, true, nil } diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index b75587a625..fb3290074f 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -2017,3 +2017,52 @@ func TestDeploy_WithoutHome(t *testing.T) { t.Fatal(err) } } + +func TestDeploy_AfterBuild(t *testing.T) { + var ( + root = FromTempDirectory(t) + ns = "myns" + ) + f := fn.Function{ + Runtime: "go", + Root: root, + } + _, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } + // prebuild function + cmd := NewBuildCmd(NewTestClient( + fn.WithBuilder(mock.NewBuilder()), + fn.WithRegistry(TestRegistry))) + + if err = cmd.Execute(); err != nil { + t.Fatal(err) + } + + deployer := mock.NewDeployer() + deployer.DeployFn = func(_ context.Context, f fn.Function) (result fn.DeploymentResult, err error) { + fmt.Printf("IMAGE IN DEPLOYER IS %v\n", f.Deploy.Image) + if f.Deploy.Image == "" { + return fn.DeploymentResult{}, fmt.Errorf("image was not set for deployer") + } + if f.Namespace != "" { + result.Namespace = f.Namespace // deployed to that requested + } else if f.Deploy.Namespace != "" { + result.Namespace = f.Deploy.Namespace // redeploy to current + } else { + err = errors.New("namespace required for initial deployment") + } + return + } + + // Deploy the function + cmd = NewDeployCmd(NewTestClient( + fn.WithDeployer(deployer), + fn.WithRegistry(TestRegistry))) + cmd.SetArgs([]string{fmt.Sprintf("--namespace=%s", ns), "--build=false"}) + + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } +} diff --git a/pkg/functions/client.go b/pkg/functions/client.go index 6968818c2a..1caeb685f7 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -456,7 +456,7 @@ func (c *Client) Update(ctx context.Context, f Function) (string, Function, erro if f, err = c.Build(ctx, f); err != nil { return "", f, err } - if f, err = c.Push(ctx, f); err != nil { + if f, _, err = c.Push(ctx, f); err != nil { return "", f, err } @@ -505,7 +505,7 @@ func (c *Client) New(ctx context.Context, cfg Function) (string, Function, error // Push the produced function image fmt.Fprintf(os.Stderr, "Pushing container image to registry\n") - if f, err = c.Push(ctx, f); err != nil { + if f, _, err = c.Push(ctx, f); err != nil { return route, f, err } @@ -739,6 +739,8 @@ func WithDeploySkipBuildCheck(skipBuiltCheck bool) DeployOption { // Errors if the function has not been built unless explicitly instructed // to ignore this build check. func (c *Client) Deploy(ctx context.Context, f Function, oo ...DeployOption) (Function, error) { + fmt.Fprintf(os.Stderr, ">> deploying image: %s\n", f.Deploy.Image) + options := &DeployOptions{} for _, o := range oo { o(options) @@ -1071,15 +1073,17 @@ func (c *Client) Invoke(ctx context.Context, root string, target string, m Invok } // Push the image for the named service to the configured registry -func (c *Client) Push(ctx context.Context, f Function) (Function, error) { +// returns in this order: 1)Function structure 2)bool indicating if push succeeded +// 3) error +func (c *Client) Push(ctx context.Context, f Function) (Function, bool, error) { if !f.Built() { - return f, ErrNotBuilt + return f, false, ErrNotBuilt } var err error imageDigest, err := c.pusher.Push(ctx, f) if err != nil { - return f, err + return f, false, err } // TODO: gauron99 - this is here because of a temporary workaround. @@ -1089,7 +1093,7 @@ func (c *Client) Push(ctx context.Context, f Function) (Function, error) { // the full image name and its digest right after building f.Build.Image = f.ImageNameWithDigest(imageDigest) - return f, err + return f, true, err } // ensureRunDataDir creates a .func directory at the given path, and diff --git a/pkg/oci/pusher_test.go b/pkg/oci/pusher_test.go index 4f92e83557..b6a71c1881 100644 --- a/pkg/oci/pusher_test.go +++ b/pkg/oci/pusher_test.go @@ -74,7 +74,7 @@ func TestPusher_Push(t *testing.T) { t.Fatal(err) } - if _, err = client.Push(context.Background(), f); err != nil { + if _, _, err = client.Push(context.Background(), f); err != nil { t.Fatal(err) } @@ -152,7 +152,7 @@ func TestPusher_BasicAuth(t *testing.T) { ctx = context.WithValue(ctx, fn.PushUsernameKey{}, username) ctx = context.WithValue(ctx, fn.PushPasswordKey{}, password) - if _, err = client.Push(ctx, f); err != nil { + if _, _, err = client.Push(ctx, f); err != nil { t.Fatal(err) } From 9d20af79a89d3199b8454fdf84acb3f16cba0177 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Wed, 7 Aug 2024 09:43:48 +0200 Subject: [PATCH 10/16] fix Signed-off-by: gauron99 --- pkg/functions/client_int_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/functions/client_int_test.go b/pkg/functions/client_int_test.go index acb538f9ca..47ade0755d 100644 --- a/pkg/functions/client_int_test.go +++ b/pkg/functions/client_int_test.go @@ -124,7 +124,7 @@ func TestDeploy_Defaults(t *testing.T) { if f, err = client.Build(context.Background(), f); err != nil { t.Fatal(err) } - if f, err = client.Push(context.Background(), f); err != nil { + if f, _, err = client.Push(context.Background(), f); err != nil { t.Fatal(err) } From d094e349c41ab031d295464a08b92bc4b25e3b39 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Wed, 7 Aug 2024 14:01:43 +0200 Subject: [PATCH 11/16] remove extra printing Signed-off-by: gauron99 --- cmd/deploy.go | 1 - cmd/deploy_test.go | 1 - pkg/functions/client.go | 1 - 3 files changed, 3 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 0cdd463877..666aaeee1d 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -372,7 +372,6 @@ func build(cmd *cobra.Command, flag string, f fn.Function, client *fn.Client, bu } else if _, err = strconv.ParseBool(flag); err != nil { return f, false, fmt.Errorf("--build ($FUNC_BUILD) %q not recognized. Should be 'auto' or a truthy value such as 'true', 'false', '0', or '1'.", flag) } else if !build { - fmt.Printf("didnt currently build anything but no errors either\n") return f, false, nil } return f, true, nil diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index fb3290074f..e58f435c5a 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -2042,7 +2042,6 @@ func TestDeploy_AfterBuild(t *testing.T) { deployer := mock.NewDeployer() deployer.DeployFn = func(_ context.Context, f fn.Function) (result fn.DeploymentResult, err error) { - fmt.Printf("IMAGE IN DEPLOYER IS %v\n", f.Deploy.Image) if f.Deploy.Image == "" { return fn.DeploymentResult{}, fmt.Errorf("image was not set for deployer") } diff --git a/pkg/functions/client.go b/pkg/functions/client.go index 1caeb685f7..2542739448 100644 --- a/pkg/functions/client.go +++ b/pkg/functions/client.go @@ -739,7 +739,6 @@ func WithDeploySkipBuildCheck(skipBuiltCheck bool) DeployOption { // Errors if the function has not been built unless explicitly instructed // to ignore this build check. func (c *Client) Deploy(ctx context.Context, f Function, oo ...DeployOption) (Function, error) { - fmt.Fprintf(os.Stderr, ">> deploying image: %s\n", f.Deploy.Image) options := &DeployOptions{} for _, o := range oo { From 82f7459fb5008888c2ceca507d071eee12029370 Mon Sep 17 00:00:00 2001 From: gauron99 Date: Fri, 16 Aug 2024 12:33:08 +0200 Subject: [PATCH 12/16] merge functions to digested Signed-off-by: gauron99 --- cmd/deploy.go | 112 ++++++++++++++++++-------------- cmd/deploy_test.go | 155 +++++++++++++++++++++++++++++++++------------ cmd/run.go | 19 ++---- 3 files changed, 186 insertions(+), 100 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 666aaeee1d..7b9496e65f 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -326,11 +326,14 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { // TODO: gauron99 - temporary fix for undigested image direct deploy (w/out // build) I think we will be able to remove this after we clean up the // building process - move the setting of built image in building phase? + fmt.Printf("justBuilt '%v'; justPushed '%v'\n", justBuilt, justPushed) + fmt.Printf("builtImage '%v'; deployimage '%v'\n", f.Build.Image, f.Deploy.Image) if (justBuilt || justPushed) && f.Build.Image != "" { // f.Build.Image is set in Push for now, just set it as a deployed image f.Deploy.Image = f.Build.Image } } + fmt.Printf("builtImage '%v'; deployimage '%v'\n", f.Build.Image, f.Deploy.Image) if f, err = client.Deploy(cmd.Context(), f, fn.WithDeploySkipBuildCheck(cfg.Build == "false")); err != nil { return } @@ -671,10 +674,11 @@ func (c deployConfig) Validate(cmd *cobra.Command) (err error) { } // Check Image Digest was included - // (will be set on the function during .Configure) var digest bool - if digest, err = isDigested(c.Image); err != nil { - return + if c.Image != "" { + if digest, err = isDigested(c.Image); err != nil { + return + } } // --build can be "auto"|true|false @@ -779,45 +783,69 @@ func printDeployMessages(out io.Writer, f fn.Function) { // by acceptable standards -- tagged, untagged -> assuming :latest) and false if // not. It is lenient in validating - does not always throw an error, just // returning false in some scenarios. -func isUndigested(v string) (validTag bool, err error) { - if v == "" { - // specif image was not given - return - } - if strings.Contains(v, "@") { - // digest has been processed separately - return - } - vv := strings.Split(v, ":") - if len(vv) < 2 { - // assume user knows what hes doing - validTag = true - return - } else if len(vv) > 2 { - err = fmt.Errorf("image '%v' contains an invalid tag (extra ':')", v) - return - } - tag := vv[1] - if tag == "" { - err = fmt.Errorf("image '%v' has an empty tag", v) - return - } - - validTag = true - return -} +// func isUndigested(v string) (validTag bool, err error) { +// if v == "" { +// // specif image was not given +// err = fmt.Errorf("provided image is emtpy") +// return +// } +// if strings.Contains(v, "@") { +// // digest has been processed separately +// return +// } +// vv := strings.Split(v, ":") +// if len(vv) < 2 { +// // assume user knows what hes doing +// validTag = true +// return +// } else if len(vv) > 2 { +// err = fmt.Errorf("image '%v' contains an invalid tag (extra ':')", v) +// return +// } +// tag := vv[1] +// if tag == "" { +// err = fmt.Errorf("image '%v' has an empty tag", v) +// return +// } + +// validTag = true +// return +// } // isDigested returns true if provided image string 'v' has digest and false if not. // Includes basic validation that a provided digest is correctly formatted. +// Given that image is not digested, image will still be validated and return +// a combination of bool (img has valid digest) and err (img is in valid format) +// Therefore returned combination of [false,nil] means "valid undigested image". func isDigested(v string) (validDigest bool, err error) { var digest string vv := strings.Split(v, "@") if len(vv) < 2 { - return // has no digest + // image does NOT have a digest, validate further + // if v == "" { + // err = fmt.Errorf("provided image is empty, cannot validate") + // return + // } + vvv := strings.Split(v, ":") + if len(vvv) < 2 { + // assume user knows what hes doing + return + } else if len(vvv) > 2 { + err = fmt.Errorf("image '%v' contains an invalid tag (extra ':')", v) + return + } + tag := vvv[1] + if tag == "" { + err = fmt.Errorf("image '%v' has an empty tag", v) + return + } + return } else if len(vv) > 2 { + // image is invalid err = fmt.Errorf("image '%v' contains an invalid digest (extra '@')", v) return } + // image has a digest, validate further digest = vv[1] if !strings.HasPrefix(digest, "sha256:") { @@ -843,24 +871,14 @@ func processImageName(fin fn.Function, configImage string) (f fn.Function, diges // check if --image was provided with a digest. 'digested' bool indicates if // image contains a digest or not (image is "digested"). digested, err = isDigested(configImage) - if err != nil { - return - } - // if image is digested, no need to process further - if digested { + // image is digested, no need to process further || error occurred + if digested || err != nil { return } - // digested = false here - // valid image can be with/without a tag and might be/not be built next - valid, err := isUndigested(configImage) - if err != nil { - return - } - if valid { - // this can be overridden when build&push=enabled with freshly built - // (digested) image OR directly deployed when build&push=disabled - f.Deploy.Image = configImage - } + // assign valid, undigested image as deployed image before any other changes. + // This can be overridden when build&push=enabled with freshly built image + // OR directly deployed when build&push=disabled as is. + f.Deploy.Image = configImage return } diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index e58f435c5a..50d49d28a6 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -2018,50 +2018,125 @@ func TestDeploy_WithoutHome(t *testing.T) { } } -func TestDeploy_AfterBuild(t *testing.T) { - var ( - root = FromTempDirectory(t) - ns = "myns" - ) - f := fn.Function{ - Runtime: "go", - Root: root, - } - _, err := fn.New().Init(f) - if err != nil { - t.Fatal(err) +// TestDeploy_CorrectImageDeployed ensures that deploying will always pass +// the correct image name to the deployer (populating the f.Deploy.Image value) +// in various scenarios. +func TestDeploy_CorrectImageDeployed(t *testing.T) { + const sha = "sha256:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + // dataset + tests := []struct { + name string + image string + buildArgs []string + deployArgs []string + shouldFail bool + shouldBuild bool + pusherActive bool + }{ + { + name: "basic test to create and deploy", + image: "myimage", + deployArgs: []string{"--image", "myimage"}, + }, + { + name: "test to deploy with prebuild", + image: "myimage", + buildArgs: []string{ + "--image=myimage", + }, + deployArgs: []string{ + "--build=false", + }, + shouldBuild: true, + }, + { + name: "test to build and deploy", + image: "myimage", + buildArgs: []string{ + "--image=myimage", + }, + shouldBuild: true, + }, + { + name: "test to deploy without build should fail", + image: "myimage", + deployArgs: []string{ + "--build=false", + }, + shouldFail: true, + }, + { + name: "test to build then deploy with push", + image: "myimage" + "@" + sha, + buildArgs: []string{ + "--image=myimage", + }, + deployArgs: []string{ + "--build=false", + "--push=true", + }, + shouldBuild: true, + pusherActive: true, + }, } - // prebuild function - cmd := NewBuildCmd(NewTestClient( - fn.WithBuilder(mock.NewBuilder()), - fn.WithRegistry(TestRegistry))) - if err = cmd.Execute(); err != nil { - t.Fatal(err) - } + // run tests + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := FromTempDirectory(t) + f := fn.Function{ + Runtime: "go", + Root: root, + } + _, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } - deployer := mock.NewDeployer() - deployer.DeployFn = func(_ context.Context, f fn.Function) (result fn.DeploymentResult, err error) { - if f.Deploy.Image == "" { - return fn.DeploymentResult{}, fmt.Errorf("image was not set for deployer") - } - if f.Namespace != "" { - result.Namespace = f.Namespace // deployed to that requested - } else if f.Deploy.Namespace != "" { - result.Namespace = f.Deploy.Namespace // redeploy to current - } else { - err = errors.New("namespace required for initial deployment") - } - return - } + // prebuild function if desired + if tt.shouldBuild { + cmd := NewBuildCmd(NewTestClient(fn.WithRegistry(TestRegistry))) + cmd.SetArgs(tt.buildArgs) + if err = cmd.Execute(); err != nil { + t.Fatal(err) + } + } - // Deploy the function - cmd = NewDeployCmd(NewTestClient( - fn.WithDeployer(deployer), - fn.WithRegistry(TestRegistry))) - cmd.SetArgs([]string{fmt.Sprintf("--namespace=%s", ns), "--build=false"}) + pusher := mock.NewPusher() + if tt.pusherActive { + pusher.PushFn = func(_ context.Context, _ fn.Function) (string, error) { + return sha, nil + } + } - if err := cmd.Execute(); err != nil { - t.Fatal(err) + deployer := mock.NewDeployer() + deployer.DeployFn = func(_ context.Context, f fn.Function) (result fn.DeploymentResult, err error) { + // verify the image passed to the deployer + if f.Deploy.Image != tt.image { + return fn.DeploymentResult{}, fmt.Errorf("image '%v' does not match the expected image '%v'\n", f.Deploy.Image, tt.image) + } + return + } + + // Deploy the function + cmd := NewDeployCmd(NewTestClient( + fn.WithDeployer(deployer), //is always specified + fn.WithPusher(pusher))) // if specified, will return sha for testing + + cmd.SetArgs(tt.deployArgs) + + // assert + err = cmd.Execute() + if tt.shouldFail { + if err == nil { + t.Fatal("expected an error but got none") + } + } else { + // should not fail + if err != nil { + t.Fatal(err) + } + } + }) } } diff --git a/cmd/run.go b/cmd/run.go index fb04a06c5d..ff9939ad29 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -173,24 +173,17 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { defer done() var ( - digested bool - validUndigested bool - justBuilt bool + digested bool + justBuilt bool ) - // check for digested image first - digested, err = isDigested(cfg.Image) - if err != nil { - return err - } - - if !digested { - validUndigested, err = isUndigested(cfg.Image) + // if image was specified, check if its digested and do basic validation + if cfg.Image != "" { + digested, err = isDigested(cfg.Image) if err != nil { return err } } - // Build // // If requesting to run via the container, build the container if it is @@ -209,7 +202,7 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { if f, justBuilt, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { return err } - if !justBuilt && validUndigested { + if !justBuilt && !digested { f.Build.Image = cfg.Image } } From 298f71b633b56b7a33a88f46c72c8d71a6b58cff Mon Sep 17 00:00:00 2001 From: gauron99 Date: Fri, 16 Aug 2024 13:08:16 +0200 Subject: [PATCH 13/16] misspell Signed-off-by: gauron99 --- cmd/deploy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 7b9496e65f..d3269a750b 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -786,7 +786,7 @@ func printDeployMessages(out io.Writer, f fn.Function) { // func isUndigested(v string) (validTag bool, err error) { // if v == "" { // // specif image was not given -// err = fmt.Errorf("provided image is emtpy") +// err = fmt.Errorf("provided image is empty") // return // } // if strings.Contains(v, "@") { From eba5c03d66932bf45cb39978ae31a632547206ed Mon Sep 17 00:00:00 2001 From: David Fridrich Date: Mon, 26 Aug 2024 14:08:07 +0200 Subject: [PATCH 14/16] simplify Signed-off-by: David Fridrich --- cmd/deploy.go | 50 +++++++++++++++++++++++++++++++------------------- cmd/run.go | 34 ++++++++++++++++++++-------------- 2 files changed, 51 insertions(+), 33 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index d3269a750b..e519d2e576 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -298,16 +298,24 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return } - // Preprocess image name. Validate the image and check whether its digested - // This might alter f.Deploy.Image. - var digested bool - f, digested, err = processImageName(f, cfg.Image) - if err != nil { - return + var ( + digested bool + justBuilt bool + justPushed bool + ) + + // Validate the image and check whether its digested or not + if cfg.Image != "" { + digested, err = isDigested(cfg.Image) + if err != nil { + return + } + // image is valid and undigested + if !digested { + f.Deploy.Image = cfg.Image + } } - var justBuilt bool - var justPushed bool // If user provided --image with digest, they are requesting that specific // image to be used which means building phase should be skipped and image // should be deployed as is @@ -323,9 +331,9 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return } } - // TODO: gauron99 - temporary fix for undigested image direct deploy (w/out - // build) I think we will be able to remove this after we clean up the - // building process - move the setting of built image in building phase? + // TODO: gauron99 - temporary fix for undigested image direct deploy + // (w/out build) This might be more complex to do than leaving like this + // image digests are created via the registry on push. fmt.Printf("justBuilt '%v'; justPushed '%v'\n", justBuilt, justPushed) fmt.Printf("builtImage '%v'; deployimage '%v'\n", f.Build.Image, f.Deploy.Image) if (justBuilt || justPushed) && f.Build.Image != "" { @@ -822,10 +830,10 @@ func isDigested(v string) (validDigest bool, err error) { vv := strings.Split(v, "@") if len(vv) < 2 { // image does NOT have a digest, validate further - // if v == "" { - // err = fmt.Errorf("provided image is empty, cannot validate") - // return - // } + if v == "" { + err = fmt.Errorf("provided image is empty, cannot validate") + return + } vvv := strings.Split(v, ":") if len(vvv) < 2 { // assume user knows what hes doing @@ -866,19 +874,23 @@ func isDigested(v string) (validDigest bool, err error) { // fields of Function structure are populated if needed. // Returns a Function structure(1), bool indicating if image was given with // digest(2) and error(3) -func processImageName(fin fn.Function, configImage string) (f fn.Function, digested bool, err error) { - f = fin +func processImageName(f fn.Function, configImage string) (fn.Function, bool, error) { + var ( + digested bool + err error + ) + // check if --image was provided with a digest. 'digested' bool indicates if // image contains a digest or not (image is "digested"). digested, err = isDigested(configImage) // image is digested, no need to process further || error occurred if digested || err != nil { - return + return f, digested, err } // assign valid, undigested image as deployed image before any other changes. // This can be overridden when build&push=enabled with freshly built image // OR directly deployed when build&push=disabled as is. f.Deploy.Image = configImage - return + return f, digested, err } diff --git a/cmd/run.go b/cmd/run.go index ff9939ad29..e4a626997f 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -172,28 +172,28 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { client, done := newClient(ClientConfig{Verbose: cfg.Verbose}, clientOptions...) defer done() - var ( - digested bool - justBuilt bool - ) - - // if image was specified, check if its digested and do basic validation - if cfg.Image != "" { - digested, err = isDigested(cfg.Image) - if err != nil { - return err - } - } // Build // // If requesting to run via the container, build the container if it is // either out-of-date or a build was explicitly requested. if cfg.Container { + var ( + digested bool + justBuilt bool + ) + buildOptions, err := cfg.buildOptions() if err != nil { return err } + // if image was specified, check if its digested and do basic validation + if cfg.Image != "" { + digested, err = isDigested(cfg.Image) + if err != nil { + return err + } + } if digested { // run cmd takes f.Build.Image - see newContainerConfig in docker/runner.go // it doesnt get saved, just runtime image @@ -207,8 +207,14 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { } } } else { // dont run digested image without a container - if digested { - return fmt.Errorf("cannot use digested image with --container=false") + if cfg.Image != "" { + digested, err := isDigested(cfg.Image) + if err != nil { + return err + } + if digested { + return fmt.Errorf("cannot use digested image with --container=false") + } } } From d095a39f125e4fdbc0499b12b125a2e45e19ab63 Mon Sep 17 00:00:00 2001 From: David Fridrich Date: Tue, 27 Aug 2024 15:43:11 +0200 Subject: [PATCH 15/16] quick fix Signed-off-by: David Fridrich --- cmd/deploy.go | 26 -------------------------- cmd/run.go | 19 ++++++++++--------- 2 files changed, 10 insertions(+), 35 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index e519d2e576..822c84de83 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -868,29 +868,3 @@ func isDigested(v string) (validDigest bool, err error) { validDigest = true return } - -// processImageName processes the image name for deployment. It ensures that -// image string is validated if --image was given and ensures that proper -// fields of Function structure are populated if needed. -// Returns a Function structure(1), bool indicating if image was given with -// digest(2) and error(3) -func processImageName(f fn.Function, configImage string) (fn.Function, bool, error) { - var ( - digested bool - err error - ) - - // check if --image was provided with a digest. 'digested' bool indicates if - // image contains a digest or not (image is "digested"). - digested, err = isDigested(configImage) - // image is digested, no need to process further || error occurred - if digested || err != nil { - return f, digested, err - } - - // assign valid, undigested image as deployed image before any other changes. - // This can be overridden when build&push=enabled with freshly built image - // OR directly deployed when build&push=disabled as is. - f.Deploy.Image = configImage - return f, digested, err -} diff --git a/cmd/run.go b/cmd/run.go index e4a626997f..d8c28b5197 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -177,10 +177,7 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { // If requesting to run via the container, build the container if it is // either out-of-date or a build was explicitly requested. if cfg.Container { - var ( - digested bool - justBuilt bool - ) + var digested bool buildOptions, err := cfg.buildOptions() if err != nil { @@ -193,20 +190,24 @@ func runRun(cmd *cobra.Command, newClient ClientFactory) (err error) { if err != nil { return err } + if !digested { + // assign valid undigested image + f.Build.Image = cfg.Image + } } + if digested { // run cmd takes f.Build.Image - see newContainerConfig in docker/runner.go // it doesnt get saved, just runtime image f.Build.Image = cfg.Image } else { - if f, justBuilt, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { + + if f, _, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { return err } - if !justBuilt && !digested { - f.Build.Image = cfg.Image - } } - } else { // dont run digested image without a container + } else { + // dont run digested image without a container if cfg.Image != "" { digested, err := isDigested(cfg.Image) if err != nil { From 4c3bbb4d3b98d9b3abffb0498628c2c556d3ace1 Mon Sep 17 00:00:00 2001 From: David Fridrich Date: Tue, 27 Aug 2024 16:30:33 +0200 Subject: [PATCH 16/16] remove prints, comment Signed-off-by: David Fridrich --- cmd/deploy.go | 36 ----------------------------------- pkg/docker/runner_int_test.go | 4 ++++ 2 files changed, 4 insertions(+), 36 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 822c84de83..cd7edcf3db 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -334,14 +334,11 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { // TODO: gauron99 - temporary fix for undigested image direct deploy // (w/out build) This might be more complex to do than leaving like this // image digests are created via the registry on push. - fmt.Printf("justBuilt '%v'; justPushed '%v'\n", justBuilt, justPushed) - fmt.Printf("builtImage '%v'; deployimage '%v'\n", f.Build.Image, f.Deploy.Image) if (justBuilt || justPushed) && f.Build.Image != "" { // f.Build.Image is set in Push for now, just set it as a deployed image f.Deploy.Image = f.Build.Image } } - fmt.Printf("builtImage '%v'; deployimage '%v'\n", f.Build.Image, f.Deploy.Image) if f, err = client.Deploy(cmd.Context(), f, fn.WithDeploySkipBuildCheck(cfg.Build == "false")); err != nil { return } @@ -787,39 +784,6 @@ func printDeployMessages(out io.Writer, f fn.Function) { } } -// isUndigested returns true if provided image string 'v' is valid ( by -// by acceptable standards -- tagged, untagged -> assuming :latest) and false if -// not. It is lenient in validating - does not always throw an error, just -// returning false in some scenarios. -// func isUndigested(v string) (validTag bool, err error) { -// if v == "" { -// // specif image was not given -// err = fmt.Errorf("provided image is empty") -// return -// } -// if strings.Contains(v, "@") { -// // digest has been processed separately -// return -// } -// vv := strings.Split(v, ":") -// if len(vv) < 2 { -// // assume user knows what hes doing -// validTag = true -// return -// } else if len(vv) > 2 { -// err = fmt.Errorf("image '%v' contains an invalid tag (extra ':')", v) -// return -// } -// tag := vv[1] -// if tag == "" { -// err = fmt.Errorf("image '%v' has an empty tag", v) -// return -// } - -// validTag = true -// return -// } - // isDigested returns true if provided image string 'v' has digest and false if not. // Includes basic validation that a provided digest is correctly formatted. // Given that image is not digested, image will still be validated and return diff --git a/pkg/docker/runner_int_test.go b/pkg/docker/runner_int_test.go index 09fdd79bd9..9bdc0f10e2 100644 --- a/pkg/docker/runner_int_test.go +++ b/pkg/docker/runner_int_test.go @@ -108,6 +108,10 @@ func TestRunDigested(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Minute*10) t.Cleanup(cancel) + // TODO: gauron99 - if image-digest-on-build is implemented, rework this + // to fit this schema -- build image (get digest) then run from temporary dir + // such that its .func stamp is not considered. All of this to remove the + // external pre-built container dependency image := testImageWithDigest prePullTestImages(t, image)