Add source timestamp field for source result #1471

HeavyWombat · 2024-01-18T12:02:48Z

Changes

Extend the API fields and CRD files to include the source timestamp in the source result.

Update Git step code to write commit timestamp result file.

Update Bundle step code to write source timestamp, which is the the most recent source file timestamp.

Extend task run setup logic to include respective command line flags for the Git and Bundle step CLIs to write the result files, which then can be used to extend the status sources fields of the BuildRun.

Fix timestamp issue in Bundle pack and push code so that only the image gets a neutral timestamp, but the packaged source files retain their respective filesystem modification timestamps.

Add integration test cases to verify the new status sources fields.

Fixes #1446

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

Git and Bundle sources now produce additional status fields in a BuildRun to return the commit timestamp of the commit being used, or the image/source timestamp of Bundle images respectively.

HeavyWombat · 2024-01-18T12:04:40Z

cmd/bundle/main_test.go

@@ -188,14 +211,16 @@ var _ = Describe("Bundle Loader", func() {
 		})

 		AfterEach(func() {
-			ref, err := name.ParseReference(testImage)
-			Expect(err).ToNot(HaveOccurred())
+			if testImage != "" {


When certain tests are skipped, the testImage string can actually be empty and therefore would cause a panic.

HeavyWombat · 2024-01-18T12:07:22Z

pkg/bundle/bundle.go

@@ -35,12 +35,12 @@ func PackAndPush(ref name.Reference, directory string, options ...remote.Option)
 		return name.Digest{}, err
 	}

-	image, err := mutate.AppendLayers(empty.Image, bundleLayer)
+	image, err := mutate.Time(empty.Image, time.Unix(0, 0))


Mutating the empty image before using it with AppendLayers makes sure that the image has timestamp 0, but the layer that is later added is not mutated as well.

pkg/apis/build/v1alpha1/buildrun_types.go

SaschaSchwarze0 · 2024-01-22T15:13:43Z

pkg/reconciler/buildrun/resources/sources/bundle.go

+		pipelineapi.TaskResult{
+			Name:        fmt.Sprintf("%s-source-%s-source-timestamp", prefixParamsResultsVolumes, name),
+			Description: "The timestamp of the most recent source file.",
+		},


I wonder if this should be rather declared in sources.go generically.

SaschaSchwarze0 · 2024-01-22T15:14:07Z

pkg/reconciler/buildrun/resources/sources/bundle.go

+		sourceResult.Bundle = &build.BundleSourceResult{Digest: imageDigest}
+	}
+
+	sourceTimestamp := findResultValue(results, fmt.Sprintf("%s-source-%s-source-timestamp", prefixParamsResultsVolumes, name))


Same here, should that be in sources.go, updateBuildRunStatusWithSourceResult ?

SaschaSchwarze0 · 2024-01-22T15:15:47Z

pkg/reconciler/buildrun/resources/sources/git.go

-	branchName         = "branch-name"
+	commitSHAResult       = "commit-sha"
+	commitAuthorResult    = "commit-author"
+	commitTimestampResult = "commit-timestamp"


Consider using a common result as suggested in bundle.go. This will help us when we want to consume the result from another step (image-processing). Ideally we there need no check which result to read but rely on a common name independent of the source type.

HeavyWombat · 2024-01-24T15:04:44Z

test/v1alpha1_samples/catalog.go

+		TypeMeta: metav1.TypeMeta{
+			Kind: string(build.ClusterBuildStrategyKind),
+		},


Using ClusterBuildStrategy caused issues since the BuildRuns created using this strategy in the test code did not have the kind and therefore failed to be usable.

SaschaSchwarze0 · 2024-01-24T15:47:11Z

pkg/reconciler/buildrun/resources/sources/utils.go

 	for _, result := range results {
 		if result.Name == name {
-			return result.Value.StringVal
+			return strings.TrimSpace(result.Value.StringVal)


Why ? If for example a commit message ends with a space then we can be accurate by not trimming.

Let me put that trim back on the other side.

Fix issue with empty string when test cases are skipped.

Extend Git step CLI to write a result file containing the source timestamp of the commit that is checked out.

SaschaSchwarze0

/approve
/lgtm

Add flag to write source timestamp into result file. Fix `PackAndPush` function to set timestamp of the base image and not for all files in the main layer of the image to keep the timestamps of the files in the bundle layer.

Update CRDs to include timestamp fields. Add timestamp fields for Git and Bundle results.

SaschaSchwarze0

/approve
/lgtm

openshift-ci · 2024-01-29T16:24:49Z

[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

HeavyWombat added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 18, 2024

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

pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 18, 2024

openshift-ci bot requested review from adambkaplan and otaviof January 18, 2024 12:02

HeavyWombat requested review from SaschaSchwarze0 and qu1queee January 18, 2024 12:03

HeavyWombat commented Jan 18, 2024

View reviewed changes

HeavyWombat force-pushed the add/source-timestamp branch 3 times, most recently from 1cc6f3f to 414bd83 Compare January 18, 2024 12:41

SaschaSchwarze0 requested changes Jan 18, 2024

View reviewed changes

pkg/apis/build/v1alpha1/buildrun_types.go Outdated Show resolved Hide resolved

HeavyWombat force-pushed the add/source-timestamp branch from 414bd83 to 594deeb Compare January 22, 2024 13:44

HeavyWombat requested a review from SaschaSchwarze0 January 22, 2024 13:44

HeavyWombat changed the title ~~Add commit, source, and image timestamps fields~~ Add source timestamp field for source result Jan 22, 2024

SaschaSchwarze0 reviewed Jan 22, 2024

View reviewed changes

HeavyWombat force-pushed the add/source-timestamp branch from 594deeb to 8492b27 Compare January 24, 2024 14:55

HeavyWombat requested a review from SaschaSchwarze0 January 24, 2024 14:55

HeavyWombat force-pushed the add/source-timestamp branch from 8492b27 to 08956ed Compare January 24, 2024 14:58

HeavyWombat commented Jan 24, 2024

View reviewed changes

SaschaSchwarze0 reviewed Jan 24, 2024

View reviewed changes

HeavyWombat added 2 commits January 29, 2024 15:53

Fix panic in test case AfterEach

6eddeaf

Fix issue with empty string when test cases are skipped.

Add source timestamp for Git step

cdac899

Extend Git step CLI to write a result file containing the source timestamp of the commit that is checked out.

HeavyWombat force-pushed the add/source-timestamp branch from 08956ed to 420c8f9 Compare January 29, 2024 15:29

HeavyWombat requested a review from SaschaSchwarze0 January 29, 2024 15:32

SaschaSchwarze0 approved these changes Jan 29, 2024

View reviewed changes

SaschaSchwarze0 added this to the release-v0.13.0 milestone Jan 29, 2024

HeavyWombat force-pushed the add/source-timestamp branch from 420c8f9 to 4eb9791 Compare January 29, 2024 15:51

Add source timestamp for Bundle step

9d8f3a2

Add flag to write source timestamp into result file. Fix `PackAndPush` function to set timestamp of the base image and not for all files in the main layer of the image to keep the timestamps of the files in the bundle layer.

Add timestamp fields to resource objects

f181f4c

Update CRDs to include timestamp fields. Add timestamp fields for Git and Bundle results.

HeavyWombat force-pushed the add/source-timestamp branch from 4eb9791 to f181f4c Compare January 29, 2024 15:52

SaschaSchwarze0 approved these changes Jan 29, 2024

View reviewed changes

openshift-ci bot assigned SaschaSchwarze0 Jan 29, 2024

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

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

openshift-merge-bot bot merged commit ae280b3 into main Jan 29, 2024
14 checks passed

openshift-merge-bot bot deleted the add/source-timestamp branch January 29, 2024 16:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add source timestamp field for source result #1471

Add source timestamp field for source result #1471

HeavyWombat commented Jan 18, 2024 •

edited

Loading

HeavyWombat Jan 18, 2024

HeavyWombat Jan 18, 2024

SaschaSchwarze0 Jan 22, 2024

SaschaSchwarze0 Jan 22, 2024

SaschaSchwarze0 Jan 22, 2024

HeavyWombat Jan 24, 2024

SaschaSchwarze0 Jan 24, 2024 •

edited

Loading

HeavyWombat Jan 24, 2024

SaschaSchwarze0 left a comment

SaschaSchwarze0 left a comment

openshift-ci bot commented Jan 29, 2024

Add source timestamp field for source result #1471

Add source timestamp field for source result #1471

Conversation

HeavyWombat commented Jan 18, 2024 • edited Loading

Changes

Submitter Checklist

Release Notes

HeavyWombat Jan 18, 2024

Choose a reason for hiding this comment

HeavyWombat Jan 18, 2024

Choose a reason for hiding this comment

SaschaSchwarze0 Jan 22, 2024

Choose a reason for hiding this comment

SaschaSchwarze0 Jan 22, 2024

Choose a reason for hiding this comment

SaschaSchwarze0 Jan 22, 2024

Choose a reason for hiding this comment

HeavyWombat Jan 24, 2024

Choose a reason for hiding this comment

SaschaSchwarze0 Jan 24, 2024 • edited Loading

Choose a reason for hiding this comment

HeavyWombat Jan 24, 2024

Choose a reason for hiding this comment

SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jan 29, 2024

HeavyWombat commented Jan 18, 2024 •

edited

Loading

SaschaSchwarze0 Jan 24, 2024 •

edited

Loading