Implement Ingress Secure Keys #420
Conversation
|
Hey, I've allowed the tests to run on your PR :) Sorry for the somehow lacking "how to contribute" doc, I have a draft I'm writing along with the v4.xx.xx branch (not pushed here yet), which should be easier to contribute to. But in the meantime, I'm happy to fill the gaps as to any question you might have, which in turn will also be helping me writing the dev doc for the v4. I think implementing the "-sk" keys support on ingress is a very good idea! I've left a few comments on your work so far. About the integration tests, ... well, I was about to explain stuff right here, but what about writing a doc on https://ovh.github.io/the-bastion/ instead? It'll be similar for v4 anyway! I'll try to do that this week. |
|
@speed47 I pushed a new commit that should help with the remarks you've made. :) Regarding the tests integration, I guess what I need to do is generate one-shot keys whose pub base64 data would be added to the proper file, but there seem to be different logic depending on the key type. So either someone is fluent enough with these tests and wants to do it quick and I'd be glad to leave that aspect to this someone, or I can try but I'll need some review and help if I fail with the expected logic. :) |
Co-authored-by: Stéphane Lesimple <speed47_github@speed47.net>
| my $fnret; | ||
|
|
||
| $fnret = OVH::Bastion::get_supported_ssh_algorithms_list(way => $way); | ||
| $fnret or osh_exit $fnret; |
There was a problem hiding this comment.
| $fnret or osh_exit $fnret; | |
| $fnret or return $fnret; |
There was a problem hiding this comment.
@speed47 I kept the osh_exit because if the function crashes I guess the underlying command should crash, shouldn't it?
There was a problem hiding this comment.
Hey, sorry, the end of the week was kinda busy :(
All functions that can be found in .inc files are expected to return, even if they return an error (where $fnret is false), so you should indeed return and not exit here.
It's up to the caller to decide whether it should exit or do something else when you func returns an error.
osh_exit is only to be used in plugins (found in bin/plugins/*)
HEXIT stands for "helper exit" and is only to be used in helpers (called under sudo, found in bin/helpers/*)
|
@speed47 small ping about my updates and questions :) |
|
You might want to rebase over current Corresponding doc is here https://ovh.github.io/the-bastion/development/setup.html#git-pre-commit-hook I'm interested in hearing about any feedback you may have on this, it's supposed to be pretty quick to setup! |
|
Do you need help on this ? I could look into finishing writing the tests |
I'll try to do the work during next week. |
I made a PR (P-EB#1) on your fork to implements the tests and things that was missing according to speed47. All tests from |
This is a draft lacking unit/functional tests.
I'd like to get your feeling about including SecureKeys.
Regarding testing, I'd be happy to write the tests but I'm not sure to have understood the code flow and style in
330-selfkeys.sh.