Change in vdsm[master]: vdsm-tool: add the cmd get-engine-web-ca-issuer

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has uploaded a new change for review.

Change subject: vdsm-tool: add the cmd get-engine-web-ca-issuer
......................................................................

vdsm-tool: add the cmd get-engine-web-ca-issuer

If oVirt Node is registered via oVirt Engine currently
we don't show to users which engine the node is registered.
This patch extracts the issuer field from engine web ca to use
the CN (commonname) data in ovirt-node-plugin-vdsm.

Change-Id: I70a19f54169498c53cabba0ac75e7a5611762ece
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1039267
Signed-off-by: Douglas Schilling Landgraf 
---
M lib/vdsm/tool/validate_ovirt_certs.py.in
1 file changed, 15 insertions(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/27651/1

diff --git a/lib/vdsm/tool/validate_ovirt_certs.py.in b/lib/vdsm/tool/validate_ovirt_certs.py.in
index 6a8c13f..149dc30 100644
--- a/lib/vdsm/tool/validate_ovirt_certs.py.in
+++ b/lib/vdsm/tool/validate_ovirt_certs.py.in
@@ -35,6 +35,7 @@
 PATH_CERT = '/etc/pki/vdsm/certs/'
 PATH_CACERT = PATH_CERT + 'cacert.pem'
 PATH_VDSMCERT = PATH_CERT + 'vdsmcert.pem'
+PATH_ENGINE_WEB_CA = PATH_CERT + 'engine_web_ca.pem'

def is_our_cafile(cacert):
@@ -60,3 +61,17 @@
                 os.chown(PATH_CACERT, uid, gid)
                 ovirtfunctions.ovirt_store_config(PATH_CACERT)
                 break
+
+
+@expose('get-engine-web-ca-issuer')
+def get_engine_web_ca_issuer():
+    """
+    Extract from engine web ca certificate the issuer field
+    """
+    if os.path.exists(PATH_ENGINE_WEB_CA):
+        cmd = [
+            EX_OPENSSL, 'x509', '-in', PATH_ENGINE_WEB_CA, '-noout', '-issuer'
+        ]
+        rc, out, err = execCmd(cmd, raw=True)
+        if not err:
+            print(out),

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-engine-web-ca-issuer
......................................................................

Patch Set 1: Verified+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-engine-web-ca-issuer
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8802/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8938/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8012/ : SUCCESS

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8836/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8972/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/8046/ : SUCCESS

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2: Verified+1

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/27651/2/lib/vdsm/tool/validate_ovirt_certs.py.in
File lib/vdsm/tool/validate_ovirt_certs.py.in:

Line 71:         cmd = [
Line 72:             EX_OPENSSL, 'x509', '-in', PATH_CACERT, '-noout', '-issuer'
Line 73:         ]
Line 74:         rc, out, err = execCmd(cmd, raw=True)
Line 75:         if not err:
It is better practice to check rc value.

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/27651/2/lib/vdsm/tool/validate_ovirt_certs.py.in
File lib/vdsm/tool/validate_ovirt_certs.py.in:

Line 71:         cmd = [
Line 72:             EX_OPENSSL, 'x509', '-in', PATH_CACERT, '-noout', '-issuer'
Line 73:         ]
Line 74:         rc, out, err = execCmd(cmd, raw=True)
Line 75:         if not err:
> It is better practice to check rc value.

It is bad practice - please fix.

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2: Code-Review-1

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2: Code-Review-1

(1 comment)

http://gerrit.ovirt.org/#/c/27651/2//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: If oVirt Node is registered via oVirt Engine currently
Line 10: we don't show to users which engine the node is registered.
Line 11: This patch extracts the issuer field from cacert.pem to use
Line 12: the CN (commonname) data in ovirt-node-plugin-vdsm.
why in the tool?? it really sounds like something that vdsm should expose. I would put it in vdsCaps or even new api

other than that, alonbl planned to as "registration tool" exactly for such issues like knowing the process better then the current scripts. as part of this tool, which relevant only for nodes, we can expose such api.

I really don't think vdsm-tool is right for that propose. please convince me that im wrong
Line 13: 
Line 14: Change-Id: I70a19f54169498c53cabba0ac75e7a5611762ece
Line 15: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1039267

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2:

why do we relay on ca name to be anything of engine? it is current implementation, but not something that should be kept nor added new dependencies.

if this is *THAT* important, the during host-deploy we extract the VdsmEnv.ENGINE_HOST and write it somewhere.

another option is for vdsm to write last N hosts ips at /var/lib once in a while as it receives connections, and we present this resolved.

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/27651/2//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: If oVirt Node is registered via oVirt Engine currently
Line 10: we don't show to users which engine the node is registered.
Line 11: This patch extracts the issuer field from cacert.pem to use
Line 12: the CN (commonname) data in ovirt-node-plugin-vdsm.
> why in the tool?? it really sounds like something that vdsm should expose.

The certificates has nothing to do with the Vdsm/Engine API. They are part of the transport, and may not exist at all (ssl=False).

The chief motivation of vdsm-tool is to control configuration of vdsm on the local host. It creates self-signed certs, and validates whether cacert really certify vdsmcert.

If there is a need to extract information from vdsm-internal files, vdsm-tool is the immediate suspect.

The commit message, however, should explain why this informaiton has to be extracted.
Line 13: 
Line 14: Change-Id: I70a19f54169498c53cabba0ac75e7a5611762ece
Line 15: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1039267

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/27651/2//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: If oVirt Node is registered via oVirt Engine currently
Line 10: we don't show to users which engine the node is registered.
Line 11: This patch extracts the issuer field from cacert.pem to use
Line 12: the CN (commonname) data in ovirt-node-plugin-vdsm.
> The certificates has nothing to do with the Vdsm/Engine API. They are part

I agree. but since registration tool for ovirt-node was on the list of things that we plan to add, I thought that this option could be good start. this verb should be exposed only if host runs as node, therefore, to not mess with node verbs and regular hosts verbs, separate tool for registration and manage node sounds good approach to me in that case
Line 13: 
Line 14: Change-Id: I70a19f54169498c53cabba0ac75e7a5611762ece
Line 15: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1039267

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/27651/2//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: If oVirt Node is registered via oVirt Engine currently
Line 10: we don't show to users which engine the node is registered.
Line 11: This patch extracts the issuer field from cacert.pem to use
Line 12: the CN (commonname) data in ovirt-node-plugin-vdsm.
> I agree. but since registration tool for ovirt-node was on the list of thin

The registration tool is planned to be ignorant of Vdsm and where vdsm stores its certificates, and it should also work on non-node setups.
Line 13: 
Line 14: Change-Id: I70a19f54169498c53cabba0ac75e7a5611762ece
Line 15: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1039267

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/27651/2//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: If oVirt Node is registered via oVirt Engine currently
Line 10: we don't show to users which engine the node is registered.
Line 11: This patch extracts the issuer field from cacert.pem to use
Line 12: the CN (commonname) data in ovirt-node-plugin-vdsm.
> The registration tool is planned to be ignorant of Vdsm and where vdsm stor

this verb (ca_issuer) plans to serve only registration flow iiuc. i prefer to see new tool for all registration related admin verbs that required and put it there. later on we'll improve it to full registration process utility. where else do you plan to use get-ca-issuer?
Line 13: 
Line 14: Change-Id: I70a19f54169498c53cabba0ac75e7a5611762ece
Line 15: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1039267

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

Itamar Heim has posted comments on this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Patch Set 2:

ping

Change in vdsm[master]: vdsm-tool: add the cmd get-ca-issuer

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has abandoned this change.

Change subject: vdsm-tool: add the cmd get-ca-issuer
......................................................................

Abandoned