Skip to content

Commit

Permalink
fix: Allow undigested images to be deployed directly (#2390)
Browse files Browse the repository at this point in the history 
* init fix

Signed-off-by: gauron99 <[email protected]>

* dont override direct deploy tag, more tests

Signed-off-by: gauron99 <[email protected]>

* fix

Signed-off-by: gauron99 <[email protected]>

* dont validate with tagged image, fix comment

Signed-off-by: gauron99 <[email protected]>

* fix

Signed-off-by: gauron99 <[email protected]>

* simplify

Signed-off-by: gauron99 <[email protected]>

* comments

Signed-off-by: gauron99 <[email protected]>

---------

Signed-off-by: gauron99 <[email protected]>
  • Loading branch information
@gauron99
gauron99 authored Jul 16, 2024
1 parent 6a75f57 commit bda9487
Show file tree
Hide file tree
Showing 3 changed files with 172 additions and 14 deletions.
94 changes: 81 additions & 13 deletions cmd/deploy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,31 +298,38 @@ 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 {
if f, err = client.Push(cmd.Context(), f); err != nil {
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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
90 changes: 90 additions & 0 deletions cmd/deploy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion cmd/run.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down

0 comments on commit bda9487

Please sign in to comment.