| blackboxsw | tabrez sorry for delay there. "identity:" looks like it is autoinstall YMA | 13:42 |
|---|---|---|
| blackboxsw | tabrez sorry for delay there. "identity:" looks like it is autoinstall YAML for the ubuntu live-server installs via subiquity. I see references here https://ubuntu.com/server/docs/install/autoinstall-reference | 13:42 |
| blackboxsw | while that is YAML that includes typical cloud-config directives, the "autoinstall:" top-level key is something only honored from the live-server installer (subiquity) and not cloud-init proper. | 13:52 |
| blackboxsw | The JSON schema that the autoinstaller supports looks to be https://discourse.ubuntu.com/t/automated-server-install-schema/16615 | 13:52 |
| blackboxsw | anything under the user-data top-level key in the autoinstaller is passed through directly to cloud-init. | 13:52 |
| blackboxsw | so holmanb per standup discussion on https://github.com/canonical/cloud-init/pull/1334 what do you think you'd prefer to do. Shall we leave the top-level check intact `if not subp.which(SSH_IMPORT_ID_BINARY)` and just skip the module? | 14:41 |
| ubottu | Pull 1334 in canonical/cloud-init "declare dependency on ssh-import-id (SC-864)" [Open] | 14:41 |
| blackboxsw | holmanb: the only aversion I have to that top-level check is that we will always emit the warning from that module even when no ssh-import-id config is provided | 14:42 |
| blackboxsw | which is why I was going down the route of trying to handle individual positive cases where ssh-import-id is provided under "users:" or as a top-level key | 14:43 |
| holmanb | blackboxsw: Thanks for the feedback and ideas. I understand the argument, agreed, warnings without meaning for the end user are not ideal. Might propose something that handles the case you suggested shortly | 15:15 |
| holmanb | blackboxsw: PR updated which should handle the "users:" case.[ | 15:51 |
| blackboxsw | holmanb: thanks review comment added I look to you and falcojr to talk me down off my "pedantic" response on the ssh-import-id Pr | 16:54 |
| holmanb | @blackboxsw: false positives should be a non-issue I think, wouldn't those configs be invalid under the schema? | 16:58 |
| blackboxsw | +1 on that @holmanb . we'd have a warning anyway for bad schema | 16:59 |
| blackboxsw | the other efficiency aspect should be do we care on something so small given that we are really only talking about 2 nanoseconds between the two options it's probably a #dontcare | 17:00 |
| holmanb | @blackboxsw: it would be prudent to compare on a large config I think, though I doubt it will yield a significantly different value until checking a config that doesn't use ssh_import_id that is extremely large | 17:04 |
| blackboxsw | right the cost of detection when traversing the entire large config will probably increase that cost significantly. What you could do to mitigate that is use your generic function but only call it on config.get("user", {}) and config.get("users", {}) | 17:10 |
| blackboxsw | that way we just check `if any[("ssh_import_id" in config , nested_key( config.get("user", {}), "ssh_import_id"), nested_key( config.get("users", {})]:` | 17:11 |
| blackboxsw | but in that case you are making two func calls instead of one. so cost there too :) | 17:12 |
| blackboxsw | either way not a strong opinion. Just an observation that should really be "non-blocking" on the review/merge. | 17:12 |
| * holmanb adds notes to write "cc_save_cycles_and_make_security_team_happy" which shuts down the instance on boot ;) | 17:14 | |
| blackboxsw | hah! | 17:15 |
| * holmanb personally likes your suggestion, I'll do that | 17:15 | |
| holmanb | assuming falcojr or others don't have dissention to opine | 17:19 |
| falcojr | no opining here | 17:25 |
| holmanb | Github helpfully assumes that +1 is supposed to be a thumbs up emoji, and gives a little popup that covers the preview tab. | 18:11 |
| holmanb | One of these I _always_ want (access to preview tab), the other I _never _ want (I will type :+1: if I really want an emoji). This little feature makes sure I don't get what I want in either case. Thanks Github *facepalm* </rant> | 18:11 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!