Add support of output image timestamp modification in image processing #1509

HeavyWombat · 2024-02-28T10:23:14Z

Changes

Implementation for SHIP 0037: build output timestamp

Main changes:

image processing CLI now supports two more command-line flags to set the image timestamp of an image or index.
API for Build Output was extended to include a string that defines what kind of timestamp modification is suppose to be done
TaskRun setup was extended to properly setup the build settings to invoke the image processing with the respective settings

Fixes #1447

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

Output image section now supports an optional timestamp field, which can be used to change the image creation timestamp, i.e. use string "SourceTimestamp" to let the output image creation timestamp to be modified to the timestamp of the source timestamp.

HeavyWombat · 2024-02-28T10:24:50Z

cmd/image-processing/main.go

-func getAnnotation() []string {
-	var annotation []string
-
-	if flagValues.annotation != nil {
-		return append(annotation, *flagValues.annotation...)
-	}
-
-	return annotation
-}
-
-func getLabel() []string {
-	var label []string
-
-	if flagValues.label != nil {
-		return append(label, *flagValues.label...)
-	}
-
-	return label
-}
-


Simplifying the flag usage meant that these became obsolete.

HeavyWombat · 2024-02-28T10:25:23Z

cmd/image-processing/main.go

+	pflag.StringArrayVar(&flagValues.annotation, "annotation", nil, "New annotations to add")
+	pflag.StringArrayVar(&flagValues.label, "label", nil, "New labels to add")


Just to unify the style with the other flags

HeavyWombat · 2024-02-28T10:26:58Z

cmd/image-processing/main.go

@@ -179,6 +201,8 @@ func runImageProcessing(ctx context.Context) error {
 		return err
 	}

+	log.Printf("Image %s@%s pushed\n", imageName.String(), digest)


This is only to allow for better time tracking in the user logs.

HeavyWombat · 2024-02-28T10:27:48Z

cmd/image-processing/main_test.go

 		})

 		It("should fail in case --image does not exist", func() {
 			Expect(run(
-				"--image", "docker.io/feqlQoDIHc/bcfHFHHXYF",


The name in the use case was actually invalid.

HeavyWombat · 2024-02-28T10:28:18Z

cmd/image-processing/main_test.go

 		})

 		It("should fail in case annotation is invalid", func() {
 			withTestImage(func(tag name.Tag) {
 				Expect(run(
+					"--insecure",


This was a bug in the test cases actually, the test case did not test what was intended to test.

HeavyWombat · 2024-02-28T10:28:58Z

cmd/image-processing/main_test.go

 					"--image", tag.String(),
 					"--annotation", "org.opencontainers.image.url*https://my-company.com/images",
-				)).To(HaveOccurred())
+				)).To(FailWith("not enough parts"))


Switched to error text matching in order to avoid not checking for that actual problem in the tests.

HeavyWombat · 2024-02-28T10:36:42Z

pkg/image/mutate.go

-
-				// there is no mutate.ReplaceManifests, therefore, remove the old one first, and then add the new one
-				imageIndex = mutate.RemoveManifests(imageIndex, func(desc containerreg.Descriptor) bool {
-					return desc.Digest.String() == digest.String()
-				})
-
-				imageIndex = mutate.AppendManifests(imageIndex, mutate.IndexAddendum{
-					Add: appendable,
-					Descriptor: containerreg.Descriptor{
-						Annotations: descriptor.Annotations,
-						MediaType:   descriptor.MediaType,
-						Platform:    descriptor.Platform,
-						URLs:        descriptor.URLs,
-					},
-				})


This code was moved into replaceManifestIn.

HeavyWombat · 2024-02-28T10:48:58Z

test/e2e/v1alpha1/e2e_image_mutate_test.go

+			Expect(annotationsOf(image)).To(HaveKeyWithValue("org.opencontainers.image.url", "https://my-company.com/images"))
+			Expect(labelsOf(image)).To(HaveKeyWithValue("maintainer", "[email protected]"))


Unrelated to timestamp work, but file needed to touched anyway and this should serve to have a common style and to be more readable.

qu1queee

adding some initial feedback, pls take a look.

docs/build.md

pkg/apis/build/v1beta1/build_types.go

pkg/reconciler/build/build_test.go

pkg/reconciler/buildrun/resources/image_processing.go

pkg/reconciler/buildrun/resources/taskrun.go

pkg/validate/output.go

Simplify the `--annotation` and `--label` flag setup.

Ref: https://github.com/shipwright-io/community/blob/main/ships/0037-build-output-timestamp.md Add optional output image timestamp field to allow for setting a custom image creation timestamp.

Add command-line flag `--image-timestamp` to update the image creation timestamp of the image when using shipwright-managed push.

qu1queee

Docs need to be improved, per the significance of this feature. Please take a look at my comments

pkg/reconciler/buildrun/resources/taskrun.go

qu1queee · 2024-03-12T08:09:05Z

docs/build.md

@@ -89,6 +89,11 @@ The `Build` definition supports the following fields:
  - `spec.timeout` - Defines a custom timeout. The value needs to be parsable by [ParseDuration](https://golang.org/pkg/time/#ParseDuration), for example, `5m`. The default is ten minutes. You can overwrite the value in the `BuildRun`.
  - `spec.output.annotations` - Refers to a list of `key/value` that could be used to [annotate](https://github.com/opencontainers/image-spec/blob/main/annotations.md) the output image.
  - `spec.output.labels` - Refers to a list of `key/value` that could be used to label the output image.
+  - `spec.output.timestamp` - Instruct the build to change the output image creation timestamp to the specified value. When omitted, the respective build strategy tool defines the output image timestamp.


Do you think we should also extend https://github.com/shipwright-io/build/blob/main/docs/build.md#defining-the-output and provide some precise samples on this feature? e.g. with yaml

In addition, why not to document it as well in https://github.com/shipwright-io/build/blob/main/docs/buildrun.md , at the end is also part of a BuildRun.

Upon double checking the docs, I realized we missed a couple of reasons in the table.

Introduce functions to mutate timestamp of an image or image index. Extend image-processing step to include required timestamp values based on the configured Build or BuildRun output settings. Add test E2E cases for timestamp values for: - Zero - SourceTimestamp - BuildTimestamp - custom timestamp

Add missing timestamp related reason fields and descriptions. Introduce consistent use of the **Note** keyword in the text. Format all tables to have the same style. Fix broken links in the table of content. Join lines to have a consistent line break style. Remove unncessary whitespaces in text. Add missing reasons in build validation table. Signed-off-by: Matthias Diester <[email protected]>

qu1queee

/lgtm
/approve

openshift-ci · 2024-03-12T09:45:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qu1queee

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~~ [qu1queee]

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

openshift-ci bot added the release-note Label for when a PR has specified a release note label Feb 28, 2024

openshift-ci bot requested review from adambkaplan and qu1queee February 28, 2024 10:23

pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Feb 28, 2024

HeavyWombat changed the title ~~Issue/1447~~ Add support of output image timestamp modification in image processing Feb 28, 2024

HeavyWombat commented Feb 28, 2024

View reviewed changes

HeavyWombat force-pushed the issue/1447 branch from 3d74db0 to 201369b Compare February 28, 2024 10:42

HeavyWombat commented Feb 28, 2024

View reviewed changes

HeavyWombat requested a review from SaschaSchwarze0 February 28, 2024 10:50

HeavyWombat force-pushed the issue/1447 branch 2 times, most recently from 67f24e8 to 167305e Compare March 1, 2024 08:34

qu1queee requested changes Mar 4, 2024

View reviewed changes

docs/build.md Outdated Show resolved Hide resolved

pkg/apis/build/v1beta1/build_types.go Outdated Show resolved Hide resolved

pkg/reconciler/build/build_test.go Outdated Show resolved Hide resolved

pkg/reconciler/buildrun/resources/image_processing.go Show resolved Hide resolved

qu1queee reviewed Mar 4, 2024

View reviewed changes

pkg/reconciler/buildrun/resources/taskrun.go Show resolved Hide resolved

qu1queee reviewed Mar 4, 2024

View reviewed changes

pkg/validate/output.go Outdated Show resolved Hide resolved

HeavyWombat force-pushed the issue/1447 branch from 167305e to f4890eb Compare March 8, 2024 08:26

HeavyWombat added 3 commits March 8, 2024 14:34

Simplify annotation and label flag

7dbe18f

Simplify the `--annotation` and `--label` flag setup.

Add output timestamp field in API

f4ffa2e

Ref: https://github.com/shipwright-io/community/blob/main/ships/0037-build-output-timestamp.md Add optional output image timestamp field to allow for setting a custom image creation timestamp.

Add --image-timestamp flag to image-processing

d77aec5

Add command-line flag `--image-timestamp` to update the image creation timestamp of the image when using shipwright-managed push.

HeavyWombat force-pushed the issue/1447 branch from f4890eb to e9a9a04 Compare March 8, 2024 13:42

HeavyWombat requested a review from qu1queee March 8, 2024 13:42

qu1queee requested changes Mar 12, 2024

View reviewed changes

HeavyWombat added 2 commits March 12, 2024 10:16

HeavyWombat force-pushed the issue/1447 branch from e9a9a04 to 237c8dd Compare March 12, 2024 09:18

qu1queee self-requested a review March 12, 2024 09:38

qu1queee approved these changes Mar 12, 2024

View reviewed changes

openshift-ci bot assigned qu1queee Mar 12, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 12, 2024

qu1queee removed request for adambkaplan and SaschaSchwarze0 March 12, 2024 09:46

openshift-merge-bot bot merged commit e1329cc into main Mar 12, 2024
14 checks passed

openshift-merge-bot bot deleted the issue/1447 branch March 12, 2024 10:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support of output image timestamp modification in image processing #1509

Add support of output image timestamp modification in image processing #1509

HeavyWombat commented Feb 28, 2024

HeavyWombat Feb 28, 2024

HeavyWombat Feb 28, 2024

HeavyWombat Feb 28, 2024

HeavyWombat Feb 28, 2024

HeavyWombat Feb 28, 2024

HeavyWombat Feb 28, 2024

HeavyWombat Feb 28, 2024

HeavyWombat Feb 28, 2024

qu1queee left a comment

qu1queee left a comment •

edited

Loading

qu1queee Mar 12, 2024

HeavyWombat Mar 12, 2024

qu1queee left a comment

openshift-ci bot commented Mar 12, 2024

		pflag.StringArrayVar(&flagValues.annotation, "annotation", nil, "New annotations to add")
		pflag.StringArrayVar(&flagValues.label, "label", nil, "New labels to add")

		Expect(annotationsOf(image)).To(HaveKeyWithValue("org.opencontainers.image.url", "https://my-company.com/images"))
		Expect(labelsOf(image)).To(HaveKeyWithValue("maintainer", "[email protected]"))

Add support of output image timestamp modification in image processing #1509

Add support of output image timestamp modification in image processing #1509

Conversation

HeavyWombat commented Feb 28, 2024

Changes

Submitter Checklist

Release Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qu1queee left a comment

Choose a reason for hiding this comment

qu1queee left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qu1queee left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Mar 12, 2024

qu1queee left a comment •

edited

Loading