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 support to Embedded Ansible for ssh user@host:path urls #19129

Merged
merged 1 commit into from
Aug 9, 2019

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Aug 9, 2019

@carbonin @NickLaMuro Please review.

This is using the same regex that the UI uses [1], since I couldn't find a "standard" one.

[1] https://github.com/ManageIQ/manageiq-ui-classic/blob/e737cc675ee006c11508bc23f038c3357a1fb089/app/assets/javascripts/directives/url_validation.js#L21

@miq-bot
Copy link
Member

miq-bot commented Aug 9, 2019

Checked commit Fryguy@4019f7e with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Member

@NickLaMuro NickLaMuro left a comment

Choose a reason for hiding this comment

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

Comment from our out of band discussion.

@@ -9,7 +9,7 @@ class GitRepository < ApplicationRecord

attr_reader :git_lock

validates :url, :format => URI.regexp(%w[http https file ssh]), :allow_nil => false
validates :url, :format => Regexp.union(URI.regexp(%w[http https file ssh]), /\A[-\w:.]+@.*:/), :allow_nil => false
Copy link
Member

Choose a reason for hiding this comment

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

Should our custom regexp:

/\A[-\w:.]+@.*:/

Be a constant so that we can reference it else where? For parsing out user/pass/etc. later?

@carbonin carbonin merged commit 11ad9b8 into ManageIQ:master Aug 9, 2019
@carbonin carbonin added this to the Sprint 118 Ending Aug 19, 2019 milestone Aug 9, 2019
@Fryguy Fryguy deleted the embedded_ansible_ssh_user_host branch August 9, 2019 18:37
simaishi pushed a commit that referenced this pull request Aug 9, 2019
Add support to Embedded Ansible for ssh user@host:path urls

(cherry picked from commit 11ad9b8)
@simaishi
Copy link
Contributor

simaishi commented Aug 9, 2019

Ivanchuk backport details:

$ git log -1
commit 03a4c95a9558cc0d285501f845778f7c146e23bb
Author: Nick Carboni <ncarboni@redhat.com>
Date:   Fri Aug 9 14:07:45 2019 -0400

    Merge pull request #19129 from Fryguy/embedded_ansible_ssh_user_host
    
    Add support to Embedded Ansible for ssh user@host:path urls
    
    (cherry picked from commit 11ad9b8a4ae6690b03b6a62a5b51301bb3390d4d)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants