cli: tilde expansion for validate command #5577
Conversation
Signed-off-by: santhoshivan23 <santhoshivan23@gmail.com>
| @@ -117,6 +118,11 @@ func (c *Cmd) Run(k *kong.Context, _ logging.Logger) error { | |||
| c.CacheDir = filepath.Join(currentPath, c.CacheDir) | |||
| } | |||
|
|
|||
| if strings.HasPrefix(c.CacheDir, "~/") { | |||
There was a problem hiding this comment.
As I mentioned in the issue, this only solves the base case to return the current user's home directory, which I admit is likely the most common by far. It does not solve the issue for the case where a specific other-user's home directory is referenced, as in --cache-dir=~otheruser/.cache. I looked around a little and there does not seem to be an official way to do this in go, all of the solutions I saw only reference the base case.
I think it's unfortunate that the CLI package prints the help message with the = sign instead of a space, and includes the quotes around the default value for the cache. The combination of those two things makes it unlikely that the user will try --cache-dir ~/.crossplane/cache which works fine in most modern shells.
I wonder if it would make sense to change the default cache location to ~/.crossplane/cache so the user doesn't end up with a bunch of cache directories lying around, since that sort of defeats the purpose of having the cache in the first place. That can be a separate issue/PR.
Given that the scope of this change is limited by available go solutions I think it's fine, it's just unfortunate that go hasn't chosen a better path for CLI argument handling.
|
Also @santhoshivan23 please finish the checklist by crossing out the lines you don't need/plan to complete. Thanks! |
|
@bobh66 thanks for the insights! I can see that the e2e tests check is failing. Is there anything that I can do about it? |
santhoshivan23
changed the title
There was a problem hiding this comment.
Took this for a test spin using the repro scenario in https://github.com/jbw976/demo-xfn and a locally built crank binary with the code in this PR. It looks to be working OK for me @santhoshivan23! 🙇♂️
# remove the cache directory
❯ rm -fr ~/.crossplane/cache
❯ ll ~/.crossplane/cache
ls: /Users/jared/.crossplane/cache: No such file or directory
# test this code and pass a dir under ~/
❯ crank-tilde beta render xr.yaml composition.yaml functions.yaml -x | crank-tilde beta validate extensions.yaml --cache-dir="~/.crossplane/cache" -
package schemas does not exist, downloading: xpkg.upbound.io/upbound/provider-aws-s3:v1.2.0
[✓] example.crossplane.io/v1, Kind=XR, example-xr validated successfully
[✓] s3.aws.upbound.io/v1beta1, Kind=Bucket, xbuckets-test-bucket validated successfully
Total 2 resources: 0 missing schemas, 2 success cases, 0 failure cases
# cache dir was populated OK in the right place under ~/
❯ ll ~/.crossplane/cache
total 0
drwxr-xr-x 3 jared staff 96B Apr 12 18:32 .
drwxr-xr-x 4 jared staff 128B Apr 12 18:32 ..
drwxr-xr-x 3 jared staff 96B Apr 12 18:32 xpkg.upbound.io
# cache dir is reused on a second run
❯ crank-tilde beta render xr.yaml composition.yaml functions.yaml -x | crank-tilde beta validate extensions.yaml --cache-dir="~/.crossplane/cache" -
[✓] example.crossplane.io/v1, Kind=XR, example-xr validated successfully
[✓] s3.aws.upbound.io/v1beta1, Kind=Bucket, xbuckets-test-bucket validated successfully
Total 2 resources: 0 missing schemas, 2 success cases, 0 failure cases
Description of your changes
The crossplane validate command has an arg names cacheDir that receives an path to the directory where the cache files for the validate process is stored. When the argument contains a value with tilde (~) that refers to the user's home directory, the tilde expansion doesn't happen implicitly by the shell. This has to be handled programmatically.
Fixes #5487
I have:
make reviewableto ensure this PR is ready for review.