Add SHIP for BuildRun Status Results

This SHIP provides a proposal on how to surface Tekton Task Results
into the BuildRun Status subresource.

ships/0005-surface-results-in-buildruns.md

@@ -0,0 +1,246 @@
+<!--
+Copyright The Shipwright Contributors
+
+SPDX-License-Identifier: Apache-2.0
+-->
+
+---
+title: neat-enhancement-idea
+authors:
+  - @qu1queee
+reviewers:
+  - @ImJasonH
+  - @SaschaSchwarze0
+approvers:
+  - @ImJasonH
+  - @SaschaSchwarze0
+creation-date: 20201-June-29
+last-updated: 20201-July-08
+status: implementable
+---
+
+# Surface Step Results in BuildRun Status
+
+## Release Signoff Checklist
+
+- [x] Enhancement is `implementable`
+- [x] Design details are appropriately documented from clear requirements
+- [x] Test plan is defined
+- [x] Graduation criteria for dev preview, tech preview, GA
+- [ ] User-facing documentation is created in [docs](/docs/)
+
+## Open Questions [optional]
+
+- The status of a custom resource ( _.status_ ) is a "privileged" part, in the sense that only a controller can modify it. How much freedom do we want to provide to strategy authors when surfacing results in the BuildRun _.status_ .
+
+- Do we need to limit the amount of results we will surface under the BuildRun _.status_ somehow ?
+
+## Summary
+
+Provide support in the Shipwright BuildRun controller to surface Tekton TaskRun Results ( _.status.taskResults_ ) into the BuildRun _.status_, at a specific or multiple subpaths.
+
+## Motivation
+
+Today Shipwright Strategies already emit Tekton [results](https://github.com/tektoncd/pipeline/blob/main/docs/tasks.md#emitting-results) on their Step definitions, see [example](https://github.com/shipwright-io/build/blob/main/samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml#L65). These results contain valuable metadata for users, like the _image digest_  or the _commit sha_ of the source code used for building. Therefore, we need to ensure that the BuildRun controller can surface these results into it´s own _.status_ subresource.
+
+### Goals
+
+- Provide a mechanism to surface higher-level information in BuildRuns reported by the strategies, by enabling users to get insights into valuable metadata without the need of understanding strategy specific details.
+- Decide on the most optimal and minimal API changes for the BuildRun _.status_ subresource, in order to host multiple results.
+
+### Non-Goals
+
+- Limit the amount of results that we can define in the _.status_ subresource of a BuildRun.
+- Override the .status.conditions with Tekton results.
+
+## Proposal
+
+A high level proposal will require three things:
+
+- [1] Define the API changes for the BuildRun CRD, this should only be for the _.status_ subresource. See [API Changes](#api-changes).
+
+- [2] Modify the BuildRun CRD to extend the _.status_ subresource with the proposed changes in [1].
+
+- [3] Modify the BuildRun controller business logic, to be able to propagate the Tekton _.status.taskResults_ into [2].
+
+### API Changes
+
+The API changes require a categorization of two types of results. To provide a clear distinction, we will classify them as follows:
+
+- **Strategy Results:** Results that are intended for surfacing higher-level information of a strategy. Is the Strategy Author responsibility to define and emit this results, if desired.
+
+- **Build Results:** Results that are generated at runtime by the BuildRun controller. These results are expected to be present all the time, as long as the step that defines them, exist.
+
+#### Strategy Results
+
+This will require to introduce `results` under the _.status_ subresource of a BuildRun. This is a list of **name/value**, where Strategy Authors can surface their desire result. This new API change for the BuildRun will be looking as follows:
+
+```yaml
+ status:
+   results:
+   - name: a-strategy-result
+     value: a-value
+   - name: another-strategy-result
+     value: another-value
+```
+
+Additionally, we will need to allow Strategy Authors to define a list of results in the strategy definition. This new API change in the Strategies will be looking as follows:
+
+```yaml
+spec:
+  results:
+    - name: a-strategy-result
+      description: a-meaningful-description
+    - name: another-strategy-result
+      description: a-meaningful-description
+  buildSteps:
+    ...
+
+```
+
+#### Build Results
+
+These are results defined at runtime. In the long term, Build Results should contain at least the following:
+
+| Result Name                       | Sources Type                       | Emitted in Step      | Emitted | Definition  | Notes                                |
+| --------------------------------- | --------------------------------- | ------------:| -----------:|------------:| ------------------------------------:|
+| shp-source-default-commit-sha     | `git`     | `source-default`   | yes          |  at runtime | not implemented under `spec.sources` |
+| shp-image-size, shp-image-digest  | `bundle`  | `bundle`  | no          |  at runtime   | not implemented under `spec.sources` |
+| shp-image-size, shp-image-digest  | not apply  | `build-and-push` | yes         |  at runtime   | none |
+
+_Note_: We should expect the `http` type of sources to also emit some results, same with the new type ( _LocalCopy_ ) being defined in [Local Source Upload].
+
+Because of the above, we would like to introduce `stepsResult` under the _.status.stepsResult_ subresource of a BuildRun. The `stepsResult` list items should be keep to a minimum.
+The `StepResult` object contains a `Step` field, in order to differentiate from where the result is emitted, therefore multiple results from the same step are supported.
+
+`stepsResult` should be a list of `StepResult`:
+
+```go
+ type StepResult struct {
+    Name         string `json:"name"`
+    Value        string `json:"value"`
+    Step         string `json:"step"`
+ }
+```
+
+A BuildRun will surface these results as follows:
+
+```yaml
+ status:
+   stepsResult:
+   - name: shp-commit-sha
+     value: <sha>
+     step: source-default
+   - name: shp-image-size
+     value: <sha>
+     step: build-and-push
+   - name: shp-image-digest
+     value: <sha>
+     step: build-and-push
+```
+
+The above allows multiple steps of the same type to emit results with a clear diferentiation of their origin.
+
+_Note_: Categorizing Build Results per step might be complex, as we will consume results directly from Tekton, where there is no clear indication of their related step, as they come in the form of key/values.
+
+### User Stories [optional]
+
+#### As a Strategy Author, I want to surface strategy higher-level information
+
+Via the `Strategy Results` we provide flexibility for Strategy Authors to decide on particular results they would
+like to surface. All they need to do is:
+
+- Define the result in their strategy, under `spec.results`, this consist of a name and a description.
+- Populate or Emit the result to `'$(results.<result-name>.path)'`
+
+#### As a Shipwright Build Developer, I want to surface Build meaningful results
+
+Shipwright Developers should conclude on which results defined at runtime will belong under `status.stepsResult`. Today, we generate the following:
+
+- shp-image-size
+- shp-image-digest
+- shp-source-default-commit-sha -> This should be changed to `shp-commit-sha`.
+
+therefore we should populate `.status.stepsResult` with the following results names:
+
+```yaml
+ status:
+   stepsResult:
+   - name: shp-commit-sha
+     value: <sha>
+     step: source-default
+   - name: shp-image-size
+     value: <sha>
+     stepI: build-and-push
+   - name: shp-image-digest
+     value: <sha>
+     stepI: build-and-push
+```
+
+### As a Shipwright CLI developer, I want to consume surfaced results
+
+Workflows on top of Shipwright should be able to consume higher-level data and present that in different ways. As long as they consume from `.status.stepsResult` or `.status.results`.
+
+### Test Plan
+
+It should include:
+
+- Integration-tests
+- Unit-tests
+
+### Release Criteria
+
+Should be available before release v1.0.0
+
+### Risks and Mitigations
+
+- `Strategy Results` could pile up, ending-up in heavy BuildRun objects.
+- `Build Results` names tending to change over time, leading to breaking changes on the workflows consumers ( _e.g. CLI_ ). For this, we need to have
+a well defined naming convention for the results we define at runtime.
+
+## Drawbacks
+
+None.
+
+## Alternatives
+
+### For Results generated at Runtime Option 1
+
+- Keep `Strategy Results` under the _.status_ subresource of a BuildRun.
+
+- Introduce `commit-sha` under the _.status_ subresource of a BuildRun. This serves as the identifier for the version of the source code from which this image was built. If the version control system is **git**, then this is the SHA. This value is already available as of today, see related [code](https://github.com/shipwright-io/build/blob/main/pkg/reconciler/buildrun/resources/sources/git.go#L24-L27). This will be looking as follows:
+
+   ```yaml
+    status:
+      commit-sha: <sha>
+   ```
+
+- Introduce `image-digest` under the _.status_ subresource of a BuildRun. This serves as the _sha256_ of the generated container image. This value is already available as of today, see an [strategy example](https://github.com/shipwright-io/build/blob/main/samples/buildstrategy/buildpacks-v3/buildstrategy_buildpacks-v3_cr.yaml#L52). This will be looking as follows:
+
+   ```yaml
+    status:
+      image-digest: <sha>
+   ```
+
+### For Results generated at Runtime Option 2
+
+- Keep `Strategy Results` under the _.status_ subresource of a BuildRun.
+
+- Introduce `sources` and `output` under the _.status_ subresource of a BuildRun. This will be looking as follows:
+
+   ```yaml
+    status:
+      sources:
+        - name: default
+          git:
+            commit-sha: abcd
+      output-image:
+        digest: sha:30213102
+   ```
+
+## Implementation History
+
+There is already a WIP [branch](https://github.com/shipwright-io/build/pull/787) that aims to implement this feature, but is currently on hold, waiting
+for this SHIP to be approved.
+
+[Local Source Upload]: https://github.com/shipwright-io/community/pull/11