-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fix issue when username/password contain spaces #106
Conversation
Fix issue CentOS-PaaS-SIG#105 Signed-off-by: Barak Korren <bkorren@redhat.com>
Can one of the admins verify this patch? |
[test] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This looks good to me, but when I tried applying the same logic to add_scc.yml#L6, it failed. I'll have to test it some more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Who has merge rights? @dirgim strange it does not work in the line you mentioned - can I see you patch and the failed test? |
command: "{{ oc_bin }} login {{ cluster_ip|quote }} --username={{ username|quote }} --password={{ password|quote }} --insecure-skip-tls-verify=true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we leave this as a shell?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
safer this way - less chance of injection, also I suspect |quote
does not really work well with shell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@ifireball It's fine, I got it working. I'm good to merge this. |
Fix issue #105
Signed-off-by: Barak Korren bkorren@redhat.com