SHIP-0034: Label propagation #86
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| <!-- | ||
| Copyright The Shipwright Contributors | ||
|
|
||
| SPDX-License-Identifier: Apache-2.0 | ||
| --> | ||
|
|
||
| --- | ||
| title: propagating-labels-to-the-pod | ||
| authors: | ||
| - "@Pinolo" | ||
| reviewers: | ||
| - TBD | ||
| approvers: | ||
| - TBD | ||
| creation-date: 2022-05-12 | ||
| last-updated: 2022-05-15 | ||
| status: - | ||
| see-also: | ||
| - "/ships/0010-buildstrategy-annotation-propagation.md" | ||
| --- | ||
|
|
||
| # Propagating labels to the pod | ||
|
|
||
| ## Release Signoff Checklist | ||
|
|
||
| - [ ] Enhancement is `implementable` | ||
| - [ ] Design details are appropriately documented from clear requirements | ||
| - [ ] Test plan is defined | ||
| - [ ] Graduation criteria for dev preview, tech preview, GA | ||
| - [ ] User-facing documentation is created in [docs](/docs/) | ||
|
|
||
| ## Open Questions | ||
|
|
||
| 1. Do we want to allow all of `BuildStrategy`, `ClusterBuildStrategy`, `Build`, `BuildRun` to be able to define labels to be propagated to the Tekton `TaskRun` `Pod`? | ||
| 1. In case of multiple allowed label sources, what is the hierarchy for overrides? (e.g. `ClusterBuildStartegy` or `BuildStrategy` > `Build` > `BuildRun`) | ||
| 1. Which label prefixes do we want to deny-list? | ||
|
|
||
| ## Summary | ||
|
|
||
| The labels set for a Shipwright build resource will be passed to the generated Tekton `TaskRun`, and from there to the corresponding `Pod`. | ||
|
|
||
| ## Motivation | ||
|
|
||
| Labels are useful to administrators and developers, in order to be able to filter resources. An example use case is the need for getting logs of a specific group of `BuildRun`s, based on e.g. a label identifying a certain service. | ||
|
|
||
| ### Goals | ||
|
|
||
| - Enable users to define labels on `ClusterBuildStrategy`,`BuildStrategy`, `Build`, `BuildRun`, that are copied to the `BuildRun`'s `Pod` | ||
|
|
||
| ### Non-Goals | ||
|
|
||
| Propagation for annotations has aleady been implemented. | ||
|
|
||
| ## Proposal | ||
|
|
||
| Shipwright resources administrators can define labels in the metadata, as with all Kubernetes objects, see the [Labels](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/) topic in the Kubernetes documentation. | ||
|
|
||
| As we did with annotations, we reserve certain label prefixes that are either used by Kubernetes or Tekton or Shipwright's controller: | ||
|
|
||
| * `?kubernetes.io` | ||
| * `?k8s.io` | ||
| * `tekton.dev` | ||
| * `*.shipwright.io` | ||
|
|
||
| When generating a Tekton `TaskRun`, the idea is to look at the labels of the `BuildStrategy` or `ClusterBuildStrategy` or `Build` or `BuildRun` and copy all labels over to the `TaskRun`, except those that use one of the reserved prefixes. | ||
|
|
||
| Valid labels found along the `strategy > build > run` hierarchy will be merged. | ||
|
|
||
| Tekton automatically copies all `TaskRun` labels to the `Pod`, see [pod.go](https://github.com/tektoncd/pipeline/blob/v0.35.1/pkg/pod/pod.go#L377). | ||
|
|
||
| For example, this metadata of a build: | ||
|
|
||
| ```yaml | ||
| apiVersion: shipwright.io/v1alpha1 | ||
| kind: Build | ||
| metadata: | ||
| labels: | ||
| somelabelkey: somelabelvalue | ||
| custom.prefix.io/somelabelkey: somelabelvalue | ||
| build.shipwright.io/somelabelkey: Value | ||
| ``` | ||
|
|
||
| will lead to the following metadata on the `TaskRun` (and `Pod`): | ||
|
|
||
| ```yaml | ||
| apiVersion: tekton.dev/v1beta1 | ||
| kind: TaskRun | ||
| metadata: | ||
| labels: | ||
| somelabelkey: somelabelvalue | ||
| custom.prefix.io/somelabelkey: somelabelvalue | ||
| ``` | ||
|
|
||
| ### Implementation Notes | ||
|
|
||
| The implementation requires the [BuilderStrategy interface](../../pkg/apis/build/v1alpha1/buildstrategy.go) to be extended with a `GetLabels` functions that is implemented in the [BuildStrategy](../../pkg/apis/build/v1alpha1/buildstrategy_types.go) and [ClusterBuildStrategy](../../pkg/apis/build/v1alpha1/clusterbuildstrategy_types.go) types by returning the object's labels. | ||
|
There was a problem hiding this comment. You only need to define the function on the interface. The implementation is already available on the Kubernetes generated types. |
||
|
|
||
| It also requires `GetLabels` functions to be implemented in the [Build](../../pkg/apis/build/v1alpha1/build_types.go) and [BuildRun](../../pkg/apis/build/v1alpha1/buildrun_types.go) types. | ||
|
There was a problem hiding this comment. This is not needed because the Build and BuildRun type are Kubernetes objects which have those functions available by default. |
||
|
|
||
| The assignment of the `TaskRun` labels needs to be done in the [generate_taskrun.go](../../pkg/reconciler/buildrun/resources/taskrun.go) file in the `GenerateTaskRun` function. The labels from the build strategy need to be copied to the `TaskRun` except those with reserved prefixes mentioned under [Proposal](#proposal). | ||
|
|
||
| Making sure the assignment takes place in the desired order (first labels from `BuilderStrategy`, then from `Build`, and finally from `BuildRun`) will allow cascading overrides. | ||
|
Comment on lines
+100
to
+102
There was a problem hiding this comment. You should merge these two paragraphs to make it more obvious. Currently the first paragraph mentions that labels only from the build strategy need to be copied. Your second paragraph discussed the override behavior. |
||
|
|
||
| ### Test Plan | ||
|
|
||
| TBD | ||
|
There was a problem hiding this comment. I think unit and integration tests are necessary. |
||
|
|
||
| ### Release Criteria | ||
|
|
||
| TBD | ||
|
|
||
| #### Upgrade Strategy [if necessary] | ||
|
|
||
| N/A | ||
|
|
||
| ### Risks and Mitigations | ||
|
|
||
| N/A | ||
|
|
||
| ## Drawbacks | ||
|
|
||
| N/A | ||
|
|
||
| ## Alternatives | ||
|
|
||
| N/A | ||
|
|
||
| ## Infrastructure Needed [optional] | ||
|
|
||
| N/A | ||
|
|
||
| ## Implementation History | ||
|
|
||
| N/A | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should here discuss the differences between labels and annotations. Conceptionally. You already link to the Kubernetes documentation about labels. There's a topic about annotations as well. The difference is clearly stated. While labels are described as Labels enable users to map their own organizational structures onto system objects in a loosely coupled fashion, without requiring clients to store these mappings., annotations (https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/) have a broader scope inclusing Directives from the end-user to the implementations to modify behavior or engage non-standard features.
Based on Kubernetes' definition, one can easily conclude that labels are safe to be propagated because they cannot be used to possibly enable unwanted behavior.
The caveat/risk you can mention is that implementations are out there which want to modify behavior, but also require the filtering capabilities of labels - and therefore use labels for behavior. Istio is an example, https://istio.io/latest/docs/setup/additional-setup/sidecar-injection/#controlling-the-injection-policy.
And that's where it becomes interesting. I personally still think that we can do propagation also from Build and BuildRun.
Our general strategy on that is that we delegate such things to policy management solutions (though we don't have a SHIP for this, like Tekton has, https://github.com/tektoncd/community/blob/main/teps/0035-document-tekton-position-around-policy-authentication-authorization.md).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the insightful digression. I see your point in adding a little background about different purpose for labels and annotations, and I will update the document accordingly.
Regarding the caveat/risk discussion, I see that I actually jumped to a conclusion (to protect shipwright's own dependencies labels) which is quite arbitrary. Considering the main purpose of labels (filtering), I believe use cases may arise where users want to add/override tekton or even k8s key prefixes. Also, the Istio situation is quite tricky, but maybe we don't want to handle it, and delegate it to policy management, as you suggest, possibly in a documented way.
As for shipwright's own labels, I don't know…
Could a solomonic resolution be to allow adding prefixes to "protected" patterns, while disallowing overrides?