)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"9e1f6944b9c98d1c6db158285eaedb9b5aff37a2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"116cee5c_3a7f013a","updated":"2021-12-03 12:01:01.000000000","message":"Hi Julia, my observations and questions inline.","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d539e352680a4cc90d36b68aee1a76652b044752","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5bca76e4_a1b07b1c","updated":"2021-12-07 16:04:17.000000000","message":"Perhaps we need a test to simulate this workaround that we apparently support?","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"}],"sushy/resources/sessionservice/sessionservice.py":[{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"9e1f6944b9c98d1c6db158285eaedb9b5aff37a2","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"2d247374_f8599ba0","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"updated":"2021-12-03 12:01:01.000000000","message":"This seems to fix the original issue, though now it looks there is a side effect with this change: sushy now falls back to basic auth. Was this intended?\n\nBecause of this being re-raised here, it\u0027s not being handled at [1] and gets till [2] that falls back to basic auth. Sushy still works, but it will never create sessions following [1].\n\n[1] https://opendev.org/openstack/sushy/src/commit/1101ac55aff333f856c898d34c9999379a0e71d5/sushy/auth.py#L159\n[2] https://opendev.org/openstack/sushy/src/commit/1101ac55aff333f856c898d34c9999379a0e71d5/sushy/auth.py#L238","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"beb24b858debb8f9ca63e0d91d7dae7b1ddbd469","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"148dff35_abcf8e18","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"042329eb_b1b14e5e","updated":"2021-12-03 15:07:44.000000000","message":"I\u0027m just thinking what\u0027s the difference compared to version where Redfish Sessions are not supported at all. Now the SessionService code is executed where it tries to create Redfish Session but it fails and falls back to Basic auth. Why not skip it and go to basic auth at the very beginning and avoid trying initiate Redfish sessions that will never succeed.\n\nAbout fixing Redfish Sessions - have to rethink how this is implemented and used. I don\u0027t know the history of this implementation and what was discussed before, but looking at the current code problem starts with that `/SessionService` needs authenticated user and sushy wants to instantiate this class before authenticated communication is established, hence this `AccessError` handling.\n\nOne solution could be to bypass accessing `/SessionService` before POST to `/SessionService/Sessions` is made (that currently is SessionService.create_session, e.g., Session should be created without instantiating `SessionService` that connects to Redfish API too early). This goes against how currently sushy is handling all the resources and that it needs to figure out the URL for `/SessionService/Sessions`.\n\nAfter this, could remove all this `except` block unless somebody needs to have that log message. This currently no longer handles AccessError as it is still anticipated by [1]^.\n\nOr maybe need to take another step back and look at why `/SessionService` needs authenticated user. I\u0027m looking from iDRAC\u0027s implementation, maybe other BMCs don\u0027t have it and this is all workaround for iDRACs?\n\nLooking at the original patch [3] it says\n\n  Implement Redfish Sessions because some vendor implementations\n  have disabled basic auth and require a Redfish Session to access\n  resources. \n\nFrom what I see now, if these vendors are also authenticating `/SessionService`, then for them there is no working basic auth fallback.\n\n[3] https://review.opendev.org/c/openstack/sushy/+/471942","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"44cce3b7bf93363d6c3d5d95c4a6254f3ede6e6b","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"cd5a5d66_e012f025","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"148dff35_abcf8e18","updated":"2021-12-03 17:40:30.000000000","message":"\u003e I\u0027m just thinking what\u0027s the difference compared to version where Redfish Sessions are not supported at all. Now the SessionService code is executed where it tries to create Redfish Session but it fails and falls back to Basic auth. Why not skip it and go to basic auth at the very beginning and avoid trying initiate Redfish sessions that will never succeed.\n\n\nWell, in the case which was reported to us, it was already authenticated, went to re-authenticate again after loosing connectivity for an intermediate time, and was unable to re-authenitcate period whilst in the mean time things expired.\n\n\u003e \n\u003e About fixing Redfish Sessions - have to rethink how this is implemented and used. I don\u0027t know the history of this implementation and what was discussed before, but looking at the current code problem starts with that `/SessionService` needs authenticated user and sushy wants to instantiate this class before authenticated communication is established, hence this `AccessError` handling.\n\nThis is true, but in re-auth cases that is always going to happen. In a sense, this could also happen and have the exact same result.\n\n0) Ironic opens a session with sushy\n1) Operator changes user password in bmc.\n2) Operator changes password in ironic\n3) Ironic, because it doesn\u0027t track on password changes (likely a bug too) for the cache, it re-uses the session from the cache.\n4) The cached session has cached credentials, of course, and attempts utilize a resource\n5) Resource auth fails, session re-auth occurs. and we hit this error as the data was bad and it can\u0027t even find the path for the Session Service at this point.\n\nI guess that is an encouraging reason we *do* need the exception. And thinking about it, it seems like AccessError *does* likely need to be independently caught of SushyError and likely just raise that all the way back out to the client.\n\n\u003e \n\u003e One solution could be to bypass accessing `/SessionService` before POST to `/SessionService/Sessions` is made (that currently is SessionService.create_session, e.g., Session should be created without instantiating `SessionService` that connects to Redfish API too early). This goes against how currently sushy is handling all the resources and that it needs to figure out the URL for `/SessionService/Sessions`.\n\u003e \n\nYeah. :\\ It is a possibility as well. In a sense vendor inconsistency on accessing /SessionService and if credentials are validated or not if supplied... just doesn\u0027t help us here.\n\n\u003e After this, could remove all this `except` block unless somebody needs to have that log message. This currently no longer handles AccessError as it is still anticipated by [1]^.\n\nThe logging is useful. Without it I would have never figured out what was going on. So personally, I\u0027d prefer to keep it.\n\n\u003e \n\u003e Or maybe need to take another step back and look at why `/SessionService` needs authenticated user. I\u0027m looking from iDRAC\u0027s implementation, maybe other BMCs don\u0027t have it and this is all workaround for iDRACs?\n\u003e \n\u003e Looking at the original patch [3] it says\n\u003e \n\u003e   Implement Redfish Sessions because some vendor implementations\n\u003e   have disabled basic auth and require a Redfish Session to access\n\u003e   resources. \n\u003e \n\u003e From what I see now, if these vendors are also authenticating `/SessionService`, then for them there is no working basic auth fallback.\n\u003e \n\u003e [3] https://review.opendev.org/c/openstack/sushy/+/471942\n\nI don\u0027t believe /SessionService is *supposed* to enforce authentication because it is supposed to be discover-able to non-authenticatd or re-authenticating clients, but it is one of those fine detail points which is easy to gloss over. \n\nI could also be very wrong on /SessionService. My memory is a little fuzzy and I\u0027d have to pull up the standard to be sure at this point.\n\nI guess there are actually *several* conundrums. If a library client wants to access the session service, we shouldn\u0027t always force them to loose their internal credentials, which means we would almost need to build a custom request handling pipeline for the /SessionService. Which seems like a lot of code and risk.","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"d539e352680a4cc90d36b68aee1a76652b044752","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"45899a7d_5ab564b5","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"1c19a6e3_68113f7f","updated":"2021-12-07 16:04:17.000000000","message":"\u003e I don\u0027t think this is fair assessment that BMC\u0027s session support is incompatible. It\u0027s sushy that tries to do things that it shouldn\u0027t.\n\u003e \n\u003e 1) The doc you linked yesterday lists 2 ways how to get Sessions URL [1]\n\nIndeed. I am aware of that. I don\u0027t see an implication implying authentication is required or expected to resolve either.\n\n\u003e 2) sushy already implements both ways, preferring the one that does not require auth (getting the link from root\u0027s property), see [2]\n\nIndeed. \n\n\u003e 3) Even when finding the Sessions URL from unauthenticated root, sushy still creates an instance for resource that requires auth [3]\n\nI\u0027m not sure why your indicating this explicitly requires authentication.\n\nIt *does* invoke the session service.\n\nSession service *does* invoke the base resource, but this occurs *before* authentication parameters are set.\n\n\n\u003e 4) Before this patch, it worked, because there was a workaround put in place that handled that AccessError inside SessionService init.\n\nCan you please reference the code your referring to specifically?\n\nI ask so, because my interpretation of the code path is obviously not the same as yours.\n\nMy interpretation is without the exception being raised, object initialization returns None in the event of a hard failure which would have caused all future use to fail.\n\nThis method https://github.com/openstack/sushy/blob/528d502520c48f64d440a713fd2d4d5e8a9d1abe/sushy/main.py#L377 would have returned None, and it is the only place where a session service is invoked.\n\nThis gets called only in one place which is https://github.com/openstack/sushy/blob/528d502520c48f64d440a713fd2d4d5e8a9d1abe/sushy/auth.py#L159 and on line 161, we would have tried None.create_session(), which would have generated an attribute error regardless if it couldn\u0027t be loaded. This was what was being observed.  I don\u0027t see a working workaround\n\nCould you be thinking of https://github.com/openstack/sushy/blob/fa2e0015854b2885e6fbc532a0836a01f2ae329d/sushy/connector.py#L112-L135 ?\n\n\u003e 5) After this patch, that contract/workaround is broken because it no longer handles creating SessionService instance before POSTing credentials\n\nI don\u0027t understand what contract/workaround your explicitly referring to. Please walk me through the code path you believe is broken, because I just don\u0027t see it.\n\n\n\u003e 6) If previously established contract/workaround is broken as it turned out faulty, then it should be fixed as part of this solution for this issue so that it both fixes the reported issue and still keeps using Sessions. BMC support for sessions is fine.\n\nI\u0027m honestly not understanding the workaround your referring to. My perception is the call can be made before credentials are set on the payload headers for the session.\n\n\u003e [1] https://www.dmtf.org/sites/default/files/standards/documents/DSP0266_1.14.1.pdf, section 15.3.4.1\n\u003e [2] https://opendev.org/openstack/sushy/src/commit/528d502520c48f64d440a713fd2d4d5e8a9d1abe/sushy/auth.py#L154\n\u003e [3] https://opendev.org/openstack/sushy/src/commit/528d502520c48f64d440a713fd2d4d5e8a9d1abe/sushy/auth.py#L159","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"23cc162e926fd18a60f654cbaca7e600cde94e66","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"bddebeb0_e34797c8","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"2b57c5d1_5a189679","updated":"2021-12-06 16:32:17.000000000","message":"I believe the intent was \"if it doesn\u0027t work, obviously the session support is incompatible, and to never go back to session support\"\n\nI\u0027ve run this past Dmitry as well, but I don\u0027t think he has wrapped his head around it yet.\n\nI *suspect* we should document that the intent is the client be re-initialized now that we raise an AccessError if there are any major issues.\n\nSo in diagnosing the customer\u0027s problem, we did see the fallback to basic auth, but I think the lack of clarity of a failure occurring is partly what drove things to actually get stuck for them.","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"0433f78adcbf691413ed9db0be15026a9985b891","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"632aaa49_622e9535","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"2d247374_f8599ba0","updated":"2021-12-03 12:19:05.000000000","message":"thinking about this, if in the end basic auth is used every time, all this for sessionservice is unnecessary, that is, this works only because no sessions are used.","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"21ad56778193ce8f096ede7abace85cc0b7e44f3","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"4c779acd_77bc4f71","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"45899a7d_5ab564b5","updated":"2021-12-07 17:20:09.000000000","message":"\u003e \u003e 3) Even when finding the Sessions URL from unauthenticated root, sushy still creates an instance for resource that requires auth [3]\n\u003e \n\u003e I\u0027m not sure why your indicating this explicitly requires authentication.\n\nbecause calling it before auth, will receive AccessError that previously was expected and handled\n\n \n\u003e \n\u003e \u003e 4) Before this patch, it worked, because there was a workaround put in place that handled that AccessError inside SessionService init.\n\u003e \n\u003e Can you please reference the code your referring to specifically?\n\u003e \n\u003e I ask so, because my interpretation of the code path is obviously not the same as yours.\n\u003e \n\u003e My interpretation is without the exception being raised, object initialization returns None in the event of a hard failure which would have caused all future use to fail.\n\nIf there are cases where instance ends up as `None` then this is another issue that needs to be fixed without creating regression, but in normal workflow prior to this patch, the instance was _not_ None, because it failed on ResourceBase and handled that exception after which it happily created SessionService instance that is a bit broken but it did the job as workaround later.\n\n\u003e \n\u003e This method https://github.com/openstack/sushy/blob/528d502520c48f64d440a713fd2d4d5e8a9d1abe/sushy/main.py#L377 would have returned None, and it is the only place where a session service is invoked.\n\nNo, it returned some kind of SessionService instance that was able to execute create_session later.\n\n\n\u003e This gets called only in one place which is https://github.com/openstack/sushy/blob/528d502520c48f64d440a713fd2d4d5e8a9d1abe/sushy/auth.py#L159 and on line 161, we would have tried None.create_session(), which would have generated an attribute error regardless if it couldn\u0027t be loaded. This was what was being observed.  I don\u0027t see a working workaround\n\nagain, no, this would be some_instance.create_session() that would create session successfully.\n\n\u003e Could you be thinking of https://github.com/openstack/sushy/blob/fa2e0015854b2885e6fbc532a0836a01f2ae329d/sushy/connector.py#L112-L135 ?\n\nno\n\n\u003e \u003e 5) After this patch, that contract/workaround is broken because it no longer handles creating SessionService instance before POSTing credentials\n\u003e \n\u003e I don\u0027t understand what contract/workaround your explicitly referring to. Please walk me through the code path you believe is broken, because I just don\u0027t see it.\n\nI\u0027m referring to handling AccessError in this file line 64 so that semi-broken SessionService instance is still created and used later to create_session.\nAdding line 69 in this file removes that handling and propagates that error higher where it falls back to basic auth.\n\n\u003e \n\u003e \n\u003e \u003e 6) If previously established contract/workaround is broken as it turned out faulty, then it should be fixed as part of this solution for this issue so that it both fixes the reported issue and still keeps using Sessions. BMC support for sessions is fine.\n\u003e \n\u003e I\u0027m honestly not understanding the workaround your referring to. My perception is the call can be made before credentials are set on the payload headers for the session.\n\nI wasn\u0027t here focusing on solution, but showing the regression. My take on fixing would be to _not_ make SessionService call before credentials are set. That is, make create_session available based on root\u0027s Session link.","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1fa479b83a7c0219a332c055a52572bb966c8d2b","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"2f1916b4_8125af4c","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"4c779acd_77bc4f71","updated":"2021-12-09 17:50:45.000000000","message":"*sigh*\n\nMoving on!\n\nOkay, I now understand what your saying after also prototyping it locally.  I\u0027m going to revert all of the merged patches and start over on this because you are right, it is a technical regression, one for which we *do* need a test to ensure we cover.\n\nIn other news, it didn\u0027t have a semi-broken session service when it failed previously. It would have still worked. What was failing was the actual object load of the resource  allowing it to authenticate.\n\nI think the issue is actually garbage auth data poisoning the underlying request. So maybe the key is to clear it, retry, try and proceed.\n\nThat *should* fix the underlying issue when the case occurs, keeping the ability to ultimately try and recover. I feel like we need a \"Yes, session auth works\" flag to halt any possible fallback, but that is likely a bit more invasive. It would help guard a full fallback lockout of the client.... I guess. And if we know it works from a prior attempt, we could *then* actually raise an AccessError exception up if we\u0027re unable to re-auth instead of falling back.","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"fcdf50e3afd8d6247f73f304085bfb06430e927a","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"042329eb_b1b14e5e","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"632aaa49_622e9535","updated":"2021-12-03 12:52:50.000000000","message":"I think we need a separate patch that exits us back out of basic auth. If you\u0027ve got an idea there I\u0027m welcome to it, but I still think we do need to raise the access error there because even if we go back into session auth with the same client object, we\u0027re likely still in a bad state which caused us to get there in the first place.","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"00f5c8993255043d93af3cfe368eb5612ddf7064","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"1c19a6e3_68113f7f","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"bddebeb0_e34797c8","updated":"2021-12-07 06:24:10.000000000","message":"I don\u0027t think this is fair assessment that BMC\u0027s session support is incompatible. It\u0027s sushy that tries to do things that it shouldn\u0027t.\n\n1) The doc you linked yesterday lists 2 ways how to get Sessions URL [1]\n2) sushy already implements both ways, preferring the one that does not require auth (getting the link from root\u0027s property), see [2]\n3) Even when finding the Sessions URL from unauthenticated root, sushy still creates an instance for resource that requires auth [3]\n4) Before this patch, it worked, because there was a workaround put in place that handled that AccessError inside SessionService init.\n5) After this patch, that contract/workaround is broken because it no longer handles creating SessionService instance before POSTing credentials\n6) If previously established contract/workaround is broken as it turned out faulty, then it should be fixed as part of this solution for this issue so that it both fixes the reported issue and still keeps using Sessions. BMC support for sessions is fine.\n\n[1] https://www.dmtf.org/sites/default/files/standards/documents/DSP0266_1.14.1.pdf, section 15.3.4.1\n[2] https://opendev.org/openstack/sushy/src/commit/528d502520c48f64d440a713fd2d4d5e8a9d1abe/sushy/auth.py#L154\n[3] https://opendev.org/openstack/sushy/src/commit/528d502520c48f64d440a713fd2d4d5e8a9d1abe/sushy/auth.py#L159","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1532251ce758b9f6ff2b7e859d6e1589b85e6f10","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"ecabc773_9481bbc1","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"cd5a5d66_e012f025","updated":"2021-12-03 17:47:39.000000000","message":"Oh, if memory serves the history on auto was operators didn\u0027t want to have to know nor set a parameter. Thought is definitely required. :\\","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":27909,"name":"Aija Jauntēva","email":"code@clusums.eu","username":"ajya"},"change_message_id":"232459c8c2ebec9ec5e3290d8d1cc9b049b03d91","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"2b57c5d1_5a189679","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"ec1fda91_3330f182","updated":"2021-12-06 15:52:09.000000000","message":"I recognize that the reported issue needs to be fixed and there are other things in Session management that are not trivial to change.\n\nI was just asking at the very beginning - \n\nwas it intended that as the result of this change auth will always fall back to Basic Auth for at least some BMCs like iDRAC? So that Redfish sessions never get created and used?\n\nIf so, should it be documented? Mentioned in release note? As now those who watch for DEBUG messages in their logs will start seeing \"Falling back to basic authentication.\", something that was not there before. Maybe this will not be noticed as it\u0027s DEBUG level and it will keep working, at least for those BMCs that have Basic Auth enabled or SessionService does not require authentication.\n\nOr are you saying, that in your testing you didn\u0027t see this fallback and Redfish Sessions still got created and used?","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"129963cea7c137ce01f315b3b5e1ac84830b6b9c","unresolved":true,"context_lines":[{"line_number":66,"context_line":"                      \u0027SessionService. If this happens before \u0027"},{"line_number":67,"context_line":"                      \u0027authentication, we\\\u0027ll have to guess the Sessions URL.\u0027,"},{"line_number":68,"context_line":"                      ae)"},{"line_number":69,"context_line":"            raise"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def _get_sessions_collection_path(self):"},{"line_number":72,"context_line":"        \"\"\"Helper function to find the SessionCollections path\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"ec1fda91_3330f182","line":69,"range":{"start_line":69,"start_character":12,"end_line":69,"end_character":17},"in_reply_to":"ecabc773_9481bbc1","updated":"2021-12-03 21:44:44.000000000","message":"So I \u0027m thinking the original change here is correct even more now. connector.py:135 does the exact same thing when refresh fails. The underlying issue is the library will never fail back as you noted.\n\nBut should be fail back be a feature?\n\nIf we fail authentication in basic auth, we get AccessError. The ironic side of this is what should go \"wait a second, lets start over here\".\n\nI guess I\u0027m worried trying to pull us back out of basic auth mode, while easy in theory, is additionally problematic because we\u0027re adding more complexity and trying to mask from the API client consumer that a hard error has occurred. The result could be that the hard error may end up in a loop if we do it on any standard response handling either, which i think becomes more and more risky the closer we get to _op in connection.py","commit_id":"a2096cd302b343c999c6a73a0a162ac0bd8c4a23"}]}