Michael Pasternak has posted comments on this change.
Change subject: API: Adding ssh username, port and fingerprint to host
information
......................................................................
Patch Set 10: I would prefer that you didn't submit this
(10 inline comments)
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
Line 1282: <xs:element ref="hooks" minOccurs="0"/>
Line 1283: <xs:element name="libvirt_version" type="Version"
minOccurs="0" maxOccurs="1"/>
Line 1284: <!-- Optionally specify the display address of this host
explicitly -->
Line 1285: <xs:element ref="display" minOccurs="0"/>
Line 1286: <xs:element ref="ssh" minOccurs="0" maxOccurs="1"/>
please consider placing it under "root_password" element
Line 1287: </xs:sequence>
Line 1288: </xs:extension>
Line 1289: </xs:complexContent>
Line 1290: </xs:complexType>
Line 1477: <xs:element ref="user" minOccurs="0" maxOccurs="1"/>
Line 1478: </xs:sequence>
Line 1479: </xs:extension>
Line 1480: </xs:complexContent>
Line 1481: </xs:complexType>
please format & drop tab/s
Line 1482:
Line 1483: <xs:element name="group" type="Group"/>
Line 1484:
Line 1485: <xs:element name="groups" type="Groups"/>
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostResource.java
Line 98: public Response install(Action action) {
Line 99: // REVISIT fencing options
Line 100: VDS vds = getEntity();
Line 101: UpdateVdsActionParameters params = new
UpdateVdsActionParameters(vds.getStaticData(), action.getRootPassword(), true);
Line 102: if (action.getSsh().isSetUser()) {
potential NPE, please check action.isSetSsh() before trying to invoke methods
on it
Line 103:
params.setPassword(action.getSsh().getUser().getPassword());
Line 104:
params.getvds().setSshUsername(action.getSsh().getUser().getUserName());
Line 105: }
Line 106: params.getvds().setSshPort(action.getSsh().getPort());
Line 99: // REVISIT fencing options
Line 100: VDS vds = getEntity();
Line 101: UpdateVdsActionParameters params = new
UpdateVdsActionParameters(vds.getStaticData(), action.getRootPassword(), true);
Line 102: if (action.getSsh().isSetUser()) {
Line 103:
params.setPassword(action.getSsh().getUser().getPassword());
by doing this, you may override root_password with NULL
Line 104:
params.getvds().setSshUsername(action.getSsh().getUser().getUserName());
Line 105: }
Line 106: params.getvds().setSshPort(action.getSsh().getPort());
Line 107:
params.getvds().setSshKeyFingerprint(action.getSsh().getFingerprint());
Line 100: VDS vds = getEntity();
Line 101: UpdateVdsActionParameters params = new
UpdateVdsActionParameters(vds.getStaticData(), action.getRootPassword(), true);
Line 102: if (action.getSsh().isSetUser()) {
Line 103:
params.setPassword(action.getSsh().getUser().getPassword());
Line 104:
params.getvds().setSshUsername(action.getSsh().getUser().getUserName());
we does not support this yet
Line 105: }
Line 106: params.getvds().setSshPort(action.getSsh().getPort());
Line 107:
params.getvds().setSshKeyFingerprint(action.getSsh().getFingerprint());
Line 108:
Line 102: if (action.getSsh().isSetUser()) {
Line 103:
params.setPassword(action.getSsh().getUser().getPassword());
Line 104:
params.getvds().setSshUsername(action.getSsh().getUser().getUserName());
Line 105: }
Line 106: params.getvds().setSshPort(action.getSsh().getPort());
you may hit exception here (cause action.getSsh().getPort() may return NULL) if
params.getvds().setSshPort() expects for int and not Integer class
Line 107:
params.getvds().setSshKeyFingerprint(action.getSsh().getFingerprint());
Line 108:
Line 109: if (vds.getVdsType()==VDSType.oVirtNode) {
Line 110: params.setIsReinstallOrUpgrade(true);
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostsResource.java
Line 100:
addParams.setPassword(host.getSsh().getUser().getPassword());
Line 101:
addParams.getvds().setSshUsername(host.getSsh().getUser().getUserName());
Line 102: }
Line 103: addParams.getvds().setSshPort(host.getSsh().getPort());
Line 104:
addParams.getvds().setSshKeyFingerprint(host.getSsh().getFingerprint());
(all) same comments as in BackendHostResource
Line 105:
Line 106: return performCreate(VdcActionType.AddVds,
Line 107: addParams,
Line 108: new
QueryIdResolver<Guid>(VdcQueryType.GetVdsByVdsId, IdQueryParameters.class));
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java
Line 51: private static final int DEFAULT_VDSM_PORT = 54321;
Line 52: private static final String MD5_FILE_SIGNATURE = "md5";
Line 53: private static final String MD5_SECURITY_ALGORITHM = "MD5";
Line 54: private static final String DEFAULT_ROOT_USERNAME = "root";
Line 55: private static final int DEFAULT_SSH_PORT = 22;
i think defaults that you've mentioned here, should reside on a server side and
should be set there only if they !=null
(otherwise you should redefine them in all clients what is error prone)
Line 56:
Line 57: private static final String HOST_OS_DELEIMITER = " - ";
Line 58:
Line 59: @Mapping(from = Host.class, to = VdsStatic.class)
Line 79: if (model.isSetSsh() && model.getSsh().isSetUser() &&
model.getSsh().getUser().isSetUserName()) {
Line 80:
entity.setSshUsername(model.getSsh().getUser().getUserName());
Line 81: } else {
Line 82: entity.setSshUsername(DEFAULT_ROOT_USERNAME);
Line 83: }
not supported yet
Line 84: if (model.isSetSsh() && model.getSsh().isSetPort() &&
model.getSsh().getPort() > 0) {
Line 85: entity.setSshPort(model.getSsh().getPort());
Line 86: } else {
Line 87: entity.setSshPort(DEFAULT_SSH_PORT);
....................................................
Commit Message
Line 9: Those additional fields are used to authenticate with the host during
Line 10: deploy process. Currently we use default port (22) and root user to
Line 11: connect to host. This patch adds the capability to use configurable
Line 12: port and specific user
Line 13:
Yaniv, well done!, two more things to do:
1. please update rsdl_metadata.yaml accordingly
2. please add new mappings in the HostMapperTest.java
thanks
Line 14: values, and avoiding using root password to install or update new host.
Line 15: Change-Id: I6b8c117d041c29780af2468888a732ee664d283c
--
To view, visit http://gerrit.ovirt.org/16247
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b8c117d041c29780af2468888a732ee664d283c
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
