Specify annotations and labels to be set on output images

[shahulsonhal]

Changes

Reference #731

Add new image mutate step container, analog to Git step.

Update Build spec to add optional annotations and labels.

Update TaskRun setup code to add an image mutate step.

Fixes #50

Submitter Checklist

• Includes tests if functionality changed/was added
• Includes docs if changes are user-facing
• Set a kind label on this PR
• Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

You can now specify annotations and labels in the Build that will be added to the image

[shahulsonhal]

/kind api-change

[SaschaSchwarze0]

Adding @imjasonh as this implements a ship written by him, Modifying Output Image.

Compared to the ship, we made one change in the implementation: we did not use the crane image and command directly. The reason is that we change the image digest and therefore need to update the result. The crane image has no shell. So, although the command writes the image digest to stdout, we have no way to direct that into the file.

Before merging this PR, we need to get the new image repository set up in quay.io/shipwright/mutate-image.

[SaschaSchwarze0]

Nice. Please have a look at multiple labels/annotations.

[SaschaSchwarze0]

Nice. Two small things. And one more addition we need for completeness:

In build.go, we have the code that validates Builds. Please add another validator for the annotations and labels so that the registration of the Build will fail if the annotation or label value is not correct (misses the =). You can still retain your current logic when you build the the TaskRun, this does not hurt. But adding this validation upfront for the Build would be nice for our users.

Above was non-sense as the key and value are separated anyway in the map that is being provided in the Build.

[SaschaSchwarze0]

/approve
/assign imjasonh

@imjasonh we need a second reviewer as it changes the API, and as it is the implementation of your Ship, can you have a look ?

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [SaschaSchwarze0]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[gabemontero]

quick sanity check: went back to the EP to refresh my mem, and while it ref'ed the use of crane to get commonality across all the different image build tools, it does list the alternative of passing via strategy params the necessary info to use each image tools options for annotating / labeling.

To be clear, I don't want to block progress of this PR in any way wrt that, but I wanted to make sure nothing in this PR is locking us into the use of the mutate via crane step. From what I see, that is in fact the case. This PR has not provided a "configure how we mutate" piece of config at this time, but I think that is OK. Seems like we can just add it if we decided to add such a thing, and build of of the new API added here.

Lastly, disclaimer: I did not open every resolved conversation to see if this was already discussed.

Given that preamble, @SaschaSchwarze0 @shahulsonhal - do you all agree that adding options on how we mutate to add labels / annotations can be easily extended in the future?

[imjasonh]

/lgtm

I think we might want to revisit writing and maintaining this code ourselves, but for now it's probably fine. If we find ourselves rebuilding more of crane in this repo we might reconsider.

Thanks for putting this together!

[imjasonh]

This might be a reason to prefer just using crane directly. If it doesn't support this already I suspect it will before we add support for it.

[SaschaSchwarze0]

quick sanity check: went back to the EP to refresh my mem, and while it ref'ed the use of crane to get commonality across all the different image build tools, it does list the alternative of passing via strategy params the necessary info to use each image tools options for annotating / labeling.

To be clear, I don't want to block progress of this PR in any way wrt that, but I wanted to make sure nothing in this PR is locking us into the use of the mutate via crane step. From what I see, that is in fact the case. This PR has not provided a "configure how we mutate" piece of config at this time, but I think that is OK. Seems like we can just add it if we decided to add such a thing, and build of of the new API added here.

Lastly, disclaimer: I did not open every resolved conversation to see if this was already discussed.

Given that preamble, @SaschaSchwarze0 @shahulsonhal - do you all agree that adding options on how we mutate to add labels / annotations can be easily extended in the future?

Hi @gabemontero, thanks for checking. You refer to this, I think:

Future Improvements: As a future improvement, we can make the Build's specified labels and annotations available as parameters to BuildStrategy authors, so they can pass labels/annotations to their build process directly. In this case the appended step will perform a no-op push, which will not trigger a subsequent image change notification.

I think the larger topic is ...

• ... if we eventually make some parts that Shipwright today handles out of the box (not just mutate image, could also be source handling even) active by default but allow build strategy authors to opt-out of them and manually handle things
• ... if we eventually add more parts that Shipwright handles out of the box, in particular image push

We do not have any pressure in down stream regarding this feature, so, we can have a broader discussion in a community meeting on this. But in general, I do not see us locked.

[gabemontero]

quick sanity check: went back to the EP to refresh my mem, and while it ref'ed the use of crane to get commonality across all the different image build tools, it does list the alternative of passing via strategy params the necessary info to use each image tools options for annotating / labeling.
To be clear, I don't want to block progress of this PR in any way wrt that, but I wanted to make sure nothing in this PR is locking us into the use of the mutate via crane step. From what I see, that is in fact the case. This PR has not provided a "configure how we mutate" piece of config at this time, but I think that is OK. Seems like we can just add it if we decided to add such a thing, and build of of the new API added here.
Lastly, disclaimer: I did not open every resolved conversation to see if this was already discussed.
Given that preamble, @SaschaSchwarze0 @shahulsonhal - do you all agree that adding options on how we mutate to add labels / annotations can be easily extended in the future?

Hi @gabemontero, thanks for checking. You refer to this, I think:

Future Improvements: As a future improvement, we can make the Build's specified labels and annotations available as parameters to BuildStrategy authors, so they can pass labels/annotations to their build process directly. In this case the appended step will perform a no-op push, which will not trigger a subsequent image change notification.

Yes correct @SaschaSchwarze0

I think the larger topic is ...

* ... if we eventually make some parts that Shipwright today handles out of the box (not just mutate image, could also be source handling even) active by default but allow build strategy authors to opt-out of them and manually handle things

Yes there are similarities between the potential plug and play implementation mechanism for labeling / annotating images and the source code upload feature, and what @otaviof and @HeavyWombat are doing there.

Perhaps with such overrides and their specifications growing, we capture those centrally vs. specific to each feature.

Perhaps something to bring up in an upcoming community meeting, with an eye toward creating an issue for a possible EP in this space.

* ... if we eventually add more parts that Shipwright handles out of the box, [in particular image push](https://github.com/shipwright-io/build/issues/165)

We do not have any pressure in down stream regarding this feature, so, we can have a broader discussion in a community meeting on this.

Understood. Conversely, I could see as we eventually migrate existing openshift build customers to shipwright, where they are use to the savings of getting the annotations/labels in the images build step vs. a separate step, with the need to sign off on an additional crane based image to do so, we may get pressure here at that point.

But it is not an immediate thing on our end for sure.

But in general, I do not see us locked.

Agreed and thanks for the confirmation / follow up.