diff --git a/cmd/deploy.go b/cmd/deploy.go index c04625d9db..4ce4a29c14 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -298,22 +298,24 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return } - // check if --image was provided with a digest. 'digested' bool indicates if - // image contains a digest or not (image is "digested"). + // Preprocess image name. Validate the image and check whether its digested + // This might alter f.Deploy.Image. var digested bool - digested, err = isDigested(cfg.Image) + f, digested, err = processImageName(f, cfg.Image) if err != nil { return } + var justBuilt 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 if digested { f.Deploy.Image = cfg.Image } else { - // if NOT digested, build and push the Function first - if f, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { + // NOT digested, build & push the Function unless specified otherwise + if f, justBuilt, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { return } if cfg.Push { @@ -321,8 +323,13 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { return } } - // f.Build.Image is set in Push for now, just set it as a deployed image - f.Deploy.Image = f.Build.Image + // 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 != "" { + // 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 { @@ -346,26 +353,28 @@ func runDeploy(cmd *cobra.Command, newClient ClientFactory) (err error) { // flag value is explicitly truthy such as 'true' or '1'. Error if flag // is neither 'auto' nor parseable as a boolean. Return CLI-specific error // message verbeage suitable for both Deploy and Run commands which feature an -// optional build step. -func build(cmd *cobra.Command, flag string, f fn.Function, client *fn.Client, buildOptions []fn.BuildOption) (fn.Function, error) { +// optional build step. Boolean return value signifies if the image has gone +// through a build process. +func build(cmd *cobra.Command, flag string, f fn.Function, client *fn.Client, buildOptions []fn.BuildOption) (fn.Function, bool, error) { var err error if flag == "auto" { if f.Built() { fmt.Fprintln(cmd.OutOrStdout(), "function up-to-date. Force rebuild with --build") + return f, false, nil } else { if f, err = client.Build(cmd.Context(), f, buildOptions...); err != nil { - return f, err + return f, false, err } } } else if build, _ := strconv.ParseBool(flag); build { if f, err = client.Build(cmd.Context(), f, buildOptions...); err != nil { - return f, err + return f, false, err } } else if _, err = strconv.ParseBool(flag); err != nil { - return f, fmt.Errorf("--build ($FUNC_BUILD) %q not recognized. Should be 'auto' or a truthy value such as 'true', 'false', '0', or '1'.", flag) + 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) } - return f, nil + return f, true, nil } func NewRegistryValidator(path string) survey.Validator { @@ -766,6 +775,33 @@ func printDeployMessages(out io.Writer, f fn.Function) { } } +// isUndigested returns true if provided image string 'v' has valid tag 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 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. func isDigested(v string) (validDigest bool, err error) { @@ -791,3 +827,35 @@ 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(fin fn.Function, configImage string) (f fn.Function, digested bool, err error) { + f = fin + // 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 { + 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 + } + return +} diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 454aa76fd8..b75587a625 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -866,6 +866,96 @@ func TestDeploy_ImageWithDigestDoesntPopulateBuild(t *testing.T) { } } +// TestDeploy_WithoutDigest ensures that images specified with --image and +// without digest are correctly processed and propagated to .Deploy.Image +func TestDeploy_WithoutDigest(t *testing.T) { + const sha = "sha256:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + + tests := []struct { + name string //name of the test + args []string //args to the deploy command + expectedImage string //expected value of .Deploy.Image + expectedToBuild bool //expected build invocation + }{ + { + name: "defaults with image flag", + args: []string{"--image=image.example.com/alice/f:latest"}, + expectedImage: "image.example.com/alice/f" + "@" + sha, + expectedToBuild: true, + }, + { + name: "direct deploy with image flag", + args: []string{"--build=false", "--push=false", "--image=image.example.com/bob/f:latest"}, + expectedImage: "image.example.com/bob/f:latest", + expectedToBuild: false, + }, + { + name: "tagged image, push disabled", + args: []string{"--push=false", "--image=image.example.com/clarance/f:latest"}, + expectedImage: "image.example.com/clarance/f:latest", + expectedToBuild: true, + }, + { + name: "untagged image w/ build & push", + args: []string{"--image=image.example.com/dominik/f"}, + expectedImage: "image.example.com/dominik/f" + "@" + sha, + expectedToBuild: true, + }, + { + name: "untagged image w/out build & push", + args: []string{"--build=false", "--push=false", "--image=image.example.com/enrique/f"}, + expectedImage: "image.example.com/enrique/f", + expectedToBuild: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // ASSEMBLE + var ( + root = FromTempDirectory(t) + deployer = mock.NewDeployer() + builder = mock.NewBuilder() + pusher = mock.NewPusher() + f = fn.Function{} + ) + + f.Name = "func" + f.Runtime = "go" + f, err := fn.New().Init(f) + if err != nil { + t.Fatal(err) + } + + // TODO: gauron99 this will be changed once we get image name+digest in + // building phase + // simulate current implementation of pusher where full image is concatenated + pusher.PushFn = func(ctx context.Context, _ fn.Function) (string, error) { + return sha, nil + } + + // ACT + cmd := NewDeployCmd(NewTestClient(fn.WithDeployer(deployer), fn.WithBuilder(builder), fn.WithPusher(pusher))) + cmd.SetArgs(test.args) + if err := cmd.Execute(); err != nil { + t.Fatal(err) + } + f, err = fn.NewFunction(root) + if err != nil { + t.Fatal(err) + } + + // ASSERT + if builder.BuildInvoked != test.expectedToBuild { + t.Fatalf("expected builder invocation: '%v', got '%v'", test.expectedToBuild, builder.BuildInvoked) + } + + if f.Deploy.Image != test.expectedImage { + t.Fatalf("expected deployed image '%v', got '%v'", test.expectedImage, f.Deploy.Image) + } + }) + } +} + // TestDepoy_InvalidRegistry ensures that providing an invalid registry // fails with the expected error. func TestDeploy_InvalidRegistry(t *testing.T) { diff --git a/cmd/run.go b/cmd/run.go index 4b67eb2c4d..f99874aac9 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -181,7 +181,7 @@ 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 { + if f, _, err = build(cmd, cfg.Build, f, client, buildOptions); err != nil { return err } }