From 0bbfe40626933bad104b998e5ff5367eb32ab49b Mon Sep 17 00:00:00 2001 From: Benjamin Somers Date: Mon, 9 Dec 2024 08:07:48 +0000 Subject: [PATCH 1/5] incusd/scriptlet: Rename prefixAuthorization to nameAuthorization Signed-off-by: Benjamin Somers --- internal/server/scriptlet/load/load.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/server/scriptlet/load/load.go b/internal/server/scriptlet/load/load.go index e27f1b6ec7c..5f4a2d9dc33 100644 --- a/internal/server/scriptlet/load/load.go +++ b/internal/server/scriptlet/load/load.go @@ -15,8 +15,8 @@ const nameInstancePlacement = "instance_placement" // prefixQEMU is the prefix used in Starlark for the QEMU scriptlet. const prefixQEMU = "qemu" -// prefixAuthorization is the prefix used in Starlark for the Authorization scriptlet. -const prefixAuthorization = "authorization" +// nameAuthorization is the name used in Starlark for the Authorization scriptlet. +const nameAuthorization = "authorization" // compile compiles a scriptlet. func compile(programName string, src string, preDeclared []string) (*starlark.Program, error) { @@ -157,17 +157,17 @@ func AuthorizationCompile(name string, src string) (*starlark.Program, error) { // AuthorizationValidate validates the authorization scriptlet. func AuthorizationValidate(src string) error { - _, err := AuthorizationCompile(prefixAuthorization, src) + _, err := AuthorizationCompile(nameAuthorization, src) return err } // AuthorizationSet compiles the authorization scriptlet into memory for use with AuthorizationRun. // If empty src is provided the current program is deleted. func AuthorizationSet(src string) error { - return set(AuthorizationCompile, prefixAuthorization, src) + return set(AuthorizationCompile, nameAuthorization, src) } // AuthorizationProgram returns the precompiled authorization scriptlet program. func AuthorizationProgram() (*starlark.Program, *starlark.Thread, error) { - return program("Authorization", prefixAuthorization) + return program("Authorization", nameAuthorization) } From 39b3d8b8bd70ec9f08a53b0080e2dff0f4ec3f3e Mon Sep 17 00:00:00 2001 From: Benjamin Somers Date: Mon, 9 Dec 2024 09:23:04 +0000 Subject: [PATCH 2/5] incusd/scriptlet: Add function checks in scriptlet validation Signed-off-by: Benjamin Somers --- internal/server/scriptlet/load/load.go | 56 +++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/internal/server/scriptlet/load/load.go b/internal/server/scriptlet/load/load.go index 5f4a2d9dc33..c766353561a 100644 --- a/internal/server/scriptlet/load/load.go +++ b/internal/server/scriptlet/load/load.go @@ -33,6 +33,47 @@ func compile(programName string, src string, preDeclared []string) (*starlark.Pr return mod, nil } +// validate validates a scriptlet by compiling it and checking the presence of required functions. +func validate(compiler func(string, string) (*starlark.Program, error), programName string, src string, requiredFunctions []string) error { + prog, err := compiler(programName, src) + if err != nil { + return err + } + + thread := &starlark.Thread{Name: programName} + globals, err := prog.Init(thread, nil) + if err != nil { + return err + } + + globals.Freeze() + + var notFound []string + for _, funName := range requiredFunctions { + // The function is missing if its name is not found in the globals. + requiredFun := globals[funName] + if requiredFun == nil { + notFound = append(notFound, funName) + continue + } + + // The function is missing if its name is not bound to a function. + _, ok := requiredFun.(*starlark.Function) + if !ok { + notFound = append(notFound, funName) + } + } + + switch len(notFound) { + case 0: + return nil + case 1: + return fmt.Errorf("The function %q is required but has not been found in the scriptlet", notFound[0]) + default: + return fmt.Errorf("The functions %q are required but have not been found in the scriptlet", notFound) + } +} + var programsMu sync.Mutex var programs = make(map[string]*starlark.Program) @@ -89,8 +130,9 @@ func InstancePlacementCompile(name string, src string) (*starlark.Program, error // InstancePlacementValidate validates the instance placement scriptlet. func InstancePlacementValidate(src string) error { - _, err := InstancePlacementCompile(nameInstancePlacement, src) - return err + return validate(InstancePlacementCompile, nameInstancePlacement, src, []string{ + "instance_placement", + }) } // InstancePlacementSet compiles the instance placement scriptlet into memory for use with InstancePlacementRun. @@ -131,8 +173,9 @@ func QEMUCompile(name string, src string) (*starlark.Program, error) { // QEMUValidate validates the QEMU scriptlet. func QEMUValidate(src string) error { - _, err := QEMUCompile(prefixQEMU, src) - return err + return validate(QEMUCompile, prefixQEMU, src, []string{ + "qemu_hook", + }) } // QEMUSet compiles the QEMU scriptlet into memory for use with QEMURun. @@ -157,8 +200,9 @@ func AuthorizationCompile(name string, src string) (*starlark.Program, error) { // AuthorizationValidate validates the authorization scriptlet. func AuthorizationValidate(src string) error { - _, err := AuthorizationCompile(nameAuthorization, src) - return err + return validate(AuthorizationCompile, nameAuthorization, src, []string{ + "authorize", + }) } // AuthorizationSet compiles the authorization scriptlet into memory for use with AuthorizationRun. From 3d1482545724af08ea78c472fd8d6988c99d4a79 Mon Sep 17 00:00:00 2001 From: Dan Biagini Date: Sat, 7 Dec 2024 15:25:14 +0000 Subject: [PATCH 3/5] client/oci: Add debug logging for subprocess commands Signed-off-by: Dan Biagini --- client/oci_images.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/client/oci_images.go b/client/oci_images.go index 1e603bb5338..3a5316365c1 100644 --- a/client/oci_images.go +++ b/client/oci_images.go @@ -14,6 +14,7 @@ import ( "github.com/lxc/incus/v6/shared/api" "github.com/lxc/incus/v6/shared/ioprogress" + "github.com/lxc/incus/v6/shared/logger" "github.com/lxc/incus/v6/shared/osarch" "github.com/lxc/incus/v6/shared/subprocess" "github.com/lxc/incus/v6/shared/units" @@ -145,13 +146,14 @@ func (r *ProtocolOCI) GetImageFile(fingerprint string, req ImageFileRequest) (*I req.ProgressHandler(ioprogress.ProgressData{Text: "Retrieving OCI image from registry"}) } - _, err = subprocess.RunCommand( + stdout, err := subprocess.RunCommand( "skopeo", "--insecure-policy", "copy", fmt.Sprintf("%s/%s", strings.Replace(r.httpHost, "https://", "docker://", -1), info.Alias), fmt.Sprintf("oci:%s:latest", filepath.Join(ociPath, "oci"))) if err != nil { + logger.Debug("Error copying remote image to local", logger.Ctx{"image": info.Alias, "stdout": stdout, "stderr": err}) return nil, err } @@ -160,13 +162,14 @@ func (r *ProtocolOCI) GetImageFile(fingerprint string, req ImageFileRequest) (*I req.ProgressHandler(ioprogress.ProgressData{Text: "Unpacking the OCI image"}) } - _, err = subprocess.RunCommand( + stdout, err = subprocess.RunCommand( "umoci", "unpack", "--keep-dirlinks", "--image", filepath.Join(ociPath, "oci"), filepath.Join(ociPath, "image")) if err != nil { + logger.Debug("Error unpacking OCI image", logger.Ctx{"image": filepath.Join(ociPath, "oci"), "stdout": stdout, "stderr": err}) return nil, err } @@ -305,6 +308,7 @@ func (r *ProtocolOCI) GetImageAlias(name string) (*api.ImageAliasesEntry, string // Get the image information from skopeo. stdout, err := subprocess.RunCommand("skopeo", "inspect", fmt.Sprintf("%s/%s", strings.Replace(r.httpHost, "https://", "docker://", -1), name)) if err != nil { + logger.Debug("Error getting image alias", logger.Ctx{"name": name, "stdout": stdout, "stderr": err}) return nil, "", err } From 9583797caeae0302a7cd8d02a4cfab6a40593be4 Mon Sep 17 00:00:00 2001 From: Dan Biagini Date: Sat, 7 Dec 2024 15:39:19 +0000 Subject: [PATCH 4/5] incusd/daemon_images: Fix error string typo for OCI connect errors Signed-off-by: Dan Biagini --- cmd/incusd/daemon_images.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/incusd/daemon_images.go b/cmd/incusd/daemon_images.go index 5795c23cb97..ab159643447 100644 --- a/cmd/incusd/daemon_images.go +++ b/cmd/incusd/daemon_images.go @@ -102,7 +102,7 @@ func ImageDownload(ctx context.Context, r *http.Request, s *state.State, op *ope // Setup OCI client remote, err = incus.ConnectOCI(args.Server, clientArgs) if err != nil { - return nil, false, fmt.Errorf("Failed to connect to simple streams server %q: %w", args.Server, err) + return nil, false, fmt.Errorf("Failed to connect to oci server %q: %w", args.Server, err) } } else if protocol == "simplestreams" { // Setup simplestreams client @@ -114,7 +114,7 @@ func ImageDownload(ctx context.Context, r *http.Request, s *state.State, op *ope // For public images, handle aliases and initial metadata if args.Secret == "" { - // Look for a matching alias + // Look for a matching alias. Note, this err message is lost! entry, _, err := remote.GetImageAliasType(args.Type, fp) if err == nil { fp = entry.Target From 5b902713b8116a27b59f67cb91993cc0c68a6712 Mon Sep 17 00:00:00 2001 From: Benjamin Somers Date: Mon, 9 Dec 2024 09:49:05 +0000 Subject: [PATCH 5/5] incusd/scriptlet: Add function args checks in scriptlet validation Signed-off-by: Benjamin Somers --- internal/server/scriptlet/load/load.go | 48 ++++++++++++++++++++------ 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/internal/server/scriptlet/load/load.go b/internal/server/scriptlet/load/load.go index c766353561a..3551bdb0841 100644 --- a/internal/server/scriptlet/load/load.go +++ b/internal/server/scriptlet/load/load.go @@ -3,6 +3,7 @@ package load import ( "fmt" "slices" + "sort" "sync" "go.starlark.net/starlark" @@ -34,7 +35,7 @@ func compile(programName string, src string, preDeclared []string) (*starlark.Pr } // validate validates a scriptlet by compiling it and checking the presence of required functions. -func validate(compiler func(string, string) (*starlark.Program, error), programName string, src string, requiredFunctions []string) error { +func validate(compiler func(string, string) (*starlark.Program, error), programName string, src string, requiredFunctions map[string][]string) error { prog, err := compiler(programName, src) if err != nil { return err @@ -49,19 +50,44 @@ func validate(compiler func(string, string) (*starlark.Program, error), programN globals.Freeze() var notFound []string - for _, funName := range requiredFunctions { + for funName, requiredArgs := range requiredFunctions { // The function is missing if its name is not found in the globals. - requiredFun := globals[funName] - if requiredFun == nil { + funv := globals[funName] + if funv == nil { notFound = append(notFound, funName) continue } // The function is missing if its name is not bound to a function. - _, ok := requiredFun.(*starlark.Function) + fun, ok := funv.(*starlark.Function) if !ok { notFound = append(notFound, funName) } + + // Get the function arguments. + argc := fun.NumParams() + var args []string + for i := range argc { + arg, _ := fun.Param(i) + args = append(args, arg) + } + + // Return an error early if the function does not have the right arguments. + match := len(args) == len(requiredArgs) + if match { + sort.Strings(args) + sort.Strings(requiredArgs) + for i := range args { + if args[i] != requiredArgs[i] { + match = false + break + } + } + } + + if !match { + return fmt.Errorf("The function %q defines arguments %q (expected: %q)", funName, args, requiredArgs) + } } switch len(notFound) { @@ -130,8 +156,8 @@ func InstancePlacementCompile(name string, src string) (*starlark.Program, error // InstancePlacementValidate validates the instance placement scriptlet. func InstancePlacementValidate(src string) error { - return validate(InstancePlacementCompile, nameInstancePlacement, src, []string{ - "instance_placement", + return validate(InstancePlacementCompile, nameInstancePlacement, src, map[string][]string{ + "instance_placement": {"request", "candidate_members"}, }) } @@ -173,8 +199,8 @@ func QEMUCompile(name string, src string) (*starlark.Program, error) { // QEMUValidate validates the QEMU scriptlet. func QEMUValidate(src string) error { - return validate(QEMUCompile, prefixQEMU, src, []string{ - "qemu_hook", + return validate(QEMUCompile, prefixQEMU, src, map[string][]string{ + "qemu_hook": {"hook_name"}, }) } @@ -200,8 +226,8 @@ func AuthorizationCompile(name string, src string) (*starlark.Program, error) { // AuthorizationValidate validates the authorization scriptlet. func AuthorizationValidate(src string) error { - return validate(AuthorizationCompile, nameAuthorization, src, []string{ - "authorize", + return validate(AuthorizationCompile, nameAuthorization, src, map[string][]string{ + "authorize": {"details", "object", "entitlement"}, }) }