Add image without actual file #1035

patriotyk · 2018-12-17T10:10:05Z

No description provided.

patriotyk · 2018-12-17T10:13:37Z

I don't understand two other tasks:
'setting director's targets to be empty.'
And
'setting staging (unsigned) director targets to be the same as current director targets.'

pattivacek · 2018-12-17T10:41:50Z

@OYTIS can probably explain better than I can!

OYTIS · 2018-12-17T10:57:24Z

@patriotyk

Current workflow of scheduling an update to director in aktualizr-repo is as following:

Add several targets to director with aktualizr-repo --command addtarget. The targets are added to the staged version of diretor/targets.json that is not accessible to remote clients.
Sign all the added targets scheduling and update aktualizr-repo --command signtargets

So there are two problems here:

We can't clear targets.json. The only way to create a new staging targets.json is to issue addtarget command, after which it will obviously be non-empty.
We're starting from scratch every time. It would be convenient to be able to take the current (signed) targets.json and use it as a base.

So I imagined two new commands:

emptytargets will make staging targets empty, if the user signs it immediately with signtargets, it will schedule an empty update (what our backend currently does when no update is scheduled).
oldtargets will populate staging targets with what is currently signed (stripping the signature of course). User can then use addtarget command to add new targets to it. I think we'll need deltarget as well, but that belongs to another task.

codecov-io · 2018-12-17T13:17:04Z

Codecov Report

Merging #1035 into master will increase coverage by 0.07%.
The diff coverage is 89.13%.

@@            Coverage Diff             @@
##           master    #1035      +/-   ##
==========================================
+ Coverage   82.26%   82.33%   +0.07%     
==========================================
  Files         196      196              
  Lines       13794    13928     +134     
==========================================
+ Hits        11347    11467     +120     
- Misses       2447     2461      +14

Impacted Files	Coverage Δ
src/aktualizr_repo/director_repo.h	`100% <ø> (ø)`	⬆️
src/aktualizr_repo/image_repo.h	`100% <ø> (ø)`	⬆️
src/aktualizr_repo/uptane_repo.h	`100% <ø> (ø)`	⬆️
src/aktualizr_repo/uptane_repo.cc	`95% <ø> (+1.66%)`	⬆️
src/aktualizr_repo/main.cc	`81.11% <85.71%> (+1.94%)`	⬆️
src/aktualizr_repo/repo_test.cc	`92.39% <89.02%> (-2.71%)`	⬇️
src/aktualizr_repo/image_repo.cc	`96.07% <89.47%> (-3.93%)`	⬇️
src/aktualizr_repo/director_repo.cc	`82.22% <93.75%> (+6.36%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1adff1...e79fa6f. Read the comment docs.

pattivacek

So far, looks good!

pattivacek · 2018-12-17T16:08:16Z

src/aktualizr_repo/main.cc

+    ("keytype", po::value<std::string>()->default_value("RSA2048"), "UPTANE key type")
+    ("targetname", po::value<std::string>(), "target's name")
+    ("targethash", po::value<std::string>(), "target's hash")
+    ("targetlength", po::value<uint64_t>(), "target's length");


These three new options should have some explanation here that they are intended for a special purpose. Something like "target name (for adding metadata without an actual file)".

pattivacek · 2018-12-17T16:09:31Z

src/aktualizr_repo/image_repo.cc

@@ -44,4 +50,17 @@ void ImageRepo::addImage(const boost::filesystem::path &image_path) {
  timestamp["meta"]["snapshot.json"]["version"] = snapshot["version"].asUInt();
  Utils::writeFile(repo_dir / "timestamp.json",
                   Utils::jsonToCanonicalStr(signTuf(Uptane::Role::Timestamp(), timestamp)));
+}
+
+void ImageRepo::addImage(const std::string &name, const std::string &hash, uint64_t length) {


Not big, but looks like length can be const.

patriotyk · 2018-12-17T21:47:28Z

@OYTIS Thanks, now I understand.

patriotyk · 2018-12-17T23:16:11Z

Looks like I have added all commands

pattivacek · 2018-12-18T11:20:51Z

@patriotyk if Travis is giving you trouble, rebase so that you get Laurent's fix to speed up the aktualizr-repo test.

patriotyk · 2018-12-19T12:00:56Z

Are there is something to fix?

lbonn · 2018-12-19T15:34:17Z

src/aktualizr_repo/image_repo.h

@@ -8,6 +8,10 @@ class ImageRepo : public Repo {
  ImageRepo(boost::filesystem::path path, const std::string &expires, std::string correlation_id)
      : Repo(Uptane::RepositoryType::Image(), std::move(path), expires, std::move(correlation_id)) {}
  void addImage(const boost::filesystem::path &image_path);


It feels like too much overloading of this addImage.

Could be helpful to give them different names.

lbonn · 2018-12-19T15:37:11Z

src/aktualizr_repo/image_repo.cc

+void ImageRepo::addImage(const std::string &name, const std::string &hash, const uint64_t length) {
+  Json::Value target;
+  target["length"] = Json::UInt(length);
+  if (hash.size() == 64) {


Not really elegant... Could we use Uptane::Hash and have too separate arguments on the commandline, like targetsha256 and targetsha512, or does it make cli handling too complicated?

lbonn · 2018-12-19T15:39:22Z

Looks fine to me, except for the two small comments.

If you make changes again, I can take a quick look on Friday.

Signed-off-by: Serhiy Stetskovych <[email protected]>

patriotyk force-pushed the feat/altualizr_repo_more_improvements branch from feea82f to ead765c Compare December 17, 2018 10:10

pattivacek reviewed Dec 17, 2018

View reviewed changes

patriotyk force-pushed the feat/altualizr_repo_more_improvements branch from ead765c to 6805d65 Compare December 17, 2018 23:15

patriotyk added the ready label Dec 18, 2018

lbonn reviewed Dec 19, 2018

View reviewed changes

patriotyk added 3 commits December 24, 2018 13:53

Add image without actual file

e88100c

Signed-off-by: Serhiy Stetskovych <[email protected]>

Implement emptytargets command

110fd04

Signed-off-by: Serhiy Stetskovych <[email protected]>

Implement oldtargets command

99b6dd9

Signed-off-by: Serhiy Stetskovych <[email protected]>

patriotyk force-pushed the feat/altualizr_repo_more_improvements branch 4 times, most recently from e892cb3 to b634fb5 Compare December 24, 2018 13:53

Use Uptane::Hash instead of std::string

ed5c434

Signed-off-by: Serhiy Stetskovych <[email protected]>

patriotyk force-pushed the feat/altualizr_repo_more_improvements branch from b634fb5 to f577ed0 Compare December 24, 2018 15:12

Rename addImage

e79fa6f

Signed-off-by: Serhiy Stetskovych <[email protected]>

patriotyk force-pushed the feat/altualizr_repo_more_improvements branch from f577ed0 to e79fa6f Compare December 26, 2018 10:26

lbonn approved these changes Jan 8, 2019

View reviewed changes

lbonn merged commit 45d8053 into master Jan 8, 2019

lbonn deleted the feat/altualizr_repo_more_improvements branch January 8, 2019 12:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add image without actual file #1035

Add image without actual file #1035

patriotyk commented Dec 17, 2018

patriotyk commented Dec 17, 2018 •

edited

Loading

pattivacek commented Dec 17, 2018

OYTIS commented Dec 17, 2018 •

edited

Loading

codecov-io commented Dec 17, 2018 •

edited

Loading

pattivacek left a comment

pattivacek Dec 17, 2018

pattivacek Dec 17, 2018

patriotyk commented Dec 17, 2018

patriotyk commented Dec 17, 2018

pattivacek commented Dec 18, 2018

patriotyk commented Dec 19, 2018

lbonn Dec 19, 2018

lbonn Dec 19, 2018

lbonn commented Dec 19, 2018

Add image without actual file #1035

Add image without actual file #1035

Conversation

patriotyk commented Dec 17, 2018

patriotyk commented Dec 17, 2018 • edited Loading

pattivacek commented Dec 17, 2018

OYTIS commented Dec 17, 2018 • edited Loading

codecov-io commented Dec 17, 2018 • edited Loading

Codecov Report

pattivacek left a comment

Choose a reason for hiding this comment

pattivacek Dec 17, 2018

Choose a reason for hiding this comment

pattivacek Dec 17, 2018

Choose a reason for hiding this comment

patriotyk commented Dec 17, 2018

patriotyk commented Dec 17, 2018

pattivacek commented Dec 18, 2018

patriotyk commented Dec 19, 2018

lbonn Dec 19, 2018

Choose a reason for hiding this comment

lbonn Dec 19, 2018

Choose a reason for hiding this comment

lbonn commented Dec 19, 2018

patriotyk commented Dec 17, 2018 •

edited

Loading

OYTIS commented Dec 17, 2018 •

edited

Loading

codecov-io commented Dec 17, 2018 •

edited

Loading