)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"a4f9afb1bf469aba02fee7febfb87e6ea749c10d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ddc55d97_a2c5530e","updated":"2022-01-10 21:24:19.000000000","message":"All, please let me know if you agree with the approach and what else I should be adding.  I can resolve any test issues as needed.","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"70b80e1d3b5bc91bc8c47cd3809bc398c7dc7296","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4aa5ef93_fd255b88","updated":"2022-01-14 18:04:14.000000000","message":"As others have noted, this is not the correct way to approach this. The -1 is for that. The APIs we have allow you to select the type of key generated, not the algorithm used to generate the keys.\n\nI think there are three ways I can see to do this:\n\n1. Add a new parameter to the \u0027POST /os-keypairs\u0027 API that allows you to specify the algorithm used:\n\n    POST /os-keypairs\n    {\n      \"name\": \"test-key\",\n      \"type\": \"ssh\",\n      \"algorithm\": \"ecdsa\"\n    }\n\n2. Add a config knob to allow the operator determine what type of SSH keys are issued.\n\n3. Change the default type returned (with or without API microversion) with no configuration option\n\nOption 1 appear to be the most correct approach initially, since the API would continue to behave identically across all clouds. However, that\u0027s a lot of work for something that an end-user would likely never care about (all SSH keys are kindofalmostsorta equal). Option 2 is arguably config-driven API behavior but the API is still returning SSH keys and afaict we never stated the algorithm used in keys returned by this API (the algorithm information is visible in the generated key). It\u0027s also much easier and backportable (downstream anyway). I\u0027d rule out option 3 since it\u0027s the worst of both worlds.","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"61f26a0f92ce63e35bb5ebf16528436e8b44d2c2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"dac3f6ef_264bf2fa","updated":"2022-01-14 19:43:31.000000000","message":"Based on discussions in #irc, it was decided that the best approach here is to simply create the relevant key in tempest itself - as is done here --\n\nhttps://review.opendev.org/c/openstack/tempest/+/807465\n\nI\u0027m waiting for that review to merge, and then I will abandon this patch.","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4773f3ff8fd74bad749814ce44e7109c7f76eed0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d974bdcb_c5e4adf2","updated":"2022-01-10 23:50:27.000000000","message":"I agree, as this is API change please add a spec for that in nova-specs repo\n\n- https://review.opendev.org/q/project:openstack/nova-specs","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"796c9cfb3cd7639695cb16986e17c3d25b30d59e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b078f22e_4b1693b8","updated":"2022-01-10 21:43:58.000000000","message":"The linter is likely going to complain about the stray whitespace in the test_keypairs.py file. I\u0027m good with this. The generators could be a little more DRY, but since one accepts a bitlength and the other doesn\u0027t, it\u0027s likely to result in more code instead of less.\n\nThis is going to need a new API microversion, right?","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d102e0db544538da7cfcd5449ec7c1b579aaff84","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"de3edc27_35c1a484","updated":"2022-01-10 23:43:33.000000000","message":"this is an api change and requires a spec","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"}],"nova/compute/api.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d102e0db544538da7cfcd5449ec7c1b579aaff84","unresolved":true,"context_lines":[{"line_number":6624,"context_line":"            return crypto.generate_fingerprint(public_key)"},{"line_number":6625,"context_line":"        elif key_type \u003d\u003d keypair_obj.KEYPAIR_TYPE_X509:"},{"line_number":6626,"context_line":"            return crypto.generate_x509_fingerprint(public_key)"},{"line_number":6627,"context_line":"        elif key_type \u003d\u003d keypair_obj.KEYPAIR_TYPE_ECDSA:"},{"line_number":6628,"context_line":"            return crypto.generate_fingerprint(public_key)"},{"line_number":6629,"context_line":""},{"line_number":6630,"context_line":"    def _generate_key_pair(self, user_id, key_type):"}],"source_content_type":"text/x-python","patch_set":1,"id":"4852066a_efd5b857","line":6627,"updated":"2022-01-10 23:43:33.000000000","message":"i dont think this is a valid use fo that field as currently specified.\n\nthis is used for the type of key not how its generated.\n\nif we are to extend nova to allow it to getnerate ecdsa keys we should add a new field to specify the key parmaters like cyper. we should not extned the key-type filed.","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"}],"nova/crypto.py":[{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"796c9cfb3cd7639695cb16986e17c3d25b30d59e","unresolved":true,"context_lines":[{"line_number":94,"context_line":"                     \u0027Error message: %s\u0027) % ex)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"def generate_key_pair(bits: int \u003d 2048) -\u003e ty.Tuple[str, str, str]:"},{"line_number":98,"context_line":"    key \u003d paramiko.RSAKey.generate(bits)"},{"line_number":99,"context_line":"    keyout \u003d io.StringIO()"},{"line_number":100,"context_line":"    key.write_private_key(keyout)"}],"source_content_type":"text/x-python","patch_set":1,"id":"326c0a32_cbf755c1","line":97,"updated":"2022-01-10 21:43:58.000000000","message":"It may make sense to turn this into a compatibility alias for a new `generate_rsa_key_pair` function, to differentiate from the new one being added (or just rename it, I\u0027m not sure whether Nova considers this an external API).","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d102e0db544538da7cfcd5449ec7c1b579aaff84","unresolved":true,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"def generate_ecdsa_key_pair() -\u003e ty.Tuple[str, str, str]:"},{"line_number":108,"context_line":"    key \u003d paramiko.ECDSAKey.generate()"},{"line_number":109,"context_line":"    keyout \u003d io.StringIO()"},{"line_number":110,"context_line":"    key.write_private_key(keyout)"},{"line_number":111,"context_line":"    private_key \u003d keyout.getvalue()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fe9e568_bb10d145","line":108,"updated":"2022-01-10 23:43:33.000000000","message":"i tought we were not allowed to use paramiko in fips mode so i dont see this as being a step forward in that respect","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"}],"nova/objects/keypair.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"9e7c520d9800d1b70459d8115fa504baa4e5d906","unresolved":true,"context_lines":[{"line_number":24,"context_line":"from nova.objects import base"},{"line_number":25,"context_line":"from nova.objects import fields"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"KEYPAIR_TYPE_ECDSA \u003d \u0027ecdsa\u0027"},{"line_number":28,"context_line":"KEYPAIR_TYPE_SSH \u003d \u0027ssh\u0027"},{"line_number":29,"context_line":"KEYPAIR_TYPE_X509 \u003d \u0027x509\u0027"},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":1,"id":"6ae8d617_cdff9b6a","line":27,"updated":"2022-01-10 21:32:15.000000000","message":"The existing KEYPAIR_TYPE values seem to distinguish the encoding method (ssh encoding or x509 encoding) not the ssh key algorithm. You might want to make this ssh_ecdsa (or similar) to continue to have that extra axis of info present.","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"},{"author":{"_account_id":5263,"name":"Jeremy Stanley","display_name":"fungi","email":"fungi@yuggoth.org","username":"fungi","status":"missing, presumed fed"},"change_message_id":"796c9cfb3cd7639695cb16986e17c3d25b30d59e","unresolved":true,"context_lines":[{"line_number":25,"context_line":"from nova.objects import fields"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"KEYPAIR_TYPE_ECDSA \u003d \u0027ecdsa\u0027"},{"line_number":28,"context_line":"KEYPAIR_TYPE_SSH \u003d \u0027ssh\u0027"},{"line_number":29,"context_line":"KEYPAIR_TYPE_X509 \u003d \u0027x509\u0027"},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"16f48873_f2c5543c","line":28,"updated":"2022-01-10 21:43:58.000000000","message":"In a similar vein, this is now ambiguous, and it might be best to have a new `KEYPAIR_TYPE_RSA` const here, and make `ssh` a (deprecated?) synonym for `rsa` instead.","commit_id":"a23712c3c289a7bbaabf7618a9ce83a8db17d411"}]}
