Skip to content

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

Merged
chenbh merged 1 commit intomainfrom
Issue-1119-git-flag-improvements
Sep 8, 2023
Merged

[Issue-1119] Adds additional validation that --git-url does not contain the repository path#329
chenbh merged 1 commit intomainfrom
Issue-1119-git-flag-improvements

Conversation

@fcaroline2020
Copy link
Contributor

  • Regex to check that there are no forward slashes after a period in the url
  • prints an error if url includes path git url should be a valid url without the repository path (ex. https://github.com)

@fcaroline2020 fcaroline2020 requested a review from a team as a code owner July 27, 2023 16:06
@chenbh
Copy link
Contributor

chenbh commented Aug 23, 2023

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
Copy link
Contributor

chenbh commented Aug 24, 2023

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
Copy link
Contributor Author

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

@fcaroline2020 fcaroline2020 force-pushed the Issue-1119-git-flag-improvements branch 2 times, most recently from c7c9ed5 to 6f45e38 Compare August 29, 2023 12:02
@fcaroline2020
Copy link
Contributor Author

fcaroline2020 commented Aug 29, 2023

@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.

@fcaroline2020 fcaroline2020 force-pushed the Issue-1119-git-flag-improvements branch 2 times, most recently from 6806b23 to 7d53af2 Compare September 4, 2023 12:51
…pository path for git url ssh and base auth case with additional tests

Signed-off-by: Caroline Scherf <fcaroline@vmware.com>
@fcaroline2020 fcaroline2020 force-pushed the Issue-1119-git-flag-improvements branch from 7d53af2 to 3276e72 Compare September 4, 2023 12:54
Copy link
Contributor

@chenbh chenbh left a comment

Choose a reason for hiding this comment

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

that's some nasty regex

@chenbh chenbh merged commit 846151e into main Sep 8, 2023
@chenbh chenbh deleted the Issue-1119-git-flag-improvements branch September 8, 2023 14:34
neil-hickey pushed a commit to neil-hickey/kpack-cli that referenced this pull request Jan 13, 2026
…/Issue-1119-git-flag-improvements

[Issue-1119] Adds additional validation that `--git-url` does not contain the repository path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants