Should check for empty string as key when setting key-value pair in property

[QiangTang]

root@controller01:~# glance image-update ubuntu-14.04-server-amd64 --property =
+-------------------------------+--------------------------------------+
| Property | Value |
+-------------------------------+--------------------------------------+
| Property '' | None |
| Property 'vmware_adaptertype' | lsiLogic |
| Property 'vmware_disktype' | streamOptimized |
| checksum | 7b26c526f6f7c8ab874ad55b2698dee0 |
| container_format | bare |
| created_at | 2016-07-21T07:37:42.000000 |
| deleted | False |
| deleted_at | None |
| disk_format | vmdk |
| id | 2fb0b8a1-1634-499d-95ee-2451ec23893e |
| is_public | True |
| min_disk | 5 |
| min_ram | 512 |
| name | ubuntu-14.04-server-amd64 |
| owner | 5b53ee384ebc4212b4086241e1075dea |
| protected | False |
| size | 971606528 |
| status | active |
| updated_at | 2016-07-22T02:25:46.000000 |
| virtual_size | None |
+-------------------------------+--------------------------------------+

The null key set success as the result.
But null key is meaningless to the user which also belong to the invalid input in other components, like cinder and nova flavor as below:

root@controller01:~# nova flavor-key m1.tiny set =
ERROR (CommandError): Invalid key: "". Keys may only contain letters, numbers, spaces, underscores, periods, colons and hyphens.

So for the consistent openstack UE and error handling, glance cli site better to add the null key check.

[Jaspreet Singh Rawel (jaspreetsinghrawel)]

root@controller:/home/ubuntu# glance image-show a3b9e5d2-f747-4432-9155-241662d6766a
+------------------+--------------------------------------+
| Property | Value |
+------------------+--------------------------------------+
| checksum | ee1eca47dc88f4879d8a229cc70a07c6 |
| container_format | bare |
| created_at | 2016-11-09T17:58:03Z |
| disk_format | qcow2 |
| id | a3b9e5d2-f747-4432-9155-241662d6766a |
| min_disk | 0 |
| min_ram | 0 |
| name | cirros\xb2 |
| owner | abe83ae828434fa9917bcee3d80a2fc7 |
| protected | False |
| size | 13287936 |
| status | active |
| tags | [] |
| updated_at | 2016-11-09T17:58:03Z |
| virtual_size | None |
| visibility | public |
+------------------+--------------------------------------+
root@controller:/home/ubuntu#
root@controller:/home/ubuntu#
root@controller:/home/ubuntu# glance image-update a3b9e5d2-f747-4432-9155-241662d6766a --property =
+------------------+--------------------------------------+
| Property | Value |
+------------------+--------------------------------------+
| | |
| checksum | ee1eca47dc88f4879d8a229cc70a07c6 |
| container_format | bare |
| created_at | 2016-11-09T17:58:03Z |
| disk_format | qcow2 |
| id | a3b9e5d2-f747-4432-9155-241662d6766a |
| min_disk | 0 |
| min_ram | 0 |
| name | cirros\xb2 |
| owner | abe83ae828434fa9917bcee3d80a2fc7 |
| protected | False |
| size | 13287936 |
| status | active |
| tags | [] |
| updated_at | 2016-11-09T19:03:41Z |
| virtual_size | None |
| visibility | public |
+------------------+--------------------------------------+
root@controller:/home/ubuntu#
root@controller:/home/ubuntu# glance --version
2.0.0
root@controller:/home/ubuntu#

[OpenStack Infra (hudson-openstack)]

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

[Ian Cordasco (icordasc)]

This is absolutely intended behaviour. Many operators have the workflow:

$ glance image-create
$ # gather data from somewhere
$ glance image-update <UUID> --name "New Name" ...

[Jaspreet Singh Rawel (jaspreetsinghrawel)]

So is the below sequence expected behaviour????????

root@controller:/home/ubuntu# glance image-show 367fc434-1eee-4b01-8dba-580452ca7dff
+------------------+--------------------------------------+
| Property | Value |
+------------------+--------------------------------------+
| checksum | ee1eca47dc88f4879d8a229cc70a07c6 |
| container_format | bare |
| created_at | 2016-11-09T18:02:08Z |
| disk_format | qcow2 |
| id | 367fc434-1eee-4b01-8dba-580452ca7dff |
| min_disk | 0 |
| min_ram | 0 |
| name | ОБРАЗ |
| owner | abe83ae828434fa9917bcee3d80a2fc7 |
| protected | False |
| size | 13287936 |
| status | active |
| tags | [] |
| updated_at | 2016-11-11T17:03:55Z |
| virtual_size | None |
| visibility | public |
+------------------+--------------------------------------+
root@controller:/home/ubuntu#
root@controller:/home/ubuntu#
root@controller:/home/ubuntu# glance image-update 367fc434-1eee-4b01-8dba-580452ca7dff --property =
+------------------+--------------------------------------+
| Property | Value |
+------------------+--------------------------------------+
| | |
| checksum | ee1eca47dc88f4879d8a229cc70a07c6 |
| container_format | bare |
| created_at | 2016-11-09T18:02:08Z |
| disk_format | qcow2 |
| id | 367fc434-1eee-4b01-8dba-580452ca7dff |
| min_disk | 0 |
| min_ram | 0 |
| name | ОБРАЗ |
| owner | abe83ae828434fa9917bcee3d80a2fc7 |
| protected | False |
| size | 13287936 |
| status | active |
| tags | [] |
| updated_at | 2016-11-11T17:04:14Z |
| virtual_size | None |
| visibility | public |
+------------------+--------------------------------------+
root@controller:/home/ubuntu# glance image-update 367fc434-1eee-4b01-8dba-580452ca7dff --remove-property ''
+------------------+--------------------------------------+
| Property | Value |
+------------------+--------------------------------------+
| checksum | ee1eca47dc88f4879d8a229cc70a07c6 |
| container_format | bare |
| created_at | 2016-11-09...

[Jaspreet Singh Rawel (jaspreetsinghrawel)]

"glance image-update 367fc434-1eee-4b01-8dba-580452ca7dff --property =" indicated in the above comment added an image property having name as null. The name of this property can never be changed later as per my understanding. The only way is then to use command "glance image-update 367fc434-1eee-4b01-8dba-580452ca7dff --remove-property ''" for removing this property with null name. So please confirm if this acceptable behaviour.

[Hemanth Makkapati (hemanth-makkapati)]

Jaspreet,
You can certainly change the name of an image. Please look at Ian's comment above for help.
And, no, you can't remove the property "name" from any image whether or not it is null. "name" is one of the core/base image properties that are always present. A user can only add/remove image extra properties.

[Jaspreet Singh Rawel (jaspreetsinghrawel)]

Ian, Hemanth, I am not removing the property "name" from any image. I think the description was misunderstood. Sorry for the inconvenience.

User is currently allowed to add extra property with "" (null) as key and some value e.g. "abcd". After adding this extra property using image-create or image-update function, the image properties will have another property with null key and "abcd" value. Please see row one in the command output below.

root@controller:/home/ubuntu# glance image-update 367fc434-1eee-4b01-8dba-580452ca7dff --property =abcd
+------------------+--------------------------------------+
| Property | Value |
+------------------+--------------------------------------+
| | abcd |
| checksum | ee1eca47dc88f4879d8a229cc70a07c6 |
| container_format | bare |
| created_at | 2016-11-09T18:02:08Z |
| disk_format | qcow2 |
| id | 367fc434-1eee-4b01-8dba-580452ca7dff |
| min_disk | 0 |
| min_ram | 0 |
| name | ОБРАЗ |
| owner | abe83ae828434fa9917bcee3d80a2fc7 |
| protected | False |
| size | 13287936 |
| status | active |
| tags | [] |
| updated_at | 2016-11-11T18:48:07Z |
| virtual_size | None |
| visibility | public |
+------------------+--------------------------------------+

The change I did is to restrict the user from using key as "" (null) during addition of extra property.

[Brian Rosmaita (brian-rosmaita)]

So the issue here is that the glanceclient lets you use an empty string as the key for a user-specified image property.

Such properties are optional in the API, but they are enabled by default [0], so we should probably assume that most deployments allow them.

The value of an additionalProperty must be a string type; we don't have any restriction on length (neither maxLength or minLength) [1]. But, what we're talking about here is the *key*, and JSON schema doesn't give you a way to put restrictions on those.

The JSON standard says, basically, that an object contains members that consist of a pair (or of a pair, members sequence) and a pair is string : value. The standard says: "A string is a sequence of zero or more Unicode characters, wrapped in double quotes, using backslash escapes" [2]. Since the v2 API uses JSON as its serialization format, I think we're stuck with allowing an empty string as the name of a user-specifiable property.

The glanceclient allows you to create such an image property, but it also allows you to delete it. So I don't think this is really a bug. It's a little weird, but not a bug.

You can also create, update, and delete a user-specified property with an empty string as the name using the API directly. Since our goal is to keep the API and glanceclient in sync as much as possible, that's another reason to invalidate this bug.

[0] https://github.com/openstack/glance/blob/d64fe8bd1bbd4be73201d4f652e16e9d47ff39d1/glance/common/config.py#L174-L196

[1] https://github.com/openstack/glance/blob/d64fe8bd1bbd4be73201d4f652e16e9d47ff39d1/glance/schema.py#L118

[2] http://www.json.org/

[OpenStack Infra (hudson-openstack)]

Change abandoned by Brian Rosmaita (<email address hidden>) on branch: master
Review: https://review.openstack.org/396084
Reason: Abandoning due to inactivity.