Nullable image description crashes v2 client

[Kamil Rykowski]

When you somehow set the image description to None the glanceclient v2 image-list crashes (as well as image-show, image-update for this particular image). The only way to show all images now is to use client v1, because it's more stable in this case.

Steps to reproduce:

1. Open Horizon and go to the edit page of any image.
2. Set description to anything eg. "123" and save.
3. Open image edit page again, remove description and save it.
4. List all images using glanceclient v2: "glance --os-image-api-version 2 image-list"
5. Be sad, because of raised exception:

None is not of type u'string'

Failed validating u'type' in schema[u'additionalProperties']:
    {u'type': u'string'}

On instance[u'description']:
    None

During investigating the issue I've found that the additionalProperties schema is set to accept only string values, so it should be expanded to allow for null values as well.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/165066

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/165360

[Brian Rosmaita (brian-rosmaita)]

I'm not convinced it makes sense to have null additionalProperties. This bug should not be fixed by modifying the schema, it should be fixed so that the client doesn't allow this behavior.

[Kamil Rykowski (kamil-rykowski)]

Agree with you Brian. When I was thinking about this one it wasn't clear for me why anyone would like to store a null as the value for the image property. Even the idea is incomprehensible I didn't want to push changes which will break backward compatibility (Horizon sets description to null as I've mentioned in bug description).

I've prepared some straightforward patches which won't break anything, but well ... I'm not totally comfortable with it and I'm open to change the direction to fix it. Here are three approaches we can go through:

- add the DEFAULT value at the database level (image_property table) to empty string
- raising exception at the Glance API when someone tries to set property to null (will probably need to make some conversion at the client level null->"")
- changing value of the property to empty string when someone tries to set it to null at the Glance API level

Any thoughts?

[Sabari Murugesan (smurugesan)]

glance v1 api allows image properties to be set as null and v2 defines a schema that doesn't allow null properties. It is so easy to run into this problem when booting instances with a hypervisor driver that leverages image locations from v2 but the image has null properties set through v1 api.

Adding to Kamil's, we can choose not to send null valued properties in response via v2 api.

[Tom Verdaat (tom-verdaat)]

Running into this bug in a very silly way: Added images using horizon (which didn't require adding a description) and then couldn't launch instances with those images after upgrading to Kilo because nova compute depends on glanceclient and it returned this error.

Full trace: https://bugs.launchpad.net/nova/+bug/1457339

[Clayton O'Neill (clayton-oneill)]

When ran into this while doing test upgrades to Kilo. After doing the upgrade, we were no longer able to boot some images, and turned out to be because we had several images with NULL description fields. I was able to validate that the glance v1 api still allows creating images of this type with the following command:

    glance image-create --name cirros_test --disk-format qcow2 --container-format bare --file cirros-0.3.4-x86_64-disk.img --is-public True --is-protected True --progress --property description=

At this point we're trying to figure out how to work around this issue. Right now it's possible for clients to use the v1 api to create images they cannot boot, and the failure mode from nova just looks like it cannot find a node to schedule on. That's pretty opaque to end users.

[David Medberry (med)]

This is basically a showstopper for Kilo upgrades.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/189761

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/189661
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=ccea7b68567afb2d5bb825d0cabf247a9526bd93
Submitter: Jenkins
Branch: master

commit ccea7b68567afb2d5bb825d0cabf247a9526bd93
Author: Flavio Percoco <email address hidden>
Date: Tue Jun 9 13:11:36 2015 +0200

    Return empty str for permissive, none, properties

    In Kilo, a patch landed to allow glance for returning None values. While
    this was not an issue for the base and registered properties in the
    `schema-image.json` file, it did break backwards compatibility for the
    custom images created in V1 that have a vulue equal to None.

    In order to restore compatibility, this changes returns empty strings
    for those custom properties that exist in the database but that are not
    part of the V2 schema. This strategy will help migrating such properties
    when updates to the image data happens and it'll be a noop for just
    reads.

    We can't use a schema migration because we don't know off hand which of
    the properties in the database are the ones that would need to be
    migrated.

    While we could skip this properties entirely, this patch prefers to send
    the empty string back as a way to create awareness on the fact that
    there's an empty property in the database. Since we didn't use to return
    these properties, we can assume they weren't being used.

    Closes-bug: #1419823
    Change-Id: I59bb27a892fe9485fc98a612ca0148a84123f5a2

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/189761
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=870e2d57bd331c9e630e11ad27b9d331c333d96f
Submitter: Jenkins
Branch: stable/kilo

commit 870e2d57bd331c9e630e11ad27b9d331c333d96f
Author: Flavio Percoco <email address hidden>
Date: Tue Jun 9 13:11:36 2015 +0200

    Return empty str for permissive, none, properties

    In Kilo, a patch landed to allow glance for returning None values. While
    this was not an issue for the base and registered properties in the
    `schema-image.json` file, it did break backwards compatibility for the
    custom images created in V1 that have a vulue equal to None.

    In order to restore compatibility, this changes returns empty strings
    for those custom properties that exist in the database but that are not
    part of the V2 schema. This strategy will help migrating such properties
    when updates to the image data happens and it'll be a noop for just
    reads.

    We can't use a schema migration because we don't know off hand which of
    the properties in the database are the ones that would need to be
    migrated.

    While we could skip this properties entirely, this patch prefers to send
    the empty string back as a way to create awareness on the fact that
    there's an empty property in the database. Since we didn't use to return
    these properties, we can assume they weren't being used.

    Closes-bug: #1419823
    Change-Id: I59bb27a892fe9485fc98a612ca0148a84123f5a2
    (cherry picked from commit ccea7b68567afb2d5bb825d0cabf247a9526bd93)

[OpenStack Infra (hudson-openstack)]

Change abandoned by Kamil Rykowski (<email address hidden>) on branch: master
Review: https://review.openstack.org/165360

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Yang Ye (ccyangye)]

I have problem when updating a properties from null value in glance v2 API.
If op is add(default client rest call), an ERROR returned
<html>
 <head>
  <title>409 Conflict</title>
 </head>
 <body>
  <h1>409 Conflict</h1>
  There was a conflict when trying to complete your request.<br /><br />
Property a already present.

 </body>
</html> (HTTP 409)
if op is replace, nothing happend

[Yang Ye (ccyangye)]

Because in function do_add of ImageController, "path_root in image.extra_properties" is true and raise an ERROR.
And in function __setitem__ of ExtraPropertiesProxy, "self.__getitem__(key) is not None" is false and then do nothing.

BTW, I perfer to change this perperties both in add and replace op because none is as not set.