Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map ImagePullPolicy from build strategy step to TaskRun step #791

Conversation

SaschaSchwarze0
Copy link
Member

Changes

When defining a build strategy, the author defines which images are to be used. For certain tools, beside immutable image tags for specific versions, there are also nightly or latest-like tags available that are not using latest themselves. In such a a case, Kubernetes sets the image pull policy implicitly to IfNotPresent and does never reload these images so that they get outdated. Examples in our context are:

In case of latest, Kubernetes would always contact the registry to check if there is a newer version. To enforce the very same behavior for other tags, the build strategy author needs to specify the Always image pull policy explicitly. But, our code does not pass this flag on to the TaskRun.

Given that this has positive security impact (assuming a newer image is more secure), but some performance cost that the build strategy author explicitly "opts in", I suggest that we pass on this field.

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

Release Notes

Build strategy authors can now specify an `imagePullPolicy` on each step, for example to enforce the `Always` policy also for images that do not use the `latest` tag

@openshift-ci openshift-ci bot added the release-note Label for when a PR has specified a release note label May 28, 2021
@openshift-ci openshift-ci bot requested review from otaviof and qu1queee May 28, 2021 15:20
@SaschaSchwarze0 SaschaSchwarze0 added the kind/feature Categorizes issue or PR as related to a new feature. label May 28, 2021
Copy link
Contributor

@qu1queee qu1queee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 . Shouldn´t we make this field usage more obvious? either by updating an existing strategy or docs?

@SaschaSchwarze0
Copy link
Member Author

+1 . Shouldn´t we make this field usage more obvious? either by updating an existing strategy or docs?

Good point. Let's set it for those strategy where this is applicable = that use mutable image tags.

Copy link
Contributor

@qu1queee qu1queee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, good one.
/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 31, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 31, 2021

[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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 31, 2021
@openshift-merge-robot openshift-merge-robot merged commit 5136c59 into shipwright-io:main May 31, 2021
@SaschaSchwarze0 SaschaSchwarze0 deleted the sascha-image-pull-policy branch May 31, 2021 09:01
@adambkaplan adambkaplan added this to the release-v0.5.0 milestone Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Label for when a PR has specified a release note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants