[V2V] Conversion Host - Handle CA bundle from UI #18762

ghost · 2019-05-14T15:34:25Z

In current implementation, we look for the CA bundle in the provider configuration. However, for OpenStack provider, it's not possible to provide the certificate authority in the UI. So, we plan on asking the user to provide it in the conversion host wizard, until we have a way to provide it when configuring the provider. This PR adds support for an API param named ~~osp_ca_bundle~~ openstack_tls_ca_certs that will be passed to the backend, which will pass it as the v2v_ca_bundle extra var.

Associated RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1708359

ghost · 2019-05-14T15:35:07Z

@miq-bot add-label transformation, bug, hammer/yes
@miq-bot add-reviewer @djberg96

djberg96 · 2019-05-14T15:59:14Z

I don't think I like this. IMO we should modify ConversionHost::Configurations#enable. We already create an Authentication object. There's already a certificate_authority field in that table.

https://github.com/ManageIQ/manageiq/blob/master/app/models/conversion_host/configurations.rb#L73

Can we attach the CA to the same authentication? Or is there a reason not to associate it with a specific ssh key? I'm not that up on CA certs, so forgive my ignorance.

ghost · 2019-05-14T16:03:02Z

The authentication we create is for CloudForms to connect to the conversion host. The CA bundle will be deployed to the conversion host, so that it can connect to OpenStack endpoint. So, that's two different purpose. In the future, I hope we have a way to associate the CA bundle to the OpenStack provider authentication directly.

djberg96 · 2019-05-14T16:07:01Z

Oh, I see, it's passed up to the v2v wrapper.

djberg96 · 2019-05-14T16:07:50Z

app/models/conversion_host/configurations.rb

@@ -68,6 +68,8 @@ def enable(params, auth_user = nil)

      ssh_key = params.delete(:conversion_host_ssh_private_key)

+      osp_ca_bundle = params.delete(:osp_ca_bundle)


Not sure about this param name. @mturley what say you?

I don't have a strong opinion here. @fdupont-redhat, @djberg96 and I briefly discussed using openstack_tls_ca_certs to be consistent with the default_tls_ca_certs used by classic-ui for RHV. Not sure how much this name matters on the backend, but anything is fine for the UI.

Sounds good to me. We have the ems_type as the prefix and a common suffix with RHV.
I've changed it.

ghost · 2019-05-14T16:10:36Z

Not exactly. It's passed to the playbook that copies it on the conversion host and update CA trust database on the conversion host. This way, it's like a natively trusted certificate and all programs will trust it, including virt-v2v-wrapper.

djberg96 · 2019-05-14T17:03:08Z

@fdupont-redhat if the ca cert bundle is present, do we need to enable an ssl verify key of any sort?

ghost · 2019-05-14T17:35:47Z

@djberg96 nope. It's transparently used. Anyhow, that's a topic we need to revisit for 5.11 to make sure the whole chain is consistent and maybe add some options.

djberg96

Approving on the condition that @mturley is ok with the param name. :)

djberg96 · 2019-05-14T17:38:38Z

app/models/conversion_host.rb

@@ -202,7 +202,7 @@ def check_conversion_host_role(miq_task_id = nil)
    tag_resource_as('disabled')
  end

-  def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_private_key = nil, miq_task_id = nil)
+  def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_private_key = nil, osp_ca_bundle = nil, miq_task_id = nil)


Thinking about this now....is there anything that was explicitly passing in a task_id? If so, the param ordering change could be significant.

In the future I think we should change this to keyword params, but that's for another time.

I don't think that the order is really important there as nothing else is using it.
But you're right, passing a hash might be a good option, in a follow up.

@mturley is it possible the task_id is getting used on a retry or status check?

@djberg96 I don't think so, not from the UI anyway. We're only checking status by polling for all tasks, and retry just uses the request_params from the existing task to start a new enablement, it doesn't pass the task_id anywhere.

agrare · 2019-05-14T19:24:50Z

app/models/conversion_host.rb

@@ -211,7 +211,7 @@ def enable_conversion_host_role(vmware_vddk_package_url = nil, vmware_ssh_privat
      :v2v_transport_method => source_transport_method,
      :v2v_vddk_package_url => vmware_vddk_package_url,
      :v2v_ssh_private_key  => vmware_ssh_private_key,
-      :v2v_ca_bundle        => resource.ext_management_system.connection_configurations['default'].certificate_authority
+      :v2v_ca_bundle        => openstack_tls_ca_certs || resource.ext_management_system.connection_configurations['default'].certificate_authority


Hey @fdupont-redhat @djberg96 couple of things:

First:
resource.ext_management_system.connection_configurations['default'].certificate_authority

Unless I'm reading this wrong, that looks like it can be replaced by resource.ext_management_system.certificate_authority because we delegate :certificate_authority to the :default_endpoint which is all the connection_configurations['default'] does but you should try this yourself, untested

Second:
Why don't we just allow users to enter their certificate authority to the OSP provider and continue to do this in a provider agnostic way?

Hey @fdupont-redhat @djberg96 couple of things:

First:
resource.ext_management_system.connection_configurations['default'].certificate_authority

Unless I'm reading this wrong, that looks like it can be replaced by resource.ext_management_system.certificate_authority because we delegate :certificate_authority to the :default_endpoint which is all the connection_configurations['default'] does but you should try this yourself, untested

Nice one. I'll test in a follow-up PR.

Second:
Why don't we just allow users to enter their certificate authority to the OSP provider and continue to do this in a provider agnostic way?

The UI doesn't allow you to enter the certificate authority. We'll create an RFE. Unless you know how to do it and we missed it :)

I doubt you missed it, but given that RHV does this already it sounds like an easily solvable problem

Yes, RHV does it but I'm not sure how the backend would handle this. Today, to enable SSL validation with custom CA, you need to install the CA certs on the appliance manually (update-ca-trust), which is not very convenient at scale. We started the conversation on ManageIQ/manageiq-providers-openstack channel on Gitter.

Usually the API clients (aka fog) allow you to pass your CA when you initiate the connection so that you don't have to modify the system certificate authorities.

Since we don't have an issue with openshift refresh I'm going to assume they just allow self signed certs without verification (@aufi correct me if I'm wrong here) so this would be specifically for v2v.

I think that the main issue is that users will expect that the CA certificate is used by OpenStack provider code and that they don't have to install it manually. So, it's more about setting the wrong expectations with the new field.

And we can enable that in the future, I want to avoid having multiple ways of doing things.

Me too. That's definitely an RFE, but it requires more work to identify how it is leveraged by fog-openstack. Comment from @aufi on Gitter:

I wasn't able display ExtManagementSystem.connection_configuration['default'] which doesn't work for me on master (and ExtManagementSystem.connection_config contains MIQ database settings, if ovirt uses it also for provider connection settings, it might be OK, I just didn't know it).
Looking on fog-openstack (and excon which is used as HTTP client), there is :ssl_ca_path expecting path with CA files what means that CA certs should be stored in filesystem. I might have missed something, but so far I haven't found an easy way to pass CA cert content from MIQ DB to fog-openstack API calls, so it looks to be more complex to me.

@agrare I've created the RFE to keep track of this.

Okay I'm alright with this as is but really really really (3 reallys means it is serious) don't want to forget about moving this out of here and to the provider

miq-bot · 2019-05-16T12:25:44Z

Checked commits fabiendupont/manageiq@74d6dcc~...6616dea with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 👍

agrare

Okay for now, want to follow up to remove this for master and use the provider default endpoint CA

…dle_cacert_from_ui [V2V] Conversion Host - Handle CA bundle from UI (cherry picked from commit d3248d6) https://bugzilla.redhat.com/show_bug.cgi?id=1711036

simaishi · 2019-05-20T18:31:54Z

Hammer backport details:

$ git log -1
commit 836f43ebf98c17120465dfe518c47ebf88b6ce7d
Author: Adam Grare <[email protected]>
Date:   Thu May 16 10:58:49 2019 -0400

    Merge pull request #18762 from fdupont-redhat/v2v_conversion_host_handle_cacert_from_ui
    
    [V2V] Conversion Host - Handle CA bundle from UI
    
    (cherry picked from commit d3248d61d29d109f481818ed78dec5e87b6cba6f)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1711036

miq-bot added bug hammer/yes v2v labels May 14, 2019

miq-bot requested a review from djberg96 May 14, 2019 15:36

djberg96 reviewed May 14, 2019

View reviewed changes

djberg96 approved these changes May 14, 2019

View reviewed changes

djberg96 reviewed May 14, 2019

View reviewed changes

agrare reviewed May 14, 2019

View reviewed changes

This was referenced May 14, 2019

Conversion Host Wizard: Authentication Step - Add OpenStack Trusted CA Certificates field ManageIQ/manageiq-v2v#951

Closed

[#951] [BZ#1708359] Add OpenStack Trusted CA Certificates field to Conversion Host Wizard ManageIQ/manageiq-v2v#952

Merged

ghost mentioned this pull request May 15, 2019

[RFE] Allow setting the CA certificate for OpenStack provider #18766

Closed

fabiendupont added 2 commits May 16, 2019 08:20

Conversion Host - Handle CA bundle from UI

74d6dcc

Renaming osp_ca_bundle to openstack_tls_ca_certs

6616dea

ghost force-pushed the v2v_conversion_host_handle_cacert_from_ui branch from e1bdca2 to 6616dea Compare May 16, 2019 12:21

agrare self-assigned this May 16, 2019

agrare approved these changes May 16, 2019

View reviewed changes

agrare merged commit d3248d6 into ManageIQ:master May 16, 2019

agrare added this to the Sprint 112 Ending May 27, 2019 milestone May 16, 2019

simaishi added hammer/backported and removed hammer/yes labels May 20, 2019

miq-bot mentioned this pull request Aug 3, 2019

[RFE] Allow setting the CA certificate for OpenStack provider ManageIQ/manageiq-providers-openstack#492

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[V2V] Conversion Host - Handle CA bundle from UI #18762

[V2V] Conversion Host - Handle CA bundle from UI #18762

ghost commented May 14, 2019 •

edited by ghost

Loading

ghost commented May 14, 2019

djberg96 commented May 14, 2019

ghost commented May 14, 2019

djberg96 commented May 14, 2019

djberg96 May 14, 2019

mturley May 14, 2019

ghost May 14, 2019

ghost commented May 14, 2019

djberg96 commented May 14, 2019

ghost commented May 14, 2019

djberg96 left a comment

djberg96 May 14, 2019 •

edited

Loading

ghost May 14, 2019

djberg96 May 14, 2019

mturley May 14, 2019

agrare May 14, 2019

ghost May 14, 2019

agrare May 14, 2019

ghost May 14, 2019

agrare May 14, 2019

ghost May 15, 2019

agrare May 15, 2019

ghost May 15, 2019

ghost May 15, 2019

agrare May 16, 2019

miq-bot commented May 16, 2019

agrare left a comment

simaishi commented May 20, 2019

		@@ -68,6 +68,8 @@ def enable(params, auth_user = nil)

		ssh_key = params.delete(:conversion_host_ssh_private_key)

		osp_ca_bundle = params.delete(:osp_ca_bundle)

[V2V] Conversion Host - Handle CA bundle from UI #18762

[V2V] Conversion Host - Handle CA bundle from UI #18762

Conversation

ghost commented May 14, 2019 • edited by ghost Loading

ghost commented May 14, 2019

djberg96 commented May 14, 2019

ghost commented May 14, 2019

djberg96 commented May 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented May 14, 2019

djberg96 commented May 14, 2019

ghost commented May 14, 2019

djberg96 left a comment

Choose a reason for hiding this comment

djberg96 May 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented May 16, 2019

agrare left a comment

Choose a reason for hiding this comment

simaishi commented May 20, 2019

ghost commented May 14, 2019 •

edited by ghost

Loading

djberg96 May 14, 2019 •

edited

Loading