Bug 1900138: Removed support for insecure mode for oVirt/RHV installation #4404

ghost · 2020-11-23T12:31:00Z

This change removes support for the insecure mode from the oVirt installer. Previously, when no certificate could be obtained from the oVirt engine the installer would proceed without certificate verification. This PR is based on #4387 and depends on OCPRHV-426.

Due to recent improvements this is no longer a valid use case and is being deprecated. The user is instead presented with a message explaining the situation and linking to the to-be-written documentation. If the user wants to use insecure mode they have to create a file named ~/.ovirt/ovirt-config.yaml with the following contents before running the installer:

ovirt_url: https://ovirt.example.com/ovirt-engine/api
ovirt_fqdn: ovirt.example.com
ovirt_pem_url: ""
ovirt_username: admin@internal
ovirt_password: super-secret-password
ovirt_insecure: true

User experience

$ bin/openshift-install create install-config
? SSH Public Key /home/janoszen/.ssh/id_rsa.pub
? Platform ovirt
? Engine FQDN[:PORT] vm-10-208.lab.eng.tlv2.redhat.com
INFO Loaded the following PEM file:
INFO    Version: 3
INFO    Signature Algorithm: SHA256-RSA
INFO    Serial Number: 4096
INFO    Issuer: CN=ovirt.example.com.39197,O=example.com,C=US
INFO    Validity:
INFO            Not Before: 2020-08-11 13:50:26 +0000 UTC
INFO            Not After: 2030-08-10 13:50:26 +0000 UTC
INFO    Subject: CN=ovirt.example.com.39197,O=example.com,C=US
? Would you like to use the above certificate to connect to Engine?  No
? Would you like to import another PEM bundle? No
ERROR ****************************************************************************
ERROR * Could not configure secure communication to the oVirt engine.            *
ERROR * As of 4.7 insecure mode for oVirt is no longer supported from the        *
ERROR * installer. Please see ____LINK____ for details how to configure insecure *
ERROR * mode manually.                                                           *
ERROR ****************************************************************************
ERROR oVirt configuration failed: cannot detect Engine CA cert imported in the system.
? Engine FQDN[:PORT]

openshift-ci-robot · 2020-11-23T12:31:04Z

@janoszen: This pull request references Bugzilla bug 1900138, which is invalid:

expected the bug to target the "4.7.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

WIP: Bug 1900138: Removed support for insecure mode for oVirt/RHV installation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ghost · 2020-11-23T12:57:35Z

/bugzilla refresh

openshift-ci-robot · 2020-11-23T12:57:43Z

@janoszen: This pull request references Bugzilla bug 1900138, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ghost · 2020-11-23T13:02:21Z

/retest

ghost · 2020-11-23T14:39:28Z

/assign @Gal-Zaidman

openshift-ci-robot · 2020-11-23T14:40:21Z

@janoszen: This pull request references Bugzilla bug 1900138, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

WIP: Bug 1900138: Removed support for insecure mode for oVirt/RHV installation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ghost · 2020-11-26T08:07:55Z

@Gal-Zaidman this one's next. We may need QA to look at this before we merge and we still need text for it.

openshift-ci-robot · 2020-11-26T08:12:59Z

@janoszen: This pull request references Bugzilla bug 1900138, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

WIP: Bug 1900138: Removed support for insecure mode for oVirt/RHV installation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-11-26T08:13:39Z

@janoszen: This pull request references Bugzilla bug 1900138, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

WIP: Bug 1900138: Removed support for insecure mode for oVirt/RHV installation

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Gal-Zaidman · 2020-11-26T16:24:57Z

We may need QA to look at this before we merge and we still need text for it.

Can you also look at the commits I think something funky happened there

ghost · 2020-11-27T11:39:46Z

@Gal-Zaidman thanks, resolved the issue with the commits. QA got back to us and it looks good to them too, now we are just waiting for docs.

Gal-Zaidman · 2020-11-29T14:32:21Z

/retest

eslutsky

we need to make sure that LINK is persistent as it will be hardcoded in the code

ghost · 2020-11-30T15:33:47Z

@eslutsky the docs bug is here: https://issues.redhat.com/browse/OCPRHV-426

eslutsky · 2020-12-01T06:47:46Z

@eslutsky the docs bug is here: https://issues.redhat.com/browse/OCPRHV-426

thanks , replied in the bug

…ure mode for oVirt/RHV installation This change removes support for the insecure mode from the oVirt installer. Previously, when no certificate could be obtained from the oVirt engine the installer would proceed without certificate verification. Due to recent improvements this is no longer a valid use case and is being deprecated. The user is instead presented with a message explaining the situation and linking to the to-be-written documentation. If the user wants to use insecure mode they have to create a file named ~/.ovirt/ovirt-config.yaml with the following contents before running the installer: ```yaml ovirt_url: https://ovirt.example.com/ovirt-engine/api ovirt_fqdn: ovirt.example.com ovirt_pem_url: "" ovirt_username: admin@internal ovirt_password: super-secret-password ovirt_insecure: true ```

ghost · 2020-12-03T12:15:10Z

Link issue has been resolved in the latest update.

dougsland · 2020-12-03T19:09:50Z

Can we start review this one? I see WIP in the title.

dougsland · 2020-12-03T20:27:45Z

The patch looks good but I would exit the loop in case user reach the level of insecure as it's a critical thing and they would need to fix their env.

ghost · 2020-12-03T21:52:47Z

@dougsland not necessarily, they might just land in that branch by mistake. Very few people will actually want to use it without any verification, I can't even think of a single reason why you would want that.

dougsland · 2020-12-03T23:41:52Z

@dougsland not necessarily, they might just land in that branch by mistake.
Very few people will actually want to use it without any verification,
I can't even think of a single reason why you would want that.

If you believe it's a dead code/scenario, probably consider removing the entire condition.
However, I believe we cannot guarantee the scenario for every single user out there.

ERROR **************************************************************************** 
ERROR * Could not configure secure communication to the oVirt engine.            * 
ERROR * As of 4.7 insecure mode for oVirt is no longer supported in the          * 
ERROR * installer. Please see the help article titled "Installing OpenShift on   * 
ERROR * RHV/oVirt in insecure mode" for details how to configure insecure mode   * 
ERROR * manually.                                                                * 
ERROR **************************************************************************** 
ERROR oVirt configuration failed: cannot detect engine ca cert imported in the system

From my understanding:
This is a case where IPI installer detected an unsupported scenario and provided an error message to users.
On top of that, it's nothing something to solve in a blink of eyes (replaces/add certs) in the server with real users
depending on it (as might require maint window).

If you believe 100% users will achieve this code part as a mistake flow and want to continue the installer alive I would replace error with info message type.

peterclauterbach · 2020-12-04T16:34:15Z

From my understanding:
This is a case where IPI installer detected an unsupported scenario and provided an error message to users.
On top of that, it's nothing something to solve in a blink of eyes (replaces/add certs) in the server with real users
depending on it (as might require maint window).

If you believe 100% users will achieve this code part as a mistake flow and want to continue the installer alive I would replace error with info message type.

I think there is a case where you've made a genuine mistake in the certificates (like point to an expired one) , we should give you a chance to fix it, instead of failing immediately. The install will loop on this 3 times, and then fail. This seem like a good compromise of being forgiving, but not letting the user do something dangerous.

dougsland · 2020-12-04T16:40:19Z

From my understanding:
This is a case where IPI installer detected an unsupported scenario and provided an error message to users.
On top of that, it's nothing something to solve in a blink of eyes (replaces/add certs) in the server with real users
depending on it (as might require maint window).
If you believe 100% users will achieve this code part as a mistake flow and want to continue the installer alive I would replace error with info message type.

I think there is a case where you've made a genuine mistake in the certificates (like point to an expired one) , we should give
you a chance to fix it, instead of failing immediately. The install will loop on this 3 times, and then fail. This seem like a good
compromise of being forgiving, but not letting the user do something dangerous.

Moving forward as PM requested.

dougsland · 2020-12-04T16:40:25Z

/lgtm

dougsland · 2020-12-04T16:40:30Z

/approve

staebler · 2020-12-04T18:05:42Z

/approve

openshift-ci-robot · 2020-12-04T18:05:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dougsland, staebler

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~docs/user/ovirt/OWNERS~~ [staebler]
~~pkg/asset/installconfig/ovirt/OWNERS~~ [staebler]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2020-12-05T01:46:58Z

/retest