From ad4b851c7fbf4b914d1c2b6c55957d4315423326 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Mon, 19 Dec 2016 12:18:09 +0100 Subject: [PATCH] =?UTF-8?q?rest:=20string=20=E2=86=92=20UUID=20conversion?= =?UTF-8?q?=20for=20resource.id=20to=20be=20unique=20per=20user?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changes the UUID5 based mechanism so it depends on the user trying to CRUD the resource. This makes sure that when using this kind of transformation, the resource id is converted to a unique id for the user, while preventing conflicting if every user wants to create a "foobar" resource. Change-Id: Iebaf3b9f8e0a198af0156008710e0c1253dc5f9d Closes-Bug: #1617918 --- ...97987e38570_no_more_slash_and_reencode.py} | 26 +++++-- gnocchi/rest/__init__.py | 71 +++++++++++-------- gnocchi/tests/gabbi/gabbits/base.yaml | 4 +- .../tests/gabbi/gabbits/batch-measures.yaml | 2 +- .../tests/gabbi/gabbits/resource-type.yaml | 15 ++-- .../tests/gabbi/gabbits/transformedids.yaml | 24 ++++++- gnocchi/utils.py | 25 +++---- .../notes/uuid5-change-8a8c467d2b2d4c85.yaml | 12 ++++ run-upgrade-tests.sh | 8 ++- 9 files changed, 123 insertions(+), 64 deletions(-) rename gnocchi/indexer/alembic/versions/{397987e38570_no_more_slash.py => 397987e38570_no_more_slash_and_reencode.py} (87%) create mode 100644 releasenotes/notes/uuid5-change-8a8c467d2b2d4c85.yaml diff --git a/gnocchi/indexer/alembic/versions/397987e38570_no_more_slash.py b/gnocchi/indexer/alembic/versions/397987e38570_no_more_slash_and_reencode.py similarity index 87% rename from gnocchi/indexer/alembic/versions/397987e38570_no_more_slash.py rename to gnocchi/indexer/alembic/versions/397987e38570_no_more_slash_and_reencode.py index 77d58404..34363257 100644 --- a/gnocchi/indexer/alembic/versions/397987e38570_no_more_slash.py +++ b/gnocchi/indexer/alembic/versions/397987e38570_no_more_slash_and_reencode.py @@ -13,7 +13,7 @@ # under the License. # -"""no-more-slash +"""Remove slashes from original resource IDs, recompute their id with creator Revision ID: 397987e38570 Revises: aba5a217ca9b @@ -49,7 +49,8 @@ resource_table = sa.Table( sqlalchemy_utils.types.uuid.UUIDType(), nullable=False), sa.Column('original_resource_id', sa.String(255)), - sa.Column('type', sa.String(255)) + sa.Column('type', sa.String(255)), + sa.Column('creator', sa.String(255)) ) resourcehistory_table = sa.Table( @@ -100,15 +101,28 @@ def upgrade(): nullable=False), ) - for resource in connection.execute(resource_table.select().where( - resource_table.c.original_resource_id.like('%/%'))): + for resource in connection.execute(resource_table.select()): + + if resource_table.c.original_resource_id is None: + # statsd resource has no original_resource_id and is NULL + continue + + try: + orig_as_uuid = uuid.UUID( + str(resource_table.c.original_resource_id)) + except ValueError: + pass + else: + if orig_as_uuid == resource_table.c.id: + continue + new_original_resource_id = resource.original_resource_id.replace( '/', '_') if six.PY2: new_original_resource_id = new_original_resource_id.encode('utf-8') new_id = sa.literal(uuidtype.process_bind_param( - str(uuid.uuid5(utils.RESOURCE_ID_NAMESPACE, - new_original_resource_id)), + str(utils.ResourceUUID( + new_original_resource_id, resource.creator)), connection.dialect)) # resource table diff --git a/gnocchi/rest/__init__.py b/gnocchi/rest/__init__.py index 41139969..6878b5ec 100644 --- a/gnocchi/rest/__init__.py +++ b/gnocchi/rest/__init__.py @@ -14,6 +14,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import functools import itertools import uuid @@ -848,8 +849,10 @@ class ResourceController(rest.RestController): def __init__(self, resource_type, id): self._resource_type = resource_type + creator = pecan.request.auth_helper.get_current_user( + pecan.request.headers) try: - self.id = utils.ResourceUUID(id) + self.id = utils.ResourceUUID(id, creator) except ValueError: abort(404, indexer.NoSuchResource(id)) self.metric = NamedMetricController(str(self.id), self._resource_type) @@ -929,8 +932,15 @@ def schema_for(resource_type): return ResourceSchema(resource_type.schema) -def ResourceID(value): - return (six.text_type(value), utils.ResourceUUID(value)) +def ResourceUUID(value, creator): + try: + return utils.ResourceUUID(value, creator) + except ValueError as e: + raise voluptuous.Invalid(e) + + +def ResourceID(value, creator): + return (six.text_type(value), ResourceUUID(value, creator)) class ResourcesController(rest.RestController): @@ -946,7 +956,9 @@ class ResourcesController(rest.RestController): # NOTE(sileht): we need to copy the dict because when change it # and we don't want that next patch call have the "id" schema = dict(schema_for(self._resource_type)) - schema["id"] = ResourceID + creator = pecan.request.auth_helper.get_current_user( + pecan.request.headers) + schema["id"] = functools.partial(ResourceID, creator=creator) body = deserialize_and_validate(schema) body["original_resource_id"], body["id"] = body["id"] @@ -956,8 +968,6 @@ class ResourcesController(rest.RestController): } target.update(body) enforce("create resource", target) - creator = pecan.request.auth_helper.get_current_user( - pecan.request.headers) rid = body['id'] del body['id'] try: @@ -1003,8 +1013,7 @@ class ResourcesController(rest.RestController): def delete(self, **kwargs): # NOTE(sileht): Don't allow empty filter, this is going to delete # the entire database. - attr_filter = deserialize_and_validate( - SearchResourceTypeController.ResourceSearchSchema) + attr_filter = deserialize_and_validate(ResourceSearchSchema) # the voluptuous checks everything, but it is better to # have this here. @@ -1129,16 +1138,16 @@ class QueryStringSearchAttrFilter(object): return cls._parsed_query2dict(parsed_query) -def _ResourceSearchSchema(v): - """Helper method to indirect the recursivity of the search schema""" - return SearchResourceTypeController.ResourceSearchSchema(v) +def ResourceSearchSchema(v): + return _ResourceSearchSchema()(v) -class SearchResourceTypeController(rest.RestController): - def __init__(self, resource_type): - self._resource_type = resource_type +def _ResourceSearchSchema(): + user = pecan.request.auth_helper.get_current_user( + pecan.request.headers) + _ResourceUUID = functools.partial(ResourceUUID, creator=user) - ResourceSearchSchema = voluptuous.Schema( + return voluptuous.Schema( voluptuous.All( voluptuous.Length(min=0, max=1), { @@ -1155,31 +1164,36 @@ class SearchResourceTypeController(rest.RestController): voluptuous.Length(min=1, max=1), voluptuous.Any( {"id": voluptuous.Any( - utils.ResourceUUID, [utils.ResourceUUID]), + [_ResourceUUID], _ResourceUUID), voluptuous.Extra: voluptuous.Extra})), voluptuous.Any( u"and", u"∨", u"or", u"∧", u"not", ): voluptuous.All( - [_ResourceSearchSchema], voluptuous.Length(min=1) + [ResourceSearchSchema], voluptuous.Length(min=1) ) } ) ) - @classmethod - def parse_and_validate_qs_filter(cls, query): + +class SearchResourceTypeController(rest.RestController): + def __init__(self, resource_type): + self._resource_type = resource_type + + @staticmethod + def parse_and_validate_qs_filter(query): try: attr_filter = QueryStringSearchAttrFilter.parse(query) except InvalidQueryStringSearchAttrFilter as e: raise abort(400, e) - return voluptuous.Schema(cls.ResourceSearchSchema, + return voluptuous.Schema(ResourceSearchSchema, required=True)(attr_filter) def _search(self, **kwargs): if pecan.request.body: - attr_filter = deserialize_and_validate(self.ResourceSearchSchema) + attr_filter = deserialize_and_validate(ResourceSearchSchema) elif kwargs.get("filter"): attr_filter = self.parse_and_validate_qs_filter(kwargs["filter"]) else: @@ -1328,13 +1342,16 @@ class SearchMetricController(rest.RestController): class ResourcesMetricsMeasuresBatchController(rest.RestController): - MeasuresBatchSchema = voluptuous.Schema( - {ResourceID: {six.text_type: MeasuresListSchema}} - ) - @pecan.expose('json') def post(self, create_metrics=False): - body = deserialize_and_validate(self.MeasuresBatchSchema) + creator = pecan.request.auth_helper.get_current_user( + pecan.request.headers) + MeasuresBatchSchema = voluptuous.Schema( + {functools.partial(ResourceID, creator=creator): + {six.text_type: MeasuresListSchema}} + ) + + body = deserialize_and_validate(MeasuresBatchSchema) known_metrics = [] unknown_metrics = [] @@ -1349,8 +1366,6 @@ class ResourcesMetricsMeasuresBatchController(rest.RestController): known_names = [m.name for m in metrics] if strutils.bool_from_string(create_metrics): - creator = pecan.request.auth_helper.get_current_user( - pecan.request.headers) already_exists_names = [] for name in names: if name not in known_names: diff --git a/gnocchi/tests/gabbi/gabbits/base.yaml b/gnocchi/tests/gabbi/gabbits/base.yaml index eeb3ed9f..ef097711 100644 --- a/gnocchi/tests/gabbi/gabbits/base.yaml +++ b/gnocchi/tests/gabbi/gabbits/base.yaml @@ -134,13 +134,13 @@ tests: project_id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea status: 201 response_headers: - location: $SCHEME://$NETLOC/v1/resource/generic/8d835270-2834-5e55-a693-fd0cf91cba3d + location: $SCHEME://$NETLOC/v1/resource/generic/2d869568-70d4-5ed6-9891-7d7a3bbf572d response_json_paths: type: generic started_at: "2014-01-03T02:02:02+00:00" project_id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea created_by_project_id: 99d13f22-3618-4288-82b8-6512ded77e4f - id: 8d835270-2834-5e55-a693-fd0cf91cba3d + id: 2d869568-70d4-5ed6-9891-7d7a3bbf572d original_resource_id: 1.2.3.4 - name: get status denied diff --git a/gnocchi/tests/gabbi/gabbits/batch-measures.yaml b/gnocchi/tests/gabbi/gabbits/batch-measures.yaml index 6cc3710c..ae3b454e 100644 --- a/gnocchi/tests/gabbi/gabbits/batch-measures.yaml +++ b/gnocchi/tests/gabbi/gabbits/batch-measures.yaml @@ -244,7 +244,7 @@ tests: response_json_paths: $.description.cause: "Unknown resources" $.description.detail: - - resource_id: "301dbf9a-4fce-52b6-9010-4484c469dcec" + - resource_id: "6b8e287d-c01a-538c-979b-a819ee49de5d" original_resource_id: "foobar" - name: push measurements to named metrics and resource with create_metrics with wrong measure objects diff --git a/gnocchi/tests/gabbi/gabbits/resource-type.yaml b/gnocchi/tests/gabbi/gabbits/resource-type.yaml index dae792f0..9ffd74e3 100644 --- a/gnocchi/tests/gabbi/gabbits/resource-type.yaml +++ b/gnocchi/tests/gabbi/gabbits/resource-type.yaml @@ -6,6 +6,11 @@ fixtures: - ConfigFixture +defaults: + request_headers: + x-user-id: 0fbb231484614b1a80131fc22f6afc9c + x-project-id: f3d41b770cc14f0bb94a1d5be9c0e3ea + tests: - name: list resource type @@ -222,8 +227,6 @@ tests: - name: post invalid resource POST: /v1/resource/my_custom_resource request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: id: d11edfca-4393-4fda-b94d-b05a3a1b3747 @@ -239,8 +242,6 @@ tests: - name: post invalid resource uuid POST: $LAST_URL request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: id: d11edfca-4393-4fda-b94d-b05a3a1b3747 @@ -258,8 +259,6 @@ tests: - name: post custom resource POST: $LAST_URL request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: id: d11edfca-4393-4fda-b94d-b05a3a1b3747 @@ -276,8 +275,6 @@ tests: - name: patch custom resource PATCH: /v1/resource/my_custom_resource/d11edfca-4393-4fda-b94d-b05a3a1b3747 request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: name: foo @@ -303,8 +300,6 @@ tests: - name: post resource with default POST: /v1/resource/my_custom_resource request_headers: - x-user-id: 0fbb2314-8461-4b1a-8013-1fc22f6afc9c - x-project-id: f3d41b77-0cc1-4f0b-b94a-1d5be9c0e3ea content-type: application/json data: id: c4110aec-6e5c-43fa-b8c5-ffdfbca3ce59 diff --git a/gnocchi/tests/gabbi/gabbits/transformedids.yaml b/gnocchi/tests/gabbi/gabbits/transformedids.yaml index b5ab2092..cc544f11 100644 --- a/gnocchi/tests/gabbi/gabbits/transformedids.yaml +++ b/gnocchi/tests/gabbi/gabbits/transformedids.yaml @@ -66,8 +66,26 @@ tests: project_id: f3d41b770cc14f0bb94a1d5be9c0e3ea status: 400 response_strings: - - "Invalid input: not a valid value for dictionary value @ data[" - - "'id'] " + - "'/' is not supported in resource id" + + + - name: post new resource non uuid again different user + POST: /v1/resource/generic + request_headers: + x-user-id: 0fbb231484614b1a80131fc22f6afc9b + x-project-id: f3d41b770cc14f0bb94a1d5be9c0e3ea + data: + id: generic zero + metrics: + cpu.util: + archive_policy_name: medium + status: 201 + response_json_paths: + created_by_user_id: 0fbb231484614b1a80131fc22f6afc9b + created_by_project_id: f3d41b770cc14f0bb94a1d5be9c0e3ea + response_headers: + # is a UUID + location: /v1/resource/generic/[a-f0-9-]{36}/ - name: post new resource non uuid POST: /v1/resource/generic @@ -151,7 +169,7 @@ tests: archive_policy_name: medium status: 400 response_strings: - - not a valid value for + - transformable resource id >255 max allowed characters for dictionary value - name: post long non uuid resource id POST: $LAST_URL diff --git a/gnocchi/utils.py b/gnocchi/utils.py index 50dc303b..1b3cd476 100644 --- a/gnocchi/utils.py +++ b/gnocchi/utils.py @@ -39,23 +39,24 @@ LOG = log.getLogger(__name__) RESOURCE_ID_NAMESPACE = uuid.UUID('0a7a15ff-aa13-4ac2-897c-9bdf30ce175b') -def ResourceUUID(value): +def ResourceUUID(value, creator): if isinstance(value, uuid.UUID): return value if '/' in value: raise ValueError("'/' is not supported in resource id") try: - try: - return uuid.UUID(value) - except ValueError: - if len(value) <= 255: - if six.PY2: - value = value.encode('utf-8') - return uuid.uuid5(RESOURCE_ID_NAMESPACE, value) - raise ValueError( - 'transformable resource id >255 max allowed characters') - except Exception as e: - raise ValueError(e) + return uuid.UUID(value) + except ValueError: + if len(value) <= 255: + # value/creator must be str (unicode) in Python 3 and str (bytes) + # in Python 2. It's not logical, I know. + if six.PY2: + value = value.encode('utf-8') + creator = creator.encode('utf-8') + return uuid.uuid5(RESOURCE_ID_NAMESPACE, + value + "\x00" + creator) + raise ValueError( + 'transformable resource id >255 max allowed characters') def UUID(value): diff --git a/releasenotes/notes/uuid5-change-8a8c467d2b2d4c85.yaml b/releasenotes/notes/uuid5-change-8a8c467d2b2d4c85.yaml new file mode 100644 index 00000000..ec6b6c51 --- /dev/null +++ b/releasenotes/notes/uuid5-change-8a8c467d2b2d4c85.yaml @@ -0,0 +1,12 @@ +--- +issues: + - >- + The conversion mechanism provided by the API to convert non-UUID resource + id to UUID is now also based on the user creating/accessing the resource. + This makes sure that the conversion generates a unique UUID for the user + and that several users can use the same string as `original_resource_id`. +upgrade: + - >- + Since `original_resource_id` is now unique per creator, that means users + cannot refer to resource by using the `original_resource_id` if the + resource was not created by them. diff --git a/run-upgrade-tests.sh b/run-upgrade-tests.sh index 4220e252..5bcdd73f 100755 --- a/run-upgrade-tests.sh +++ b/run-upgrade-tests.sh @@ -109,12 +109,16 @@ RESOURCE_IDS=( "5a301761-cccc-46e2-8900-8b4f6fe6675a" ) # NOTE(sileht): / are now _ -[ "$have_resource_type_post" ] && RESOURCE_ID_EXT="5a301761_dddd_46e2_8900_8b4f6fe6675a" +# NOTE(jdanjou): and we reencode for admin:admin, but we cannot authenticate as +# admin:admin in basic since ":" is forbidden in any username, so let's use the direct +# computed ID +[ "$have_resource_type_post" ] && RESOURCE_ID_EXT="517920a9-2e50-58b8-88e8-25fd7aae1d8f" + dump_data $GNOCCHI_DATA/new # NOTE(sileht): change the output of the old gnocchi to compare with the new without '/' $GSED -i -e "s,5a301761/dddd/46e2/8900/8b4f6fe6675a,5a301761_dddd_46e2_8900_8b4f6fe6675a,g" \ - -e "s,19235bb9-35ca-5f55-b7db-165cfb033c86,fe1bdabf-d94c-5b3a-af1e-06bdff53f228,g" $GNOCCHI_DATA/old/resources.list + -e "s,19235bb9-35ca-5f55-b7db-165cfb033c86,517920a9-2e50-58b8-88e8-25fd7aae1d8f,g" $GNOCCHI_DATA/old/resources.list echo "* Checking output difference between Gnocchi $old_version and $new_version" diff -uNr $GNOCCHI_DATA/old $GNOCCHI_DATA/new