Refuse to download OSTree targets with the fake/binary package manager. #1653
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1653 +/- ##
==========================================
+ Coverage 82.74% 82.76% +0.02%
==========================================
Files 191 191
Lines 12085 12089 +4
==========================================
+ Hits 10000 10006 +6
+ Misses 2085 2083 -2
Continue to review full report at Codecov.
|
pattivacek
force-pushed
the
fix/abort-ostree-with-fake-pacman
branch
2 times, most recently
from
9177703
to
c8459e7
Compare
| @@ -110,5 +110,10 @@ bool PackageManagerFake::fetchTarget(const Uptane::Target& target, Uptane::Fetch | |||
| return false; | |||
| } | |||
|
|
|||
| if (target.IsOstree()) { | |||
There was a problem hiding this comment.
I would probably prefer something like if (target.type_ != this->type_), but that should be also good for now.
There was a problem hiding this comment.
Me too, but for now we don't expose package manager types... unless I've forgotten something?
There was a problem hiding this comment.
No, we don't. I just was thinking that we might want to do something like this in the future,
There was a problem hiding this comment.
PackageManagerInterface::name() is kind of a package manager type :)
There was a problem hiding this comment.
@mike-sul True, but it still doesn't have 1:1 match with update type.
There was a problem hiding this comment.
Yeah, I know, maybe adding something like package_manager->isTargetSupported(const Target&) (or logicalEcu->isTargetSupported()) so the pack manager implementation decides whether it can handle the given target or not would be the best here.
src/libaktualizr/uptane/tuf.cc
Outdated
| @@ -135,6 +135,7 @@ Target::Target(std::string filename, EcuMap ecus, std::vector<Hash> hashes, uint | |||
| correlation_id_(std::move(correlation_id)) { | |||
| // sort hashes so that higher priority hash algorithm goes first | |||
| std::sort(hashes_.begin(), hashes_.end(), [](const Hash &l, const Hash &r) { return l.type() < r.type(); }); | |||
| type_ = "binary"; | |||
There was a problem hiding this comment.
Maybe, it is better to set to "unknown"? I also think that "BINARY" is capitalized, though you can never be sure. Should we use .toLower()?
There was a problem hiding this comment.
Maybe, it is better to set to "unknown"?
Agreed, done.
I also think that "BINARY" is capitalized, though you can never be sure. Should we use
.toLower()?
We don't appear to use "BINARY"; the only comparison we use for Target types is with "OSTREE". I think for now the comparison is fine as is.
Previously the download would succeed due to the empty length but the installation would fail. Better to fail as early as possible. Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
pattivacek
force-pushed
the
fix/abort-ostree-with-fake-pacman
branch
from
c8459e7
to
2fe1cb6
Compare
Previously the download would succeed due to the empty length but the installation would fail. Better to fail as early as possible.