diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index 8576e3c3fc..6624f1a484 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,10 @@ REST API Version History ======================== +1.98 (Flamingo) + +Add support for patching object attributes with keys containing ~ or /. + 1.97 (Flamingo) ----------------------- diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 61397d385b..115fba914c 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -399,6 +399,17 @@ def patched_validate_with_schema(patched_dict, schema, validator=None): validator('patch', patched_dict) +def allow_special_characters_in_patch_fields(): + """Check if special characters are allowed in patch field keys.""" + return (api.request.version.minor + >= versions.MINOR_98_SUPPORT_SPECIAL_CHAR_IN_ATTRIBUTES) + + +def decode_json_pointer(path): + """Decode JSON pointer as per RFC 6901 to support '~' and '/'.""" + return path.replace('~1', '/').replace('~0', '~') + + def patch_validate_allowed_fields(patch, allowed_fields): """Validate that a patch list only modifies allowed fields. @@ -410,7 +421,10 @@ def patch_validate_allowed_fields(patch, allowed_fields): """ fields = set() for p in patch: - path = p['path'].split('/')[1] + if allow_special_characters_in_patch_fields(): + path = decode_json_pointer(p['path'].split('/')[1]) + else: + path = p['path'].split('/')[1] if path not in allowed_fields: msg = _("Cannot patch %s. Only the following can be updated: %s") raise exception.Invalid( diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index e8259ae649..a3e73855c3 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -135,6 +135,7 @@ BASE_VERSION = 1 # v1.95: Add node support for disable_power_off # v1.96: Migrate inspection rules from Inspector # v1.97: Add description field to port. +# v1.98: Add support for object attributes with keys containing ~ or /. MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -234,6 +235,7 @@ MINOR_94_PORT_NODENAME = 94 MINOR_95_DISABLE_POWER_OFF = 95 MINOR_96_INSPECTION_RULES = 96 MINOR_97_PORT_DESCRIPTION = 97 +MINOR_98_SUPPORT_SPECIAL_CHAR_IN_ATTRIBUTES = 98 # When adding another version, update: # - MINOR_MAX_VERSION @@ -241,7 +243,7 @@ MINOR_97_PORT_DESCRIPTION = 97 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_97_PORT_DESCRIPTION +MINOR_MAX_VERSION = MINOR_98_SUPPORT_SPECIAL_CHAR_IN_ATTRIBUTES # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index b39f6e9891..3a03ca2a4d 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -848,7 +848,7 @@ RELEASE_MAPPING = { # make it below. To release, we will preserve a version matching # the release as a separate block of text, like above. 'master': { - 'api': '1.97', + 'api': '1.98', 'rpc': '1.61', 'objects': { 'Allocation': ['1.1'], diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index 1f12798749..1d7ed547fb 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -90,6 +90,62 @@ class TestApiUtils(base.TestCase): self.assertRaises(exception.PatchError, utils.apply_jsonpatch, doc, patch) + def test_apply_jsonpatch_with_tilde(self): + """Unescaped tilde fails.""" + doc = {"foo": {"bar~baz": "value"}} + patch = [{"op": "replace", "path": "/foo/bar~baz", + "value": "new_value"}] + self.assertRaises(exception.PatchError, + utils.apply_jsonpatch, doc, patch) + + def test_apply_jsonpatch_with_slash(self): + """Unescaped forward slash fails.""" + doc = {"foo": {"bar/baz": "value"}} + patch = [{"op": "replace", "path": "/foo/bar/baz", + "value": "new_value"}] + self.assertRaises(exception.PatchError, + utils.apply_jsonpatch, doc, patch) + + def test_apply_jsonpatch_with_escaped_tilde(self): + """Escaped tilde (~0) should work per RFC 6902.""" + doc = {"foo": {"bar~baz": "value"}} + patch = [{"op": "replace", "path": "/foo/bar~0baz", + "value": "new_value"}] + expected = {"foo": {"bar~baz": "new_value"}} + result = utils.apply_jsonpatch(doc, patch) + self.assertEqual(expected, result) + + def test_apply_jsonpatch_with_escaped_slash(self): + """Escaped slash (~1) should work per RFC 6902.""" + doc = {"foo": {"bar/baz": "value"}} + patch = [{"op": "replace", "path": "/foo/bar~1baz", + "value": "new_value"}] + expected = {"foo": {"bar/baz": "new_value"}} + result = utils.apply_jsonpatch(doc, patch) + self.assertEqual(expected, result) + + def test_apply_jsonpatch_with_escaped_complex_path(self): + doc = {"foo": {"~bar/baz~": "value"}} + patch = [{"op": "replace", "path": "/foo/~0bar~1baz~0", + "value": "new_value"}] + expected = {"foo": {"~bar/baz~": "new_value"}} + result = utils.apply_jsonpatch(doc, patch) + self.assertEqual(expected, result) + + def test_apply_jsonpatch_with_invalid_escape_sequence(self): + doc = {"foo": {"bar": "value"}} + patch = [{"op": "replace", "path": "/foo/bar~2baz", + "value": "new_value"}] + self.assertRaises(exception.PatchError, + utils.apply_jsonpatch, doc, patch) + + def test_apply_jsonpatch_with_incomplete_escape(self): + doc = {"foo": {"bar": "value"}} + patch = [{"op": "replace", "path": "/foo/bar~", + "value": "new_value"}] + self.assertRaises(exception.PatchError, + utils.apply_jsonpatch, doc, patch) + def test_get_patch_values_no_path(self): patch = [{'path': '/name', 'op': 'update', 'value': 'node-0'}] path = '/invalid' @@ -300,10 +356,12 @@ class TestApiUtils(base.TestCase): validate) self.assertIn("big ouch", str(e)) - def test_patch_validate_allowed_fields(self): - allowed_fields = ['one', 'two', 'three'] + @mock.patch.object(api, 'request', autospec=False) + def test_patch_validate_allowed_fields(self, mock_request): + mock_request.version.minor = 94 + allowed_fields = ['one', 'two', 'three', 'four/five'] - # patch all + # patch all (except 'four/five') self.assertEqual( {'one', 'two', 'three'}, utils.patch_validate_allowed_fields([ @@ -319,6 +377,13 @@ class TestApiUtils(base.TestCase): {'path': '/one'}, ], allowed_fields)) + # patch one (special field not supported) + self.assertRaises( + exception.Invalid, + utils.patch_validate_allowed_fields, + [{'path': '/four~1five'}], + allowed_fields) + # patch invalid field e = self.assertRaises( exception.Invalid, @@ -330,8 +395,31 @@ class TestApiUtils(base.TestCase): "one, two, three", str(e)) @mock.patch.object(api, 'request', autospec=False) - def test_sanitize_dict(self, mock_req): - mock_req.public_url = 'http://192.0.2.1:5050' + def test_patch_validate_allowed_special_fields(self, mock_request): + mock_request.version.minor = 98 + allowed_fields = ['one', 'two', 'three', 'four/five', 'four~five'] + + # patch all + self.assertEqual( + {'one', 'two', 'three', 'four/five', 'four~five'}, + utils.patch_validate_allowed_fields([ + {'path': '/one'}, + {'path': '/two'}, + {'path': '/three'}, + {'path': '/four~0five'}, + {'path': '/four~1five'}, + ], allowed_fields)) + + # patch one (special field) + self.assertEqual( + {'four/five'}, + utils.patch_validate_allowed_fields([ + {'path': '/four~1five'}, + ], allowed_fields)) + + @mock.patch.object(api, 'request', autospec=False) + def test_sanitize_dict(self, mock_request): + mock_request.public_url = 'http://192.0.2.1:5050' node = obj_utils.get_test_node( self.context, diff --git a/releasenotes/notes/support-special-characters-in-patch-field-e077fb994661362c.yaml b/releasenotes/notes/support-special-characters-in-patch-field-e077fb994661362c.yaml new file mode 100644 index 0000000000..0fa95e7d17 --- /dev/null +++ b/releasenotes/notes/support-special-characters-in-patch-field-e077fb994661362c.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Updates the patch validation logic to support special characters + (``~`` and ``/``) in field keys, provided they align with the + escaping rules defined in RFC 6901 (JSON Pointer) as required for + the `path` field specified in RFC 6902 (JSON Patch).