Dependabot updates to rust dependencies #2049
Conversation
| fn tls_server_roots_pem() -> Vec<Pem> { | ||
| match std::fs::read(&*CERT_PATH) { | ||
| Ok(data) => { | ||
| let mut v = pem::parse_many(&data); | ||
| let mut v = match pem::parse_many(&data) { | ||
| Ok(v) => v, | ||
| Err(err) => { | ||
| error!("failed to parse {}: {}", CERT_PATH.display(), err); | ||
| Vec::new() |
There was a problem hiding this comment.
We probably should modify this function to return a Result if we can, I'd prefer it fail rather than disregard invalid certs.
There was a problem hiding this comment.
At the moment we're already disregarding invalid certs further down in the tls_server_roots_pem function, so I'm a bit reluctant to change behavior in a dependency update commit. Should I open up an issue to re-evaluate how this handles errors?
There was a problem hiding this comment.
reluctant to change behavior in a dependency update commit
I think I agree with your assessment. Previously parse_many was swallowing errors, so to remain consistent we should swallow errors here. Otherwise, somebody could have a new Bottlerocket error that they didn't have before with an invalid cert.
Previous error swallowing:
| @@ -15,7 +15,7 @@ use std::collections::HashMap; | |||
| use std::num::NonZeroUsize; | |||
| use std::path::{Path, PathBuf}; | |||
| use std::{fs, process}; | |||
| use structopt::StructOpt; | |||
There was a problem hiding this comment.
Does it not build if we don't use clap?
We should probably remove structopt in favor of just clap going forward, though it could be done as a separate task. I haven't investigated what a migration looks like, but the feature set we're using should have something synonymous in clap.
Check out the structop docs here: https://docs.rs/structopt/latest/structopt/#maintenance
There was a problem hiding this comment.
From what I can tell we would want to switch to clap sooner rather than later. Because of the version bump, our tools don't build unless we prefix every clap with structopt::, or we add it to the use declaration.
There was a problem hiding this comment.
it could be done as a separate task
Yes, I think replacing StructOpt with clap should be done separately from regular library updates.
Does it not build if we don't
use clap
Seems like this change is fine... it makes it compile!
jpculp
force-pushed
the
dependabot-rollup
branch
from
a4e7e7e
to
dd90b92
Compare
|
Rebased on develop. |
|
Bump Note: |
|
Looks like this closes #2015? |
jpculp
force-pushed
the
dependabot-rollup
branch
from
7e8db4d
to
c1aedeb
Compare
|
jpculp
force-pushed
the
dependabot-rollup
branch
from
c1aedeb
to
25b2877
Compare
|
Fixed version in commit message. |
No, ignore me. The webpki version is not changing and neither is the webpki-roots version. |
Description of changes:
This is a collection of rust dependency updates brought on by dependabot.
Testing done:
apiclient exec.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.