Skip to content

Commit

Permalink
Add EP on Build Strategies Parametrization
Browse files Browse the repository at this point in the history 
This covers the use case introduced in issue
shipwright-io#184

This also will provide an enhancement in the current Build API, by
migrating existing fields into a more readable/understandable path.
  • Loading branch information
@qu1queee
qu1queee committed Mar 29, 2021
1 parent c27504c commit 7c567c4
Showing 1 changed file with 269 additions and 0 deletions.
269 changes: 269 additions & 0 deletions docs/proposals/parameterize-strategies.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
<!--
Copyright The Shipwright Contributors
SPDX-License-Identifier: Apache-2.0
-->

---
title: Parameterize Build Strategies
authors:
- "@qu1queee"
reviewers:
- "@SaschaSchwarze0"
- "@ImJasonH"
- "@sbose78"
approvers:
- "@SaschaSchwarze0"
- "@ImJasonH"
creation-date: 2021-03-25
last-updated: 2021-03-26
status: implementable
---

# Parameterize Build Strategies

## Release Signoff Checklist

- [x] Enhancement is `implementable`
- [ ] 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]

> 1. This will break the Build API.
When should we do this? Do we need backwards compatibility?

> 2. This will force strategy administrators to document required Parameters.
How to do this?

## Summary

Today we use a feature of Tekton call [Parameters](https://github.com/tektoncd/pipeline/blob/main/docs/tasks.md#specifying-parameters) when defining a `Task` spec. This feature allow us to populate a Parameter value with the users specifications from the `Build` API. In this way, we can manipulate on each strategy step arguments (_**arg** key_).

## Motivation

There are several reasons for the need of a more well define parameterization mechanism:

- **Extensibility**: The current approach locks all strategies into three parameters, we need to allow N parameters if needed, depending on the strategy administrator requirement.

- **API enhancement**: There are some fields in the Build API, like `spec.builder.image` that could belong to a parameterize block. Reserving a parent field in the Build API for a use-case that is not present across strategies is not ideal, it just generate complexity on the API level.

- **Flexibility**: Providing a new API block on the `Build` or `BuildRun` for defining N parameters, will provide more flexibility for strategy administrators when defining new strategies. This will also simplify any potential documentation around "How to build a strategy?".

- **Transparency**: At the moment is not clear which are the default parameters across strategies, for both strategy admins and Build users. Using Tekton Params nomenclature in the strategies will make it clearly.

### Goals

- Introduce a `spec.params` field across Build and BuildRun API´s, as a way for users to provide N parameters according to their preferred strategy of choice. Defining `spec.params` in a BuildRun overrides any definition of the same parameter in Build´s.

- Revisit what we consider today as default Parameters that are defined on runtime when creating `TaskRuns`. These are `DOCKERFILE`, `CONTEXT_DIR` and `BUILDER_IMAGE`. Decide if we need to reduce or extend the default definitions.

- Expose the usage of Tekton Params directly on the strategies, in favor of simplicity and readability. This boils down to doing `inputs.params.DOCKERFILE` instead of `build.dockerfile`.

### Non-Goals

- Build from scratch a paremeterize mechanism, by replacing the one available in Task/TaskRuns.

- Provide support for Tekton Params of the type `array`.

## Proposal

### Part 1: Introduce spec.params

Introduce the `spec.params` field across Build and BuildRun resources. This will define a list of parameters definition, of the type `string`. This new `spec.params` does not provide support for Tekton params of the type `array`. This field can only be use, if the parameter is supported in the strategy of choice.

For example:

```yaml
apiVersion: shipwright.io/v1alpha1
kind: ClusterBuildStrategy
metadata:
name: a-cluster-strategy
spec:
buildSteps: #Content omitted for this example
params:
- name: a-param
description: A description of this parameter definition.
default: The default parameter value.
```
```yaml
apiVersion: shipwright.io/v1alpha1
kind: Build
metadata:
name: a-build
spec:
source: #Content omitted for this example
strategy:
name: a-strategy-with-params
output: #Content omitted for this example
params:
- name: a-param
value: A parameter value.
```
```yaml
apiVersion: shipwright.io/v1alpha1
kind: BuildRun
metadata:
name: a-buildrun
spec:
buildRef:
name: a-build
params:
- name: a-param
value: Another parameter value because my build is not up-to-date.
```
_Note_: If a **Buildrun** specifies `a-param` via its `spec.params` ,this will override the value defined in the `a-build`. In other words, BuildRuns have a higher priority when defining parameters.

### Part 2: Runtime Parameters

As mentioned in the Goals section, we currently define three parameters that can be constructed on runtime, also know as _runtime parameters_. Runtime parameters are define during runtime, on the creation of a Taskrun, where the paremeter definition and the parameter value mapping takes place. These runtime parameters are:

- DOCKERFILE
- CONTEXT_DIR
- BUILDER_IMAGE (_optional: It only take place if the spec.builder.image is defined_)

This EP propose to stop using `BUILDER_IMAGE` as a runtime Parameter but rather to delegate its definition to user of N strategy. This means `BUILDER_IMAGE` should be defined under `spec.params` in the future.

The list of runtime parameters will then look as follows:

- DOCKERFILE
- CONTEXT_DIR

_Note_: This should not mean we lock-in our runtime parameter, as we move on with the adoption of more tools, we might need to increase the amount of runtime parameters in the future.

### Part 3: Runtime Parameters

As referenced in [issue 694](https://github.com/shipwright-io/build/issues/694), we should remove the internal plumbing we do at runtime to map Build API definition´s in the strategy, in favor of the Tekton nomenclature. For example:

_In the Kaniko strategy `build-and-push` step args:_

Instead of doing:

```yaml
--context=/workspace/source/$(build.source.contextDir)
```

we should do:

```yaml
--context=/workspace/source/$(PARAMS.CONTEXT_DIR)
```

### User Stories [optional]

Strategy authors will be able to define and document the usage of N parameters in their strategies. Build users will then need to define the required parameter values if they want to opt-in for the usage of certain strategies.

#### BuildKit Story

For [`Buildkit`](https://github.com/moby/buildkit), which is another tool for doing in-cluster builds, the usage of the runtime parameters(DOCKERFILE,CONTEXT_DIR) would not be enough. Therefore, the usage of the parameters block, fits in the BuildKit strategy.

An strategy admin will require to define the following strategy:

```yaml
---
apiVersion: shipwright.io/v1alpha1
kind: ClusterBuildStrategy
metadata:
name: buildkit
spec:
params:
- name: DOCKERFILE_NAME
description: Name of your Dockerfile inside the DOCKERFILE context
default: "Dockerfile"
buildSteps:
- name: build-and-push
image: ...
command:
- buildctl-daemonless.sh
args:
- --debug
- build
- --progress=plain
- --frontend=dockerfile.v0
- --opt
- filename=$(params.DOCKERFILE_NAME)
- --local
- context=/workspace/source/$(params.CONTEXT_DIR)
- --local
- dockerfile=/workspace/source/$(params.DOCKERFILE)
- --output
- type=image,name=$(build.output.image),push=true
- --export-cache
- type=inline
- --import-cache
- type=registry,ref=$(build.output.image)
```

while a user of the above strategy, will require to provide a value for `DOCKERFILE_NAME` if the default is not enough.

```yaml
apiVersion: shipwright.io/v1alpha1
kind: Build
metadata:
name: buildkit-build
spec:
source: #Content omitted for this example
strategy:
name: buildkit
output: #Content omitted for this example
params:
- name: DOCKERFILE_NAME
value: "FoobarDockerfile"
```

### Implementation Details/Notes/Constraints [optional]

@ImJasonH already have an implementable prototype via [link](https://github.com/shipwright-io/build/compare/master...ImJasonH:params), which fits this EP. This already describes the core logic.

This implementation requires the [Build API](../../pkg/apis/build/v1alpha1/build_types.go), the [BuildRun API](../../pkg/apis/build/v1alpha1/buildrun_types.go) and the [Strategy API](../../pkg/apis/build/v1alpha1/buildstrategy.go) to support the `params` field.

This implementation also seeks to remove this [logic](https://github.com/shipwright-io/build/blob/master/pkg/reconciler/buildrun/resources/taskrun.go#L35-L49) in favor of simplicity and readability, as mentioned in the GOALS section.

### Risks and Mitigations

- API breaking change when removing the `spec.builder.image`. _Note_: this feature might not be widely adopted.

- Strategy administrators require a way to communicate parameters needed to operate with certain strategies.

- Not doing this, will lock Shipwright users and Strategy administrators to the current three runtime parameters we define today.

## Design Details

### Test Plan

- This requires new unit and integration tests.

### Graduation Criteria

Should be part of any release before our v1.0.0

##### Removing a deprecated feature

Yes, this migrate `spec.builder.image` into a the `spec.params`.

### Upgrade / Downgrade Strategy

For a running Shipwright deployment, no change is required. For a running `Build` with a reference to `spec.builder.image` and X strategy, we recommend the `Build` to be recreated after the strategy X is compliant with the builder image parameter, based on the parameter name in order to specify a builder image.

### Version Skew Strategy

N/A

## Implementation History

N/A

## Drawbacks

None

## Alternatives

None at the moment.

0 comments on commit 7c567c4

Please sign in to comment.