Oved Ourfali has posted comments on this change.

Change subject: engine,webdmin: Restrict destination host parameter for 
administrator roles
......................................................................


Patch Set 6: Looks good to me, but someone else must approve

(3 inline comments)

Overall it is okay.
Some minor comments.

....................................................
File backend/manager/dbscripts/upgrade/03_02_0510_add_edit_admin_vm_props.sql
Line 18:     id,
Line 19:     205 -- EDIT_ADMIN_TEMPLATE_PROPERTIES
Line 20: from roles
Line 21: where
Line 22:     name in ('SuperUser', 'ClusterAdmin', 'DataCenterAdmin', 
'TemplateAdmin')
Unless I'm missing something, it seems that ClusterAdmin shouldn't get this 
permission. Let's see if someone else in the reviewers has something to say 
about that.
Line 23:     and not exists (
Line 24:        select 1 from roles_groups rg
Line 25:        where
Line 26:            rg.role_id in (select id from roles where name in 
('SuperUser', 'ClusterAdmin', 'DataCenterAdmin', 'TemplateAdmin')


....................................................
File 
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Constants.java
Line 1126: 
Line 1127:     @DefaultStringValue("Allow changing VM administrative 
properties")
Line 1128:     String allowChangingVmAdminPropertiesRoleTreeTooltip();
Line 1129: 
Line 1130:     @DefaultStringValue("Allow changing template administrative 
properties")
s/template/Template (capitalize "template").
Line 1131:     String allowChangingTemplateAdminPropertiesRoleTreeTooltip();
Line 1132: 
Line 1133:     @DefaultStringValue("Allow to override the currently opened 
remote console session")
Line 1134:     String allowReconnectToVmRoleTreeTooltip();


....................................................
File 
frontend/webadmin/modules/uicompat/src/main/resources/org/ovirt/engine/ui/uicompat/LocalizedEnums.properties
Line 22: ActionGroup___MANIPULATE_STORAGE_DOMAIN=Manipulate Status
Line 23: ActionGroup___CREATE_TEMPLATE=Create
Line 24: ActionGroup___DELETE_TEMPLATE=Delete
Line 25: ActionGroup___EDIT_TEMPLATE_PROPERTIES=Edit Properties
Line 26: ActionGroup___EDIT_ADMIN_TEMPLATE_PROPERTIES=Edit administrative 
properties
Is that supposed to contain "template" as well in the value? What is this 
property used for?
Line 27: ActionGroup___CONFIGURE_TEMPLATE_NETWORK=Assign Network to Template
Line 28: ActionGroup___COPY_TEMPLATE=Copy
Line 29: ActionGroup___CREATE_CLUSTER=Create
Line 30: ActionGroup___DELETE_CLUSTER=Delete


--
To view, visit http://gerrit.ovirt.org/11303
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5294854d24b235f2c50fa7f3d4e7472cf7598b53
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Libor Spevak <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Libor Spevak <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to