From e0a7b09f82dee17f51a73395cb309e910a7ea61c Mon Sep 17 00:00:00 2001 From: Harsh Thakur Date: Thu, 7 Sep 2023 23:32:24 +0530 Subject: [PATCH 1/4] Add push flag to support pushing to registry directly Signed-off-by: Harsh Thakur --- pkg/buildkit/buildkit.go | 44 ++++++++++++++++++++++++++++++++++++++++ pkg/patch/cmd.go | 1 + pkg/patch/patch.go | 10 +++++++-- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index 8a2a2938..d88f8ed3 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -237,3 +237,47 @@ func SolveToDocker(ctx context.Context, c *client.Client, st *llb.State, configD }) return eg.Wait() } + +func SolveToRegistry(ctx context.Context, c *client.Client, st *llb.State, configData []byte, tag string) error { + def, err := st.Marshal(ctx) + if err != nil { + log.Errorf("st.Marshal failed with %s", err) + return err + } + + dockerConfig := config.LoadDefaultConfigFile(os.Stderr) + attachable := []session.Attachable{authprovider.NewDockerAuthProvider(dockerConfig)} + solveOpt := client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterImage, + Attrs: map[string]string{ + "name": tag, + "push": "true", + // Pass through resolved configData from original image + exptypes.ExporterImageConfigKey: string(configData), + }, + }, + }, + Frontend: "", // i.e. we are passing in the llb.Definition directly + Session: attachable, // used for authprovider, sshagentprovider and secretprovider + } + + ch := make(chan *client.SolveStatus) + eg, ctx := errgroup.WithContext(ctx) + eg.Go(func() error { + _, err := c.Solve(ctx, def, solveOpt, ch) + return err + }) + eg.Go(func() error { + var c console.Console + if cn, err := console.ConsoleFromFile(os.Stderr); err == nil { + c = cn + } + // not using shared context to not disrupt display but let us finish reporting errors + _, err = progressui.DisplaySolveStatus(context.TODO(), c, os.Stdout, ch) + return err + }) + + return eg.Wait() +} diff --git a/pkg/patch/cmd.go b/pkg/patch/cmd.go index 156acc0c..f9ffd111 100644 --- a/pkg/patch/cmd.go +++ b/pkg/patch/cmd.go @@ -68,6 +68,7 @@ func NewPatchCmd() *cobra.Command { flags.BoolVar(&ua.ignoreError, "ignore-errors", false, "Ignore errors and continue patching") flags.StringVarP(&ua.format, "format", "f", "openvex", "Output format, defaults to 'openvex'") flags.StringVarP(&ua.output, "output", "o", "", "Output file path") + flags.StringVarP(&ua.pushDest, "push", "p", "", "Push patched image to destination registry. Format: /:. Note: this takes precedence over tag flag if set. ") if err := patchCmd.MarkFlagRequired("image"); err != nil { panic(err) diff --git a/pkg/patch/patch.go b/pkg/patch/patch.go index b80df1b8..675e9876 100644 --- a/pkg/patch/patch.go +++ b/pkg/patch/patch.go @@ -133,8 +133,14 @@ func patchWithContext(ctx context.Context, image, reportFile, patchedTag, workin return err } - if err = buildkit.SolveToDocker(ctx, config.Client, patchedImageState, config.ConfigData, patchedImageName); err != nil { - return err + if pushDest != "" { + if err = buildkit.SolveToRegistry(ctx, config.Client, patchedImageState, config.ConfigData, pushDest); err != nil { + return err + } + } else { + if err = buildkit.SolveToDocker(ctx, config.Client, patchedImageState, config.ConfigData, patchedImageName); err != nil { + return err + } } // create a new manifest with the successfully patched packages From 44ac19e6b86ad668a89b5d9c6d6dc47643d4c67b Mon Sep 17 00:00:00 2001 From: Harsh Thakur Date: Fri, 10 Nov 2023 00:43:38 +0530 Subject: [PATCH 2/4] add validation for registry/repo/image being passed Signed-off-by: Harsh Thakur --- pkg/patch/cmd.go | 7 +++- pkg/patch/patch.go | 81 ++++++++++++++++++++++++++--------------- pkg/patch/patch_test.go | 55 ++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 31 deletions(-) diff --git a/pkg/patch/cmd.go b/pkg/patch/cmd.go index f9ffd111..a6d6d7e7 100644 --- a/pkg/patch/cmd.go +++ b/pkg/patch/cmd.go @@ -26,6 +26,7 @@ type patchArgs struct { format string output string bkOpts buildkit.Opts + push bool } func NewPatchCmd() *cobra.Command { @@ -51,7 +52,9 @@ func NewPatchCmd() *cobra.Command { ua.format, ua.output, ua.ignoreError, - bkopts) + ua.push, + bkopts, + ) }, } flags := patchCmd.Flags() @@ -68,7 +71,7 @@ func NewPatchCmd() *cobra.Command { flags.BoolVar(&ua.ignoreError, "ignore-errors", false, "Ignore errors and continue patching") flags.StringVarP(&ua.format, "format", "f", "openvex", "Output format, defaults to 'openvex'") flags.StringVarP(&ua.output, "output", "o", "", "Output file path") - flags.StringVarP(&ua.pushDest, "push", "p", "", "Push patched image to destination registry. Format: /:. Note: this takes precedence over tag flag if set. ") + flags.BoolVarP(&ua.push, "push", "p", false, "Push patched image to destination registry") if err := patchCmd.MarkFlagRequired("image"); err != nil { panic(err) diff --git a/pkg/patch/patch.go b/pkg/patch/patch.go index 675e9876..f8f5b0d3 100644 --- a/pkg/patch/patch.go +++ b/pkg/patch/patch.go @@ -5,12 +5,13 @@ import ( "errors" "fmt" "os" + "strings" "time" + ref "github.com/distribution/reference" log "github.com/sirupsen/logrus" "golang.org/x/exp/slices" - "github.com/distribution/reference" "github.com/project-copacetic/copacetic/pkg/buildkit" "github.com/project-copacetic/copacetic/pkg/pkgmgr" "github.com/project-copacetic/copacetic/pkg/report" @@ -24,13 +25,13 @@ const ( ) // Patch command applies package updates to an OCI image given a vulnerability report. -func Patch(ctx context.Context, timeout time.Duration, image, reportFile, patchedTag, workingFolder, scanner, format, output string, ignoreError bool, bkOpts buildkit.Opts) error { +func Patch(ctx context.Context, timeout time.Duration, image, reportFile, patchedTag, workingFolder, scanner, format, output string, ignoreError, push bool, bkOpts buildkit.Opts) error { timeoutCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() ch := make(chan error) go func() { - ch <- patchWithContext(timeoutCtx, image, reportFile, patchedTag, workingFolder, scanner, format, output, ignoreError, bkOpts) + ch <- patchWithContext(timeoutCtx, image, reportFile, patchedTag, workingFolder, scanner, format, output, ignoreError, push, bkOpts) }() select { @@ -55,31 +56,11 @@ func removeIfNotDebug(workingFolder string) { } } -func patchWithContext(ctx context.Context, image, reportFile, patchedTag, workingFolder, scanner, format, output string, ignoreError bool, bkOpts buildkit.Opts) error { - imageName, err := reference.ParseNamed(image) +func patchWithContext(ctx context.Context, image, reportFile, patchedTag, workingFolder, scanner, format, output string, ignoreError, push bool, bkOpts buildkit.Opts) error { + patchedImageName, err := patchedImageTarget(image, patchedTag) if err != nil { return err } - if reference.IsNameOnly(imageName) { - log.Warnf("Image name has no tag or digest, using latest as tag") - imageName = reference.TagNameOnly(imageName) - } - taggedName, ok := imageName.(reference.Tagged) - if !ok { - err := errors.New("unexpected: TagNameOnly did create Tagged ref") - log.Error(err) - return err - } - tag := taggedName.Tag() - if patchedTag == "" { - if tag == "" { - log.Warnf("No output tag specified for digest-referenced image, defaulting to `%s`", defaultPatchedTagSuffix) - patchedTag = defaultPatchedTagSuffix - } else { - patchedTag = fmt.Sprintf("%s-%s", tag, defaultPatchedTagSuffix) - } - } - patchedImageName := fmt.Sprintf("%s:%s", imageName.Name(), patchedTag) // Ensure working folder exists for call to InstallUpdates if workingFolder == "" { @@ -133,12 +114,12 @@ func patchWithContext(ctx context.Context, image, reportFile, patchedTag, workin return err } - if pushDest != "" { - if err = buildkit.SolveToRegistry(ctx, config.Client, patchedImageState, config.ConfigData, pushDest); err != nil { + if push { + if err = buildkit.SolveToRegistry(ctx, config.Client, patchedImageState, config.ConfigData, *patchedImageName); err != nil { return err } } else { - if err = buildkit.SolveToDocker(ctx, config.Client, patchedImageState, config.ConfigData, patchedImageName); err != nil { + if err = buildkit.SolveToDocker(ctx, config.Client, patchedImageState, config.ConfigData, *patchedImageName); err != nil { return err } } @@ -163,7 +144,49 @@ func patchWithContext(ctx context.Context, image, reportFile, patchedTag, workin } // vex document must contain at least one statement if output != "" && len(validatedManifest.Updates) > 0 { - return vex.TryOutputVexDocument(validatedManifest, pkgmgr, patchedImageName, format, output) + return vex.TryOutputVexDocument(validatedManifest, pkgmgr, *patchedImageName, format, output) } return nil } + +func patchedImageTarget(image, patchedTag string) (*string, error) { + imageName, err := ref.ParseNamed(image) + if err != nil { + return nil, err + } + if ref.IsNameOnly(imageName) { + log.Warnf("Image name has no tag or digest, using latest as tag") + imageName = ref.TagNameOnly(imageName) + } + taggedName, ok := imageName.(ref.Tagged) + if !ok { + err := errors.New("unexpected: TagNameOnly did create Tagged ref") + log.Error(err) + return nil, err + } + tag := taggedName.Tag() + var patchedImageName string + if patchedTag == "" { + if tag == "" { + log.Warnf("No output tag specified for digest-referenced image, defaulting to `%s`", defaultPatchedTagSuffix) + patchedTag = defaultPatchedTagSuffix + } else { + patchedTag = fmt.Sprintf("%s-%s", tag, defaultPatchedTagSuffix) + } + } + + slashCount := strings.Count(patchedTag, "/") + if slashCount > 0 { + if slashCount < 2 { + err := fmt.Errorf("invalid tag %s, must be in the form /:", patchedTag) + log.Error(err) + return nil, err + } + // this implies user has passed a destination image name, not just a tag + patchedImageName = patchedTag + } else { + patchedImageName = fmt.Sprintf("%s:%s", imageName.Name(), patchedTag) + } + + return &patchedImageName, nil +} diff --git a/pkg/patch/patch_test.go b/pkg/patch/patch_test.go index b363c282..296048a2 100644 --- a/pkg/patch/patch_test.go +++ b/pkg/patch/patch_test.go @@ -44,3 +44,58 @@ func TestRemoveIfNotDebug(t *testing.T) { os.RemoveAll(workingFolder) }) } + +func TestPatchedImageTarget(t *testing.T) { + tests := []struct { + name string + image string + patchedTag string + want string + wantErr bool + }{ + { + name: "tag passed is empty", + image: "docker.io/library/nginx:1.21.3", + patchedTag: "", + want: "docker.io/library/nginx:1.21.3-patched", + wantErr: false, + }, + + { + name: "tag passed with value", + image: "docker.io/library/nginx:1.21.3", + patchedTag: "custom", + want: "docker.io/library/nginx:custom", + wantErr: false, + }, + { + name: "tag passed but without registry or repo", + image: "docker.io/library/nginx:1.21.3", + patchedTag: "my.registry/nginx:1.21.3-patched", + want: "", + wantErr: true, + }, + { + name: "tag passed contains registry, repo and image", + image: "docker.io/library/nginx:1.21.3", + patchedTag: "my.registry.io/myrepo/nginx:1.21.3-patched", + want: "my.registry.io/myrepo/nginx:1.21.3-patched", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := patchedImageTarget(tt.image, tt.patchedTag) + if (err != nil) != tt.wantErr { + t.Errorf("patchedImageTarget() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != nil { + if *got != tt.want { + t.Errorf("patchedImageTarget() = %v, want %v", *got, tt.want) + } + } + }) + } +} From 8fb40f0f9709d1ae5f8dd5de8738161edbc81fdd Mon Sep 17 00:00:00 2001 From: Harsh Thakur Date: Fri, 10 Nov 2023 01:07:15 +0530 Subject: [PATCH 3/4] fix logs and image registry names Signed-off-by: Harsh Thakur --- pkg/patch/patch.go | 3 +-- pkg/patch/patch_test.go | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/patch/patch.go b/pkg/patch/patch.go index f8f5b0d3..c99c6913 100644 --- a/pkg/patch/patch.go +++ b/pkg/patch/patch.go @@ -155,7 +155,7 @@ func patchedImageTarget(image, patchedTag string) (*string, error) { return nil, err } if ref.IsNameOnly(imageName) { - log.Warnf("Image name has no tag or digest, using latest as tag") + log.Warn("Image name has no tag or digest, using latest as tag") imageName = ref.TagNameOnly(imageName) } taggedName, ok := imageName.(ref.Tagged) @@ -179,7 +179,6 @@ func patchedImageTarget(image, patchedTag string) (*string, error) { if slashCount > 0 { if slashCount < 2 { err := fmt.Errorf("invalid tag %s, must be in the form /:", patchedTag) - log.Error(err) return nil, err } // this implies user has passed a destination image name, not just a tag diff --git a/pkg/patch/patch_test.go b/pkg/patch/patch_test.go index 296048a2..a9e706a1 100644 --- a/pkg/patch/patch_test.go +++ b/pkg/patch/patch_test.go @@ -71,15 +71,15 @@ func TestPatchedImageTarget(t *testing.T) { { name: "tag passed but without registry or repo", image: "docker.io/library/nginx:1.21.3", - patchedTag: "my.registry/nginx:1.21.3-patched", + patchedTag: "myregistry.azurecr.io/nginx:1.21.3-patched", want: "", wantErr: true, }, { name: "tag passed contains registry, repo and image", image: "docker.io/library/nginx:1.21.3", - patchedTag: "my.registry.io/myrepo/nginx:1.21.3-patched", - want: "my.registry.io/myrepo/nginx:1.21.3-patched", + patchedTag: "myregistry.azurecr.io/myrepo/nginx:1.21.3-patched", + want: "myregistry.azurecr.io/myrepo/nginx:1.21.3-patched", wantErr: false, }, } From 82060be60e135c99d400f9f2661066f7e16bee77 Mon Sep 17 00:00:00 2001 From: Harsh Thakur Date: Fri, 10 Nov 2023 01:23:28 +0530 Subject: [PATCH 4/4] Dedup Solve methods Signed-off-by: Harsh Thakur --- pkg/buildkit/buildkit.go | 85 +++++++++++++++------------------------- pkg/patch/patch.go | 10 +---- 2 files changed, 34 insertions(+), 61 deletions(-) diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index d88f8ed3..66ef9d81 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -182,7 +182,7 @@ func SolveToLocal(ctx context.Context, c *client.Client, st *llb.State, outPath return nil } -func SolveToDocker(ctx context.Context, c *client.Client, st *llb.State, configData []byte, tag string) error { +func Solve(ctx context.Context, c *client.Client, st *llb.State, configData []byte, tag string, push bool) error { def, err := st.Marshal(ctx) if err != nil { log.Errorf("st.Marshal failed with %s", err) @@ -192,23 +192,7 @@ func SolveToDocker(ctx context.Context, c *client.Client, st *llb.State, configD pipeR, pipeW := io.Pipe() dockerConfig := config.LoadDefaultConfigFile(os.Stderr) attachable := []session.Attachable{authprovider.NewDockerAuthProvider(dockerConfig)} - solveOpt := client.SolveOpt{ - Exports: []client.ExportEntry{ - { - Type: client.ExporterDocker, - Attrs: map[string]string{ - "name": tag, - // Pass through resolved configData from original image - exptypes.ExporterImageConfigKey: string(configData), - }, - Output: func(_ map[string]string) (io.WriteCloser, error) { - return pipeW, nil - }, - }, - }, - Frontend: "", // i.e. we are passing in the llb.Definition directly - Session: attachable, // used for authprovider, sshagentprovider and secretprovider - } + solveOpt := generateSolveOpts(push, tag, configData, attachable, pipeW) solveOpt.SourcePolicy, err = build.ReadSourcePolicy() if err != nil { return err @@ -229,55 +213,50 @@ func SolveToDocker(ctx context.Context, c *client.Client, st *llb.State, configD _, err = progressui.DisplaySolveStatus(context.TODO(), c, os.Stdout, ch) return err }) - eg.Go(func() error { - if err := dockerLoad(ctx, pipeR); err != nil { - return err - } - return pipeR.Close() - }) + if !push { + eg.Go(func() error { + if err := dockerLoad(ctx, pipeR); err != nil { + return err + } + return pipeR.Close() + }) + } return eg.Wait() } -func SolveToRegistry(ctx context.Context, c *client.Client, st *llb.State, configData []byte, tag string) error { - def, err := st.Marshal(ctx) - if err != nil { - log.Errorf("st.Marshal failed with %s", err) - return err +func generateSolveOpts(push bool, tag string, configData []byte, attachable []session.Attachable, pipeW *io.PipeWriter) client.SolveOpt { + if push { + return client.SolveOpt{ + Exports: []client.ExportEntry{ + { + Type: client.ExporterImage, + Attrs: map[string]string{ + "name": tag, + "push": "true", + // Pass through resolved configData from original image + exptypes.ExporterImageConfigKey: string(configData), + }, + }, + }, + Frontend: "", // i.e. we are passing in the llb.Definition directly + Session: attachable, // used for authprovider, sshagentprovider and secretprovider + } } - - dockerConfig := config.LoadDefaultConfigFile(os.Stderr) - attachable := []session.Attachable{authprovider.NewDockerAuthProvider(dockerConfig)} - solveOpt := client.SolveOpt{ + return client.SolveOpt{ Exports: []client.ExportEntry{ { - Type: client.ExporterImage, + Type: client.ExporterDocker, Attrs: map[string]string{ "name": tag, - "push": "true", // Pass through resolved configData from original image exptypes.ExporterImageConfigKey: string(configData), }, + Output: func(_ map[string]string) (io.WriteCloser, error) { + return pipeW, nil + }, }, }, Frontend: "", // i.e. we are passing in the llb.Definition directly Session: attachable, // used for authprovider, sshagentprovider and secretprovider } - - ch := make(chan *client.SolveStatus) - eg, ctx := errgroup.WithContext(ctx) - eg.Go(func() error { - _, err := c.Solve(ctx, def, solveOpt, ch) - return err - }) - eg.Go(func() error { - var c console.Console - if cn, err := console.ConsoleFromFile(os.Stderr); err == nil { - c = cn - } - // not using shared context to not disrupt display but let us finish reporting errors - _, err = progressui.DisplaySolveStatus(context.TODO(), c, os.Stdout, ch) - return err - }) - - return eg.Wait() } diff --git a/pkg/patch/patch.go b/pkg/patch/patch.go index c99c6913..5a076c9d 100644 --- a/pkg/patch/patch.go +++ b/pkg/patch/patch.go @@ -114,14 +114,8 @@ func patchWithContext(ctx context.Context, image, reportFile, patchedTag, workin return err } - if push { - if err = buildkit.SolveToRegistry(ctx, config.Client, patchedImageState, config.ConfigData, *patchedImageName); err != nil { - return err - } - } else { - if err = buildkit.SolveToDocker(ctx, config.Client, patchedImageState, config.ConfigData, *patchedImageName); err != nil { - return err - } + if err = buildkit.Solve(ctx, config.Client, patchedImageState, config.ConfigData, *patchedImageName, push); err != nil { + return err } // create a new manifest with the successfully patched packages