buildpacks-community / kpack-cli

A command line interface for interacting with kpack.
Other
33 stars 15 forks source link

[Issue-1119] Adds additional validation that `--git-url` does not contain the repository path #329

Closed fcaroline2020 closed 1 year ago

fcaroline2020 commented 1 year ago
chenbh commented 1 year ago

I think we also need to handle the ssh case, both git@github.com:owner and git@github.com:owner/repo should fail validation.

But then having a port in an http scheme should be valid (https://github.enterprise:1234). There's also esoteric case like query strings (https://github.com?query=foo.bar).

I wonder if we should just say screw it, try to parse it via url.Parse, and extract the Host portion from there. Would the UX be okay as long as we print the final parsed host/domain?

chenbh commented 1 year ago

I wonder if we should just say screw it, try to parse it via url.Parse, and extract the Host portion from there. Would the UX be okay as long as we print the final parsed host/domain?

Turns out url.Parse doesn't like the colon in the ssh shorthand form git@github.com:org/repo

fcaroline2020 commented 1 year ago

@chenbh I'll add the ssh case and give you a heads up once its ready

fcaroline2020 commented 1 year ago

@chenbh according to kp-cli help doc

 "--git-url" and "--git-ssh-key" to create SSH based git credentials.
  "--git-url" should not contain the repository path (eg. git@github.com not git@github.com:my/repo)
  Alternatively, provided the credentials in the "GIT_SSH_KEY_PATH" env var instead of the "--git-ssh-key" flag.

  "--git-url" and "--git-user" to create Basic Auth based git credentials.
  "--git-url" should not contain the repository path (eg. https://github.com not https://github.com/my/repo) 

From this and your suggestion above I have added validation logic and the following test cases

Basic auth case:

valid urls

invalid urls

ssh case:

valid url

Invalid url

The case with query e.g https://github.com?query=foo.bar will fail with the current logic.