-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Reserve /tekton/ paths and "tekton-internal-" volume names #1701
Conversation
The following is the coverage report on pkg/.
|
@imjasonh needs a rebase 👼 🙏 |
cb44002
to
b636792
Compare
The following is the coverage report on pkg/.
|
#1700 is merged, so I think we can remove the hold /hold cancel |
Tests currently fail because we had previously allowed users to define their own I think that just means we need to keep allowing those to be mounted over, but not fully-internal paths like I'll try to fix this up today or tomorrow. |
b165cca
to
5c542a4
Compare
/lgtm |
/lgtm |
This prevents collisions between user-specified volumes and Tekton-internal volumes used to support execution. Some validation changes: - volume names starting with "tekton-internal-" are not valid - volumeMounts that mount at /tekton/* are not valid Tekton's own internal volume mounts are already mounted at /tekton/*, and now all volume names start with "tekton-internal-" Some Tekton-internal volume names were previously randomized to prevent collisions, which is no longer necessary, so they're no longer randomized. creds-init mounts annotated K8s secrets into the creds-init process. Previously those were mounted at /var/build-secrets/* (a relic of knative/build times). Now those are mounted at /tekton/creds-secrets/* Some init container names were also randomly generated, which is unnecessary, so that's gone too.
5c542a4
to
e1d2c60
Compare
/lgtm |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vdemeester 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 |
Reserve /tekton/ paths and "tekton-internal-" volume names
This prevents collisions between user-specified volumes and
Tekton-internal volumes used to support execution.
Some validation changes:
/tekton/home
, which we allow users to mount over, for instance if they want a PVC volume to persist the default home directory.Tekton's own internal volume mounts are already mounted at /tekton/*,
and now all volume names start with "tekton-internal-"
Some Tekton-internal volume names were previously randomized to prevent
collisions, which is no longer necessary, so they're no longer
randomized.
creds-init mounts annotated K8s secrets into the creds-init process.
Previously those were mounted at /var/build-secrets/* (a relic of
knative/build times). Now those are mounted at /tekton/creds-secrets/*
Some init container names were also randomly generated, which is
unnecessary, so that's gone too.
/hold
dependent of #1700
Fixes #1304
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmd
dir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes