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

Set insecure_connection to target provider as default behavior. #327

Merged

Conversation

ghost
Copy link

@ghost ghost commented Jun 13, 2018

During tests of V2V solution, we identified that it is highly probable that the certificate presented by RHV-M is not valid. virt-v2v-wrapper has an option to work in insecure mode and this PR makes it the default behavior to avoid transformation to fail due to an invalid certificate.

@miq-bot
Copy link
Member

miq-bot commented Jun 13, 2018

Checked commit fabiendupont@9e9248d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

Copy link
Contributor

@mkanoor mkanoor left a comment

Choose a reason for hiding this comment

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

if possible can we pick up the secure connection from the method instance as a default value. Or put a TODO in this file to indicate that we want it to be configurable

@ghost
Copy link
Author

ghost commented Jun 13, 2018

@mkanoor It would mean changing the class schema. And this adds more complexity. Anyway, you're right it has to be configurable. I was going to create an issue to track the v2v configuration topic as it is broader than certificates validation. But we don't know yet what component will be targeted to store configuration.

@mkanoor
Copy link
Contributor

mkanoor commented Jun 13, 2018

@fdupont-redhat
I was thinking of adding a default value in the methods input parameter section and have it default to
secure_connection => false.
In the ruby method you would be using @handle.inputs['secure_connection'] || true

Its your choice, I am ok with just updating the method with a Git Issue pointing to that line in the method to make it configurable

@ghost
Copy link
Author

ghost commented Jun 13, 2018

@mkanoor
There's more than one way to do it, and that approach is elegant. Anyway, it's a broader topic about v2v configuration as a whole. At the moment, I'd rather have it hard coded and open an issue for 5.9.4.

@gmcculloug gmcculloug merged commit cd8a56f into ManageIQ:master Jun 13, 2018
@gmcculloug gmcculloug added this to the Sprint 88 Ending Jun 18, 2018 milestone Jun 13, 2018
simaishi pushed a commit that referenced this pull request Jun 13, 2018
…s_default

Set insecure_connection to target provider as default behavior.
(cherry picked from commit cd8a56f)
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 752d25cab22c8e0521f92f596bc9c54ca5d19b41
Author: Greg McCullough <[email protected]>
Date:   Wed Jun 13 11:43:18 2018 -0400

    Merge pull request #327 from fdupont-redhat/v2v_insecure_connection_as_default
    
    Set insecure_connection to target provider as default behavior.
    (cherry picked from commit cd8a56f09fc22a882c89abafbd4f7589409184af)

@ghost ghost deleted the v2v_insecure_connection_as_default branch July 31, 2018 07:15
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