Bug 1003281 - [RHEVM-ENGINE] SSH port can be updated from REST API while it is locked in the UI
Summary: [RHEVM-ENGINE] SSH port can be updated from REST API while it is locked in th...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine-restapi
Version: 3.3.0
Hardware: Unspecified
OS: Unspecified
unspecified
high
Target Milestone: ---
: 3.3.0
Assignee: Yaniv Bronhaim
QA Contact: Elena
URL:
Whiteboard: infra
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-09-01 15:34 UTC by Barak Dagan
Modified: 2016-02-10 19:01 UTC (History)
14 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-09-10 12:07:13 UTC
oVirt Team: Infra
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
engine + cli logs (9.24 KB, application/x-compressed-tar)
2013-09-01 15:34 UTC, Barak Dagan
no flags Details


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 18754 0 None None None Never

Description Barak Dagan 2013-09-01 15:34:26 UTC
Created attachment 792611 [details]
engine + cli logs

Description of problem:
After an host is added, It's ssh port can't be changed from the UI - but it can be changed using REST API (CLI)

Version-Release number of selected component (if applicable):
is12

How reproducible:
100%

Steps to Reproduce:
1.
2.
3.

Actual results:


Expected results:
Expecting an error from REST, that ssh port is immutable.

Additional info:

Comment 1 Barak Dagan 2013-09-01 15:37:51 UTC
Reproduce:

# action host MyHost deactivate 
# update host MyHost --ssh-port 55

1) CLI (REST) response.
2) check in DB: 
   psql -U postgres -d engine -c 'select vds_name, ssh_port from vds'| less -s

Comment 2 Yaniv Bronhaim 2013-09-02 09:51:37 UTC
In host update action only ssh.fingerprint field can me modified.

Comment 3 Michael Pasternak 2013-09-08 14:07:33 UTC
(In reply to Barak Dagan from comment #0)
> Created attachment 792611 [details]
> engine + cli logs
> 
> Description of problem:
> After an host is added, It's ssh port can't be changed from the UI - but it
> can be changed using REST API (CLI)
> 

so why this is a bug? api/sdk/cli are wider than UI by definition, we should
block it only if it does not work for our business logic, but than why exposing
it to begin with?

Comment 4 Barak Dagan 2013-09-08 14:35:26 UTC
Why to expose it in update if its supposed to be blocked ? good question. It shouldn't be exposed in that action IMO.

Comment 5 Alon Bar-Lev 2013-09-08 14:41:14 UTC
API can be much more permissive than UI.

There is nothing wrong in UI be more restricted, to guard user, while other interfaces such as API and protocol to relay on the programmer/administrator to know what they are doing.

So I am unsure what the problem is.

Comment 6 Yaniv Bronhaim 2013-09-08 14:53:52 UTC
It doesn't matter or related to the UI. In edit operation the user cannot modify the port nor the IP address of the host. This is how it should work. If the user wants to change the port, the user can remove the host and adding it again. After first authentication, all the authentication parameters cannot be modified expect the fingerprint. The way it is currently is wrong and should be fixed.

For another field with the same spirit, currently the UI blocks modification of the ssh username field in add host. the patch http://gerrit.ovirt.org/#/c/18756/ blocks it also via the rest api. If its common to allow wider api, I don't mind leaving this field for update during add new host operation and abandon the patch.

Comment 7 Michael Pasternak 2013-09-08 15:10:30 UTC
(In reply to Yaniv Bronhaim from comment #6)
> It doesn't matter or related to the UI. In edit operation the user cannot
> modify the port nor the IP address of the host. This is how it should work.
> If the user wants to change the port, the user can remove the host and
> adding it again. After first authentication, all the authentication
> parameters cannot be modified expect the fingerprint. The way it is
> currently is wrong and should be fixed.
> 
> For another field with the same spirit, currently the UI blocks modification
> of the ssh username field in add host. the patch
> http://gerrit.ovirt.org/#/c/18756/ blocks it also via the rest api. If its
> common to allow wider api, I don't mind leaving this field for update during
> add new host operation and abandon the patch.

Yaniv,

this fix is wrong, rsdl_metadata is a metadata..., metadata != restriction,
i.e if it won't be exposed via rsdl, one still can change it via api/sdk/cli,
if you want to block it, it should be done in one place, command.can_do_action,

though i'm not sure you should disallow changing these props via update,
simply because (as you've mentioned by yourself) it means reinstalling the host

just think of situation where sysadmin changing host internals,
will have to have add-host permissions to do the maintenance ...

Comment 8 Alon Bar-Lev 2013-09-08 15:15:17 UTC
I disagree.

Modifying ip, port, user and any other attribute should be opened via sdk/cli/api to allow maintaining hosts in non standard situations, examples: modify the port ssh is listening, modify the address of a host, modify the user.

Blocking these making sense in UI, but not for maintenance tools.

Comment 9 Michael Pasternak 2013-09-08 15:22:49 UTC
(In reply to Alon Bar-Lev from comment #8)
> I disagree.
> 
> Modifying ip, port, user and any other attribute should be opened via
> sdk/cli/api to allow maintaining hosts in non standard situations, examples:
> modify the port ssh is listening, modify the address of a host, modify the
> user.
> 
> Blocking these making sense in UI, but not for maintenance tools.

this is my 50 cents as well, (though one can find changing these via UI useful,
but this is your call)

Comment 10 Alon Bar-Lev 2013-09-09 08:58:08 UTC
> Andrew Cathrow 2013-09-09 04:49:30 EDT
> Flags: pm_ack? → pm_ack+

Hi Yaniv,

Can you please explain what we are trying to fix?

> Updating ssh fields in adding new host:
> * Setting password as optional parameters (when using pk the field can be
>   omitted).

This I understand, and I thought that we already have that.

> * Removing port, ip and authentications fields from updating operation, as
>   updating always uses pk authentication.

This I do not understand. Why do you think that update always uses specific method?

Comment 11 Yaniv Bronhaim 2013-09-10 12:07:13 UTC
After few confusions about the differences between API and UI fields, I understand that API should include all updateable fields and it should not necessary reflect the UI. 

From the API\sdk cli we should be able to update all host's parameters.

closing as NOTABUG,

The fix of allowing updating all host's field is part of Bug #999640


Note You need to log in before you can comment on or make changes to this bug.