Juan Hernandez has posted comments on this change. Change subject: restapi: CRUD for OpenStackVolume authentication keys ......................................................................
Patch Set 6: (9 comments) We are adding a "uuid" attribute to the new entity, and then we are using it as the identifier of the resource in the URL. Is this correct? Can we assume that we will never have two secrets with the same "uuid" attribute? https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/OpenstackVolumeAuthenticationKeyUsageType.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/OpenstackVolumeAuthenticationKeyUsageType.java: Line 11: public static OpenstackVolumeAuthenticationKeyUsageType fromValue(String v) { Line 12: try { Line 13: return valueOf(v.toUpperCase()); Line 14: } catch (IllegalArgumentException e) { Line 15: return null; Consider sending an error message to the log describing this problem, something like this: log.error("The value \"{}\" isn't a valid Open Stack volume authentication key ussage", v); log.error("Exception", e); Line 16: } Line 17: } https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeyResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeyResource.java: Line 26: Line 27: @Produces({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) Line 28: public interface OpenStackVolumeAuthenticationKeyResource { Line 29: @GET Line 30: public OpenstackVolumeAuthenticationKey get(); The @DELETE method needs to go here, because of Resteasy 3 requirements: @DELETE Response remove(); Line 31: Line 32: @PUT Line 33: @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) Line 34: public OpenstackVolumeAuthenticationKey update(OpenstackVolumeAuthenticationKey resource); https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeysResource.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/openstack/OpenStackVolumeAuthenticationKeysResource.java: Line 40: public Response add(OpenstackVolumeAuthenticationKey authenticationKey); Line 41: Line 42: @DELETE Line 43: @Path("{id}") Line 44: public Response remove(@PathParam("id") String id); This "remove" method needs to be in the resource interface. This is a new requirement of the version of Resteasy included in WildFly. Line 45: Line 46: @Path("{id}") Line 47: OpenStackVolumeAuthenticationKeyResource getOpenStackVolumeAuthenticationKey(@PathParam("id") String id); https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml File backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml: Line 3917: openstack_volume_authentication_key.value: xs:string Line 3918: openstack_volume_authentication_key.usage_type: xs:string Line 3919: optionalArguments: Line 3920: openstack_volume_authentication_key.description: xs:string Line 3921: description: add a new authentication key to the OpenStack volume provider No need to repeat the description when there is only one signature. Line 3922: - name: /openstackvolumeproviders/{openstackvolumeprovider:id}/authenticationkeys/{authenticationkey:id}|rel=update Line 3923: description: update the specified authentication key Line 3924: request: Line 3925: body: Line 3924: request: Line 3925: body: Line 3926: parameterType: OpenstackVolumeAuthenticationKey Line 3927: signatures: Line 3928: - mandatoryArguments: {} Don't add this empty list: - optionalArguments: ... Line 3929: optionalArguments: Line 3930: openstack_volume_authentication_key.value: xs:string Line 3931: openstack_volume_authentication_key.usage_type: xs:string Line 3932: openstack_volume_authentication_key.description: xs:string Line 3929: optionalArguments: Line 3930: openstack_volume_authentication_key.value: xs:string Line 3931: openstack_volume_authentication_key.usage_type: xs:string Line 3932: openstack_volume_authentication_key.description: xs:string Line 3933: description: update the specified authentication key Same, no need to repeat the description. Line 3934: - name: /openstackvolumeproviders/{openstackvolumeprovider:id}/authenticationkeys|rel=delete Line 3935: description: delete the specified authentication key Line 3936: Line 3937: - name: /openstacknetworkproviders|rel=get https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeAuthenticationKeyResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeAuthenticationKeyResource.java: Line 35: Line 36: protected BackendOpenStackVolumeAuthenticationKeyResource(String providerId, String id) { Line 37: super(id, OpenstackVolumeAuthenticationKey.class, LibvirtSecret.class); Line 38: this.providerId = providerId; Line 39: this.guid = asGuidOr404(id); I think that this assignment to "guid" already happens in the base class? Doesn't it? Line 40: } Line 41: Line 42: @Override Line 43: public OpenstackVolumeAuthenticationKey get() { https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeAuthenticationKeysResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeAuthenticationKeysResource.java: Line 93: OpenStackVolumeAuthenticationKeyResource authenticationKeyResource = getOpenStackVolumeAuthenticationKey(id); Line 94: LibvirtSecret libvirtSecret = map(authenticationKeyResource.get(), null); Line 95: LibvirtSecretParameters parameters = new LibvirtSecretParameters(libvirtSecret); Line 96: return performAction(VdcActionType.RemoveLibvirtSecret, parameters); Line 97: } The "remove" method needs to be moved to the entity resource because of Resteasy 3 issues. Line 98: Line 99: @Override Line 100: @SingleEntityResource Line 101: public OpenStackVolumeAuthenticationKeyResource getOpenStackVolumeAuthenticationKey(String id) { https://gerrit.ovirt.org/#/c/41969/6/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeProvidersResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/openstack/BackendOpenStackVolumeProvidersResource.java: Line 47 Line 48 Line 49 Line 50 Line 51 Are you removing the "certificates" sub-collection on purpose? -- To view, visit https://gerrit.ovirt.org/41969 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1041a9760c29919d86da5479ce29db666d18a924 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
