Dan Kenigsberg has posted comments on this change.
Change subject: register: Add --ca-file
......................................................................
Patch Set 2: Code-Review-1
(4 comments)
https://gerrit.ovirt.org/#/c/44576/2/lib/vdsm/tool/register.py
File lib/vdsm/tool/register.py:
Line 119: self.logger.debug("VDSM UUID: {uuid_provided}".format(
Line 120: uuid_provided=self.vdsm_uuid))
Line 121:
Line 122: if ca_file is None:
Line 123: self.temp_ca_file = True
why do you need this boolean? having ca_engine=None is enough to tell that we do not need
to store the ca cert.
Line 124: self.ca_engine = None
Line 125: else:
Line 126: self.temp_ca_file = False
Line 127: self.ca_engine = ca_file
Line 207: cert_validation -- SSL cert will be verified
Line 208:
Line 209: Returns: Content of http request
Line 210: """
Line 211: if self.check_fqdn and cert_validation:
is this related to this patch? I think it deserves its own justification
Line 212: cert_validation = self.ca_engine
Line 213: else:
Line 214: cert_validation = False
Line 215:
Line 294: """
Line 295: self.logger.info("Collecting CA data from Engine...")
Line 296:
Line 297: if self.ca_engine is None:
Line 298: ca_dir = "/tmp"
never use /tmp directly. tempdir knows which path should be used for temporary files.
Line 299: else:
Line 300: ca_dir = os.path.dirname(self.ca_engine)
Line 301:
Line 302: if self.ca_engine and os.path.exists(self.ca_engine):
Line 387: """
Line 388: self._execute_http_request(self.url_reg)
Line 389: self.logger.info("Registration completed, host is pending
approval"
Line 390: " on Engine:
{e}".format(e=self.engine_fqdn))
Line 391:
this cleanup does not seem related to execute_registration.
I find this logic very confusing. download_ca should download the ca cert to a temporary
directory.
If the user asks to persist it, a special function would copy the file and persist it.
Immediately after use, the temp dir should be removed.
Line 392: if self.temp_ca_file:
Line 393: os.unlink(self.ca_engine)
Line 394:
Line 395:
--
To view, visit
https://gerrit.ovirt.org/44576
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie24793674569107148c832f0395807586044b95e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes