Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REQ] Add support for other output modes as buildctl does #604

Open
1 task
ashnamehrotra opened this issue May 2, 2024 · 5 comments · May be fixed by #621
Open
1 task

[REQ] Add support for other output modes as buildctl does #604

ashnamehrotra opened this issue May 2, 2024 · 5 comments · May be fixed by #621
Assignees
Labels
enhancement New feature or request

Comments

@ashnamehrotra
Copy link
Contributor

What kind of request is this?

None

What is your request or suggestion?

// TODO: Add support for other output modes as buildctl does.

Turning copacetic TODO comments into issues from https://docs.google.com/spreadsheets/d/1XwNj1J6e2FrUhlqaIsV10l8_tgov7WodlkvpNZXYZMU/edit#gid=1386834576.

Are you willing to submit PRs to contribute to this feature request?

  • Yes, I am willing to implement it.
@h4l0gen
Copy link

h4l0gen commented May 11, 2024

Hi @ashnamehrotra, I'm interested in this issue. Could you please assign it to me? This way, other contributors won't work on the same issue, and we can avoid wasting valuable time.
Thank you!

@h4l0gen
Copy link

h4l0gen commented May 11, 2024

Hey @ashnamehrotra in first look I thought of this solution. I've added support for multiple output formats to the patch command in the patch.go file. This includes support for Docker images, OCI images.
Proposed key changes from my side are:

  1. Added a new parameter outputFormat to the Patch and patchWithContext functions.
  2. In the patchWithContext function, I've added a switch statement to handle the different output formats.
  3. Updated the cmd.go file to add a new flag for the outputFormat and map it to the patchArgs struct.

Here is an example of updated patchWithContext function

func patchWithContext(ctx context.Context, ch chan error, image, reportFile, patchedTag, workingFolder, scanner, format, output, outputFormat string, ignoreError bool, bkOpts buildkit.Opts) error {
// ...
	solveOpt := client.SolveOpt{
        Exports: []client.ExportEntry{},
        Frontend: "",
        Session:  attachable,
    }

    switch outputFormat {
    case "docker":
        solveOpt.Exports = append(solveOpt.Exports, client.ExportEntry{
            Type: client.ExporterDocker,
            Attrs: map[string]string{
                "name": patchedImageName,
            },
            Output: func(_ map[string]string) (io.WriteCloser, error) {
                return pipeW, nil
            },
        })
    case "oci":
        solveOpt.Exports = append(solveOpt.Exports, client.ExportEntry{
            Type:  client.ExporterOCI,
            Attrs: map[string]string{},
            Output: func(_ map[string]string) (io.WriteCloser, error) {
                return os.Create(fmt.Sprintf("%s.tar", patchedImageName))
            },
        })
    default:
        return fmt.Errorf("unsupported output format: %s", outputFormat)
    }

This should provide the necessary flexibility to users, allowing them to choose the desired output format for the patched image. This is required change in cmd.go

func NewPatchCmd() *cobra.Command {
// ...
	flags.StringVarP(&ua.outputFormat, "output-format", "F", "docker", "Output format for the patched image (docker, oci)")

and default value is set to docker

@h4l0gen
Copy link

h4l0gen commented May 11, 2024

Please take a look and provide your thoughts on this and let me know if you feel any changes or modifications. I've done all changes with this approach and creating WIP:PR for now, and modify it accordingly if you feel any changes.
Thank you!

@ChristofferNissen
Copy link

Hello 👋

I am in need of also have the option of exporting as OCI image tar archieve directly to local filesystem. I have copied the code from v0.6.2 and modified in my own repository, you can see what i did to implement the functionality. Would like to get rid of this local copy by having it integrated into the main code base :)

Example

@h4l0gen
Copy link

h4l0gen commented May 26, 2024

Hi @ChristofferNissen thankyou for your wonderful work. I will surely like to take a look on it, i am just little busy with some other work. I will be back in some time, and we will discuss this implementation along with @ashnamehrotra. BTW I've raised a PR, It has only a small problem, it is just stopping after sending tarball, if you like to take a look on that. I will give you complete problem scenario in some time, not on my laptop right now. Thank you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: 🆕 New
Development

Successfully merging a pull request may close this issue.

3 participants