Juan Hernandez has posted comments on this change.

Change subject: restapi: Allow enabling/disabling SSO
......................................................................


Patch Set 3:

(13 comments)

....................................................
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
Line 621:           <xs:element ref="reported_device_types" minOccurs="0"/>
Line 622:           <xs:element ref="ip_versions" minOccurs="0"/>
Line 623:           <xs:element ref="snapshot_statuses" minOccurs="0"/>
Line 624:           <xs:element ref="content_types" minOccurs="0" />
Line 625:           <xs:element ref="hook_states" minOccurs="0" />
Add here the reference to the <sso_methods> elements (for the capabilities 
resource):

  <xs:element ref="disk_formats" minOccurs="0"/>
Line 626:           <xs:element ref="stages" minOccurs="0" />
Line 627:         </xs:sequence>
Line 628:       </xs:extension>
Line 629:     </xs:complexContent>


Line 1020:           </xs:appinfo>
Line 1021:         </xs:annotation>
Line 1022:       </xs:element>
Line 1023:     </xs:sequence>
Line 1024:   </xs:complexType>
Define here the SSO method types to be used by the capabilities resource:

  <xs:element name="sso_methods" type="SsoMethods"/>

  <xs:complexType name="SsoMethods">
    <xs:sequence>
      <xs:element name="sso_method" type="xs:string" minOccurs="0" 
maxOccurs="unbounded">
        <xs:annotation>
          <xs:appinfo>
            <jaxb:property name="SsoMethods"/>
          </xs:appinfo>
        </xs:annotation>
      </xs:element>
    </xs:sequence>
  </xs:complexType>
Line 1025: 
Line 1026:   <!-- Common to all resources -->
Line 1027: 
Line 1028:   <xs:complexType name="ActionableResource">


Line 2298:     </xs:sequence>
Line 2299:     <xs:attribute name="type" type="xs:string"/>
Line 2300:   </xs:complexType>
Line 2301: 
Line 2302:   <!-- Single Sign-On Section -->
For each type added you need to add a new element definition. This isn't 
required by the schema definition, but it is required by the generator of the 
Python SDK. You need to put something like this here:

  <xs:element name="sso" type="Sso"/>
Line 2303:   <xs:complexType name="Sso">
Line 2304:     <xs:sequence>
Line 2305:       <xs:element name="methods" type="SsoMethods" minOccurs="1" 
maxOccurs="1" />
Line 2306:     </xs:sequence>


Line 2304:     <xs:sequence>
Line 2305:       <xs:element name="methods" type="SsoMethods" minOccurs="1" 
maxOccurs="1" />
Line 2306:     </xs:sequence>
Line 2307:   </xs:complexType>
Line 2308: 
I know I asked you before to rename the type from "Methods" to "SsoMethods", 
but I was wrong: we tend to reuse these elements, even if they are for 
different purposes. In addition the "SsoMethods" name should be reserved for he 
capabilities resource. So, all in all, please rename this back to "Methods" and 
add the element definition:

  <xs:element name="methods" type="Methods"/>

  <xs:complexType name="Methods">
    ...
Line 2309:   <xs:complexType name="SsoMethods">
Line 2310:     <xs:sequence>
Line 2311:       <xs:element name="method" type="SsoMethod" minOccurs="0" 
maxOccurs="unbounded" />
Line 2312:     </xs:sequence>


Line 2307:   </xs:complexType>
Line 2308: 
Line 2309:   <xs:complexType name="SsoMethods">
Line 2310:     <xs:sequence>
Line 2311:       <xs:element name="method" type="SsoMethod" minOccurs="0" 
maxOccurs="unbounded" />
We don't support XSD enums, we must use only strings. Instead of using 
xs:enumeration here use xs:string. Then define the SsoMethod enum manually (in 
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model),
 something like this:

  public enum SsoMethod {
    GUEST_AGENT;

    public String value() {
      return name().toLowerCase();
    }

    public static SsoMethod fromValue(String value) {
      try {
        return valueOf(value.toUpperCase());
      } catch (IllegalArgumentException e) {
        return null;
      }
    }
  }

Then add it to BackendCapabiltiesResource. This should be added only for 
version >= 3.4.
Line 2312:     </xs:sequence>
Line 2313:   </xs:complexType>
Line 2314: 
Line 2315:   <xs:simpleType name="SsoMethod">


Line 2315:   <xs:simpleType name="SsoMethod">
Line 2316:     <xs:restriction base="xs:string">
Line 2317:       <xs:enumeration value="GUEST_AGENT"/>
Line 2318:     </xs:restriction>
Line 2319:   </xs:simpleType>
Remove this type, as you should use xsd:string instead of enum.
Line 2320: 
Line 2321:   <xs:complexType name="HighAvailability">
Line 2322:     <xs:sequence>
Line 2323:       <xs:element name="enabled" type="xs:boolean" minOccurs="0" 
maxOccurs="1"/>


Line 2526:           <xs:element name="creation_time" type="xs:dateTime" 
minOccurs="0"/>
Line 2527:           <xs:element name="origin" type="xs:string" minOccurs="0"/>
Line 2528:           <xs:element name="stateless" type="xs:boolean" 
minOccurs="0"/>
Line 2529:           <xs:element name="delete_protected" type="xs:boolean" 
minOccurs="0"/>
Line 2530:           <xs:element name="sso" type="Sso" minOccurs="0" 
maxOccurs="1"/>
Instead of this:

  name="sso" type="Sso"

Use this:

  ref="sso"
Line 2531:           <xs:element ref="console" minOccurs="0" maxOccurs="1"/>
Line 2532:           <xs:element name="timezone" type="xs:string" minOccurs="0" 
maxOccurs="1"/>
Line 2533:           <xs:element ref="domain" minOccurs="0" maxOccurs="1"/>
Line 2534:           <xs:element name="custom_properties" 
type="CustomProperties" minOccurs="0"/>


....................................................
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 145:           vm.comment: xs:string
Line 146:           vm.stateless: xs:boolean
Line 147:           vm.permissions.clone: xs:boolean
Line 148:           vm.delete_protected: xs:boolean
Line 149:           vm.sso.methods.method: --COLLECTION: {method: 'xs:string'}
Same correction needed here.
Line 150:           vm.console.enabled: xs:boolean
Line 151:           vm.cpu.mode: xs:string
Line 152:           vm.cpu.topology.sockets: xs:int
Line 153:           vm.cpu_shares: xs:int


Line 194:           vm.description: xs:string
Line 195:           vm.comment: xs:string
Line 196:           vm.stateless: xs:boolean
Line 197:           vm.delete_protected: xs:boolean
Line 198:           vm.sso.methods.method: --COLLECTION: {method: 'xs:string'}
Same correction needed here.
Line 199:           vm.console.enabled: xs:boolean
Line 200:           vm.cpu.topology.sockets: xs:int
Line 201:           vm.placement_policy.affinity: xs:string
Line 202:           vm.placement_policy.host.id|name: xs:string


Line 236:                 vm.comment: xs:string
Line 237:                 vm.stateless: xs:boolean
Line 238:                 vm.permissions.clone: xs:boolean
Line 239:                 vm.delete_protected: xs:boolean
Line 240:                 vm.sso.methods.method: --COLLECTION: {method: 
'xs:string'}
Same correction needed here.
Line 241:                 vm.cpu.mode: xs:string
Line 242:                 vm.cpu.topology.sockets: xs:int
Line 243:                 vm.placement_policy.affinity: xs:string
Line 244:                 vm.placement_policy.host.id|name: xs:string


Line 3155:           template.domain.name: xs:string
Line 3156:           template.type: xs:string
Line 3157:           template.stateless: 'xs:boolean'
Line 3158:           template.delete_protected: xs:boolean
Line 3159:           template.sso.methods.method: --COLLECTION: {method: 
'xs:string'}
Same correction needed here.
Line 3160:           template.console.enabled: xs:boolean
Line 3161:           template.placement_policy.affinity: xs:string
Line 3162:           template.description: xs:string
Line 3163:           template.comment: xs:string


Line 3202:           template.domain.name: xs:string
Line 3203:           template.type: xs:string
Line 3204:           template.stateless: 'xs:boolean'
Line 3205:           template.delete_protected: xs:boolean
Line 3206:           template.sso.methods.method: --COLLECTION: {method: 
'xs:string'}
Same correction needed here.
Line 3207:           template.console.enabled: xs:boolean
Line 3208:           template.placement_policy.affinity: xs:string
Line 3209:           template.description: xs:string
Line 3210:           template.comment: xs:string


....................................................
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/SsoMapper.java
Line 21:         return model;
Line 22:     }
Line 23: 
Line 24:     @Mapping(from = Sso.class, to = SsoMethod.class)
Line 25:     public static SsoMethod map(Sso model, SsoMethod template) {
And this logic to produde GUEST_AGENT should be in the backend, not in the API. 
This method should return null if no <sso> element is present, and then the 
backend should take that and set correct default method.
Line 26:         if (model != null && model.getMethods() != null && 
model.getMethods().getMethod() != null && model.getMethods().getMethod().size() 
== 1) {
Line 27:             if (model.getMethods().getMethod().get(0) == 
org.ovirt.engine.api.model.SsoMethod.GUEST_AGENT) {
Line 28:                 return SsoMethod.GUEST_AGENT;
Line 29:             }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I25bf94646ea89684152e48f25ad604db6e59e86c
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to