feat(*): add support for http basic auth to bindle server#165
feat(*): add support for http basic auth to bindle server#165vdice merged 3 commits intodeislabs:mainfrom
Conversation
- also adds support for ignoring bindle server cert errors Signed-off-by: Vaughn Dice <vaughn.dice@fermyon.com>
src/wagi_app.rs
Outdated
| (Some(bindle_id), None, Some(bindle_url), None) => { | ||
| // Cases: got a bindle id and server URL. Can't have a bindir dir or module file. | ||
| // No basic http auth creds for bindle server supplied | ||
| (Some(bindle_id), None, Some(bindle_url), None, None, None) => { |
There was a problem hiding this comment.
This duplicates a considerable amount of code and breaks up the flow of cases even more than it was already broken up (which I admit was quite a bit).
An alternative to consider:
- Keep the match pattern as it is
- Write a helper method that matches on the user and password values to create an auth object
- Use that auth object to create the Bindle client
The perennial problem with this, I know, and something that really vexes me about the Bindle client design, is the auth type being a type-level parameter of the client. The solution is to nick the AnyAuth type from the Hippo CLI and use that as the auth type.
This should be how the Bindle client works by default and I will die on this hill.
There was a problem hiding this comment.
Thanks @itowlson. I pushed a commit which refactors things with an approach derived from hippo-cli. I do like how it slims down the CLI config -> emplacing a remote bindle flow, however, I'm anticipating there are still some things to tweak. One: I couldn't figure out how to keep the 'Debug' trait on the structs that use the newly introduced BindleConnectionInfo struct/impl due to the underlying token_manager, so I removed the trait to get things working -- no doubt this isn't ideal 😂. WDYT?
Signed-off-by: Vaughn Dice <vaughn.dice@fermyon.com>
itowlson
left a comment
itowlson
left a comment
There was a problem hiding this comment.
Minor tweaks but otherwise looks really nice and clean
src/wagi_config.rs
Outdated
| ModuleConfigFile(PathBuf), | ||
| StandaloneBindle(PathBuf, bindle::Id), | ||
| RemoteBindle(url::Url, bindle::Id), | ||
| RemoteBindle(bindle::Id, BindleConnectionInfo), |
There was a problem hiding this comment.
Reverse order of tuple to stay consistent with the StandaloneBindle case
| RemoteBindle(bindle::Id, BindleConnectionInfo), | |
| RemoteBindle(BindleConnectionInfo, bindle::Id), |
src/handler_loader/emplacer.rs
Outdated
| let client = bindle::client::Client::new(bindle_base_url.as_str(), token)?; | ||
|
|
||
| self.emplace_bindle(&client, id).await | ||
| async fn emplace_remote_bindle(self, id: &bindle::Id, bindle_connection_info: crate::bindle_util::BindleConnectionInfo) -> anyhow::Result<EmplacedHandlerConfiguration> { |
There was a problem hiding this comment.
Consider reverting arg order to retain consistency with emplace_standalone_bindle
| async fn emplace_remote_bindle(self, id: &bindle::Id, bindle_connection_info: crate::bindle_util::BindleConnectionInfo) -> anyhow::Result<EmplacedHandlerConfiguration> { | |
| async fn emplace_remote_bindle(self, bindle_connection_info: crate::bindle_util::BindleConnectionInfo, id: &bindle::Id) -> anyhow::Result<EmplacedHandlerConfiguration> { |
…g trait Signed-off-by: Vaughn Dice <vaughn.dice@fermyon.com>
2 participants
Closes #154