carvel-dev
Using newRootPath and legalPaths together result in missing LICENSE file
What steps did you take: [A clear and concise description steps that can be used to reproduce the problem.]
A typical GitHub project generally includes its license at the root of the project. When specifying the newRootPath, this will adjust the root path of the project after the download and since the legalPath likely exists outside of newRootPath, the legalPath ends up missing.
Consider this config:
---
apiVersion: vendir.k14s.io/v1alpha1
kind: Config
directories:
# vertical pod autoscaler
- path: source/cnpo.tanzulabs.vmware.com/tenancy/vertical-pod-autoscaler
contents:
- path: vendor
git:
url: https://github.com/kubernetes/autoscaler.git
ref: vertical-pod-autoscaler/v0.9.2
includePaths:
- vertical-pod-autoscaler/deploy/*.yaml
excludePaths:
- vertical-pod-autoscaler/deploy/kustomization.yaml
- vertical-pod-autoscaler/deploy/vpa-beta-crd.yaml
- vertical-pod-autoscaler/deploy/vpa-beta2-crd.yaml
- vertical-pod-autoscaler/deploy/vpa-crd.yaml
newRootPath: vertical-pod-autoscaler/deploy
legalPaths:
- LICENSE
What happened: [A small description of the issue]
Directory was appropriately downloaded, however the legalPaths were missing because (assumption) it existed outside of the newRootPath
What did you expect: [A description of what was expected]
The legalPaths would be placed within the newRootPath specified.
Anything else you would like to add: [Additional information that will assist in solving the issue.]
Environment:
- vendir version (execute
vendir --version):
vendir --version
vendir version 0.19.0
Succeeded
- OS (e.g. from
/etc/os-release): MacOS
Vote on this request
This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.
👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"
We are also happy to receive and review Pull Requests if you want to help working on this issue.
this one is tricky. do you think we should just drop these files in? should we error if there are existing files with matching names?, im thinking we can add:
newRootPath: ...
newRootIncludeLegal: true # <---
Yeah, understood this could get a bit tricky. I think that newRootIncludeLegal would definitely work. I'm trying to think if there are any other use cases, beyond legal, in which this may apply, but I can't think of any.
Thinking out loud, if there wasn't a newRootIncludeLegal, I would expect the legalPaths to simply show up in the defined newRootPath unless there is an overlapping file (e.g. the newRootPath contains a LICENSE as well as the legalPaths), in which case an error would be produced.
Thinking out loud, if there wasn't a newRootIncludeLegal
For clarification, do you mean if the configuration wasn't provided by the user and defaulted to false, or if it didn't even exist in the first place?
What I am seeing here is we will introduce a configuration that provides users the option to include legal paths when changing root, but that will default to false, so users need to be explicit that they want the legal paths preserved. Is that correct?
Thinking out loud, if there wasn't a newRootIncludeLegal
For clarification, do you mean if the configuration wasn't provided by the user and defaulted to false, or if it didn't even exist in the first place?
What I am seeing here is we will introduce a configuration that provides users the option to include legal paths when changing root, but that will default to false, so users need to be explicit that they want the legal paths preserved. Is that correct?
I'm thinking that if the legalPaths is specified, the user MUST explicitly set newRootIncludeLegal: true in order to preserve the legal paths. Otherwise the behavior is exactly as it exists today.
The one corner case that I'm juggling in my head is, what happens when the newRootPath includes the same document as the legalPaths does? Should it error out? Or should the newRoothPath document take precedence? My first instinct is to tell the user to fix their configuration and that there is a problem.