Update patch set 15

Patch Set 15:

(1 comment)

Patch-set: 15
Attention: {"person_ident":"Gerrit User 27900 \u003c27900@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_28356\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 28356 \u003c28356@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_28356\u003e replied on the change"}

8305797037e04aa67186e4aec1e3ee8adfd25ab9

@@ -16,6 +16,24 @@
       "message": "still -1\n\n- previous change of adding \"domain\" field was unnecessary, since it is already present under the user. Sadly this became clear only after landing the implementation and starting deploying the feature\n- jsonschema was modified in a previous change with an error (see https://review.opendev.org/c/openstack/keystone/+/915401). Sadly also this became clear only accidentally from another initiative\n- all this forces me to become nitpicky. Mapping schema is already over-complicated for understanding (especially having domain property duplicated causes confusion for most parties that started relying on the changed functionality - nobody understands clearly its purpose and side-effects when user.domain !\u003d domain or only one of them set). Adding new field into the schema (projects_json) just to save few lines of code is not a good design and strongly differs from the pattern in all other Keystone APIs and especially in the same api (https://opendev.org/openstack/keystone/src/branch/master/keystone/federation/utils.py#L107 or https://opendev.org/openstack/keystone/src/branch/master/keystone/federation/utils.py#L129 for reference). I am strongly against of adding \"projects_json\" parameter and prefer extending existing \"projects\" field\n\nThe change purpose is good, the API change is bad from my perspective",
       "revId": "8305797037e04aa67186e4aec1e3ee8adfd25ab9",
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
+    },
+    {
+      "unresolved": false,
+      "key": {
+        "uuid": "d192afcb_c899f0c1",
+        "filename": "/PATCHSET_LEVEL",
+        "patchSetId": 15
+      },
+      "lineNbr": 0,
+      "author": {
+        "id": 28356
+      },
+      "writtenOn": "2024-04-10T11:41:58Z",
+      "side": 1,
+      "message": "Hello Artem, \nThe previous normalization of the Domain was necessary. We tried to use that ourselves, and we just found out that it does not work after checking the code. The domain in the user definition was expected to override the domain in the root of the definition, but it was not doing that. It can be used as an override mechanism, as it is already used for other elements, such as group and project. Then, one can have a generic domain defined at the root of the mapping definition, and an override at a specific element (user/project/group).\n\nAgain, we did not introduce that domain at the root, we just made the code to respect/follow the domain defined at the user(object) that is defined in the attribute definition.\n\nRegarding the schema that you pointed out. That indeed slipped by, but that is not used in the code, I checked, and we have that internally. We never saw the problem because we were not adding the schema_version in the attribute mapping itself, we were just passing via the API. I already gave my +1 there in your patch. Sorry for that mistake.\n\nRegarding the field \"projects_json\", what makes you feel so strongly against it? Could you describe/explain in detail?\n\nI mean, just saying that you did not understand/like the use of domain at the root of the attribute mapping schema is not enough to justify something like that. Also, a small mistake made in a patch (that we had to maintain and worked to carry it along 4 years, where we never understood why people were so strong against it) does not justify crucifying somebody; anyways, water under the bridge. \n\nI also would like to stress that the use of \"domain\" at the attribute mapping schema root level was not introduced by us. We just added the support to override it in the user object that is defined in the attribute mapping schema with the very first attribute schema; this in fact was already being done for project and group definitions. That processing of domain in different levels is an override mechanism; if you do not understand what that means, I can try to illustrate for you some more detailed explanation. However, and again, it is not introduced by me, and it is what we understand from the code we can see.",
+      "parentUuid": "23247a08_cc07ced4",
+      "revId": "8305797037e04aa67186e4aec1e3ee8adfd25ab9",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
     }
   ]
 }