Add an option to publish stack images directly

[dmikusa]

Summary

Instead of outputing OCI image files, this PR adds a --publish flag which signals jam to instead upload the created images directly to a registry. It will upload to the image reference listed in the --buildOutput and --runOutput arguments (instead of a path, use an image reference like docker.io/foo/bar:0.0.2.

Internally, this splits the client.Export code into two parts. One part outputs the images to a temporary directory and another converts them to an OCI file or uploads them directly to the registry.

Here's the output of a jam create-stack using this upload functionality:

• https://hub.docker.com/r/dmikusa/run-jammy-base/tags
• https://hub.docker.com/r/dmikusa/build-jammy-base/tags

Use Cases

The main problem with using the existing Export functionality and then uploading images with a tool like skopeo is that it cannot yet deal with multi-architecture images. Since we're trying to add this functionality to Paketo stacks, something needs to change.

This was a small code change to jam that adds the functionality necessary to upload images. It should also shorten pipelines because now you don't need the actual files. The images go directly to the registry. If you then need to tag or copy them to other registries you can efficiently do that with skopeo or crane.

Question

1. I don't really know how to test the addition here. An integration test isn't really possible because you'd need an external registry as part of that test. There also doesn't seem to be an easy way to mock out this functionality, nor would that provide a lot of benefits.

   I tried to break down the code such that the existing tests for Export would exercise most of the functionality and the rest is just a few calls to go-containerregistry which does the hard work and is already well tested. If you feel like we need a test and have ideas about how we could go about that, I'd be happy to add one. Just let me know.

2. I'm not 100% sure how well this will fit into the existing CI for stacks. This is going to publish directly to the registry and right now, I believe the build and publish are separate steps. Correct me if I'm wrong, but it looks like the build step runs and then saves the OCI image files to the GitHub release. We then have skopeo come by and upload those files to the registry.

   If it would help, we could make the publish command be done in addition to the Export, instead of only running one or the other. That might be easier to fit into the CI, as you'd still get the OCI image files. Regardless of what happens, something is going to need to change with CI because skopeo can't upload multi-arch images (at least not right now), so CI is going to fail when it tries to do that.

   I'm not an expert on these pipelines, so feel free to ignore this, but a couple ideas:

    - Use this PR as it is now. Then download the image file as an OCI file, attach it to the Release.
    - I modify this PR to both upload and export to a file. Then modify the pipelines to just attach the files to the Release. Skip the `skopeo` parts where it uploads (tagging could still be done).
   

   I'm open to something else as well, just let me know.

Checklist

• I have viewed, signed, and submitted the Contributor License Agreement.
• [] I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
• [?] I have added an integration test, if necessary.
• I have reviewed the styleguide for guidance on my code quality.
• I'm happy with the commit history on this PR (I have rebased/squashed as needed).

[robdimsdale]

Thanks @dmikusa .

The code generally looks fine to me. I have one small observation, which is inline.

As far as the integration testing, I think it's possible to set up an integration test against a docker repository, but that would require some changes to the test structure:

• providing the docker registry and credentials (if necessary - I think you could use an unauthenticated local registry as well but I haven't validated this)
• making it easy to skip that test suite if you don't have access to a registry.

However, I have a more general concern: how we would use this in our CI pipeline. As-is, the stacks pipelines could leverage this new feature by publishing the stack at the end of the pipeline using jam create-stack --publish. This would create the stack again and publish it. But I'd rather not do this for two reasons:

1. Pragmatically, it can take a while to create a stack. For example, the full stack takes about 18 minutes to build. Building a stack again just to publish it effectively doubles the pipeline duration.
2. Philosophically, I'd rather build an artifact once, test it, and publish the same artifact that was tested. In practice I trust jam-create stack to build the same image twice in a reproducible manner, but if we can use the same artifact throughout the testing and publishing phase, I'd prefer that.

This leads me to suggest either an addition to or a replacement for this PR: the idea of publishing pre-built stacks. I can see the value in jam create-stack --publish for people who want to build and publish a stack in one go, even if that's not the process we want for our stack creation. I think that could be a good community feature.

I think this additional command would be called something like jam publish-stack which takes the pre-built oci archive and publishes it as a multi-arch image. I'm guessing that we can use the existing Upload code in this PR for that purpose, but I haven't fully validated that.

Alternatively, I could see a change to our pipeline process which would allow us to use the behavior in this PR and also meet the two goals above. If we build and publish the stack with a temporary version tag (e.g. -rc to signify release-candidate) to the dockerhub registry, we can download that same image in the tests, and then in the finalize/publish/upload step we could remove the -rc tags and add the latest and the version tags. I'm assuming that skopeo/docker can be used to change the tags of an existing image, though I haven't validated this.

My preference would be to add jam publish-stack </path/to/oci/tgz> (with jam create-stack --publish as a handy shortcut to build and publish). I'd rather not invert all our pipelines to introduce the concept of -rc builds unless we have to.

Looking at those two commands, I'd expect them to have a similar set of flags, so rather than re-using --build-output and --run-output, I think it would be clearer to introduce --image-reference (or similar naming).

So the two-phase command pair would look like the following:

jam create-stack \
  --build-output ./build.oci \
  --run-output ./run.oci \
  --config /path/to/config

jam publish-stack \
  --build-archive ./build.oci \
  --run-archive ./run.oci \
  --build-image-reference <remote-ref-build> \
  --run-image-reference <remote-ref-run>

and the all-in-one command would look like:

jam create-stack \
  --build-output ./build.oci \
  --run-output ./run.oci \
  --publish \
  --build-image-reference <remote-ref-build> \
  --run-image-reference <remote-ref-run>

In sketching this out, I see some optimizations we could make with flag handling. For example if you provide --publish to jam create-stack then perhaps you don't also have to provide --build-output/--run-output. We may also want to alias the --build-output/--run-output flags to --build-archive/--run-archive to provide symmetry between jam create-stack and jam publish-stack.

[dmikusa]

My preference would be to add jam publish-stack </path/to/oci/tgz> (with jam create-stack --publish as a handy shortcut to build and publish). I'd rather not invert all our pipelines to introduce the concept of -rc builds unless we have to.

I don't think that should be hard, and I agree it should be able to use upload in both places. It will have to extract the tgz to a temp folder, so there's a little extra I/O but nothing too bad, then push from the folder. Not a big deal, so I'll make that change (add publish-stack and keep the existing one-shot publish).

Looking at those two commands, I'd expect them to have a similar set of flags, so rather than re-using --build-output and --run-output, I think it would be clearer to introduce --image-reference (or similar naming).
In sketching this out, I see some optimizations we could make with flag handling. For example if you provide --publish to jam create-stack then perhaps you don't also have to provide --build-output/--run-output. We may also want to alias the --build-output/--run-output flags to --build-archive/--run-archive to provide symmetry between jam create-stack and jam publish-stack.

I was trying to avoid complicated flag handling, but I can give this a shot. Hopefully, it won't be too bad 🤞

[robdimsdale]

I was trying to avoid complicated flag handling, but I can give this a shot. Hopefully, it won't be too bad 🤞

Yeah. I think we can avoid those optimizations to start with. We can always introduce them later - they won't be breaking changes - so feel free to do the simplest thing for this PR.

[dmikusa]

@robdimsdale Requested changes have been submitted.

I went slightly differently with the create-stack change. The --publish flag will publish in addition to building instead making it build or publish. The absence of the flag just means it will build like normal. This made the flag arguments simpler.

[robdimsdale]

This looks great. I have one minor comment about the flag description (inline) but otherwise I think this is good to go.

[robdimsdale]

The --publish flag will publish in addition to building instead making it build or publish. The absence of the flag just means it will build like normal. This made the flag arguments simpler.

Perfect. I think that's an intuitive UI and I agree it simplifies the code too.

[dmikusa]

@robdimsdale I fixed the issue you mentioned as well as the code quality issues reported. I don't think we'd run into symlinks like that, but it doesn't hurt to try and protect from that problem. When you get a second, needs your review again.

[robdimsdale]

Looks great - thanks for cleaning up the potential tar expansion issues. I agree we're unlikely to encounter them, so I was willing to merge without addressing them, but it is better to handle them.

[dmikusa]

Thanks @robdimsdale - I don't want to be pushy, could we cut a release to get this out? Or do you have plans for the time frame of the next release?

[robdimsdale]

Already cut a new release @dmikusa :)