Skip to content
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

Add url normalization to generator command. #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ximes
Copy link

@ximes ximes commented Nov 6, 2023

The generate command fails if the local stack repo has been set up with ssh urls instead of https urls.

Steps to reproduce

  1. Set up a new local repo for reclaim-the-stack setting its origin to a ssh url
    e.g. git remote add origin [email protected]:your-user/your-stack.git)
  2. Create a new k context
    here, k will error if we try to insert a ssh url, so we may add https://github.com/your-user/your-stack.git instead, to pass this step).
    However:
  3. Run k generate [any args]
    A parsing error like: bad URI(is not URI?): "[email protected]:your-user/your-stack.git" (URI::InvalidURIError) is raised.

PR patches step 3 by comparing a 'normalized' version of the repo path instead.
If this is needed, there may be more places where it can be added - e.g. during setup of the k context.

👍 / 👎

@dbackeus
Copy link
Contributor

dbackeus commented Nov 8, 2023

@ximes is this intended to support ssh urls for k context:add as well or just the one on your own gitops repo clone?

@dbackeus
Copy link
Contributor

dbackeus commented Nov 8, 2023

If the latter the impementation can be as simple as:

context_repo = URI K_CONTEXT.fetch("repository").delete_suffix(".git")
current_repo = `git remote get-url origin`.strip.delete_suffix(".git")

unless current_repo.end_with? context_repo.path.delete_prefix("/")

@p-wall
Copy link
Contributor

p-wall commented Jun 18, 2024

+1, would be useful for the k contexts to support ssh urls

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.

3 participants