Merge pull request #1305 from advancedtelematic/fix/ota-2183/enable-s…

…treaming-of-garage-tool-uploads

fix/ota-2183/Enable streaming of garage-push/deploy uploads

src/sota_tools/check.cc

@@ -23,7 +23,7 @@ static size_t writeString(void *contents, size_t size, size_t nmemb, void *userp
   return size * nmemb;
 }
 
-int CheckRefValid(const TreehubServer &treehub, const std::string &ref, RunMode mode, int max_curl_requests) {
+int CheckRefValid(TreehubServer &treehub, const std::string &ref, RunMode mode, int max_curl_requests) {
   // Check if the ref is present on treehub. The traditional use case is that it
   // should be a commit object, but we allow walking the tree given any OSTree
   // ref.

src/sota_tools/check.h

@@ -11,6 +11,6 @@
 /**
  *Check if the ref is present on the server and in targets.json
  */
-int CheckRefValid(const TreehubServer& treehub, const std::string& ref, RunMode mode, int max_curl_requests);
+int CheckRefValid(TreehubServer& treehub, const std::string& ref, RunMode mode, int max_curl_requests);
 
 #endif

src/sota_tools/garage_deploy.cc

@@ -115,18 +115,20 @@ int main(int argc, char **argv) {
     if (mode == RunMode::kDefault) {
       if (push_credentials.CanSignOffline()) {
         bool ok = OfflineSignRepo(ServerCredentials(push_credentials.GetPathOnDisk()), name, commit, hardwareids);
-        return static_cast<int>(!ok);
+        if (ok) {
+          if (CheckRefValid(fetch_server, ostree_commit, mode, max_curl_requests) != EXIT_SUCCESS) {
+            LOG_FATAL << "Check if the ref is present on the server or in targets.json failed";
+            return EXIT_FAILURE;
+          }
+        } else {
+          return EXIT_FAILURE;
+        }
       }
       LOG_FATAL << "Online signing with garage-deploy is currently unsupported";
       return EXIT_FAILURE;
     } else {
       LOG_INFO << "Dry run. Not attempting offline signing.";
     }
-
-    if (CheckRefValid(fetch_server, ostree_commit, mode, max_curl_requests) != EXIT_SUCCESS) {
-      LOG_FATAL << "Check if the ref is present on the server or in targets.json failed";
-      return EXIT_FAILURE;
-    }
   } catch (OSTreeCommitParseError &e) {
     LOG_FATAL << e.what();
     return EXIT_FAILURE;

src/sota_tools/ostree_http_repo.h

@@ -9,7 +9,7 @@
 
 class OSTreeHttpRepo : public OSTreeRepo {
  public:
-  explicit OSTreeHttpRepo(const TreehubServer* server) : server_(server) {}
+  explicit OSTreeHttpRepo(TreehubServer* server) : server_(server) {}
   ~OSTreeHttpRepo() override = default;
 
   bool LooksValid() const override;
@@ -20,7 +20,7 @@ class OSTreeHttpRepo : public OSTreeRepo {
   bool FetchObject(const boost::filesystem::path& path) const override;
   static size_t curl_handle_write(void* buffer, size_t size, size_t nmemb, void* userp);
 
-  const TreehubServer* server_;
+  TreehubServer* server_;
   const TemporaryDirectory root_;
 };
 

src/sota_tools/ostree_object.cc

@@ -2,6 +2,7 @@
 
 #include <assert.h>
 
+#include <sys/stat.h>
 #include <cstring>
 #include <iostream>
 
@@ -21,7 +22,7 @@ OSTreeObject::OSTreeObject(const OSTreeRepo &repo, const std::string &object_nam
       refcount_(0),
       is_on_server_(PresenceOnServer::kObjectStateUnknown),
       curl_handle_(nullptr),
-      form_post_(nullptr) {
+      fd_(nullptr) {
   if (!boost::filesystem::is_regular_file(file_path_)) {
     throw std::runtime_error(file_path_.native() + " is not a valid OSTree repo.");
   }
@@ -206,7 +207,7 @@ void OSTreeObject::MakeTestRequest(const TreehubServer &push_target, CURLM *curl
   request_start_time_ = std::chrono::steady_clock::now();
 }
 
-void OSTreeObject::Upload(const TreehubServer &push_target, CURLM *curl_multi_handle, const RunMode mode) {
+void OSTreeObject::Upload(TreehubServer &push_target, CURLM *curl_multi_handle, const RunMode mode) {
   if (mode == RunMode::kDefault || mode == RunMode::kPushTree) {
     LOG_INFO << "Uploading " << object_name_;
   } else {
@@ -222,25 +223,27 @@ void OSTreeObject::Upload(const TreehubServer &push_target, CURLM *curl_multi_ha
   }
   curlEasySetoptWrapper(curl_handle_, CURLOPT_VERBOSE, get_curlopt_verbose());
   current_operation_ = CurrentOp::kOstreeObjectUploading;
+  push_target.SetContentType("Content-Type: application/octet-stream");
   push_target.InjectIntoCurl(Url(), curl_handle_);
   curlEasySetoptWrapper(curl_handle_, CURLOPT_USERAGENT, Utils::getUserAgent());
   curlEasySetoptWrapper(curl_handle_, CURLOPT_WRITEFUNCTION, &OSTreeObject::curl_handle_write);
   curlEasySetoptWrapper(curl_handle_, CURLOPT_WRITEDATA, this);
   http_response_.str("");  // Empty the response buffer
 
-  assert(form_post_ == nullptr);
-  struct curl_httppost *last_item = nullptr;
-  const CURLFORMcode form_err =
-      curl_formadd(&form_post_, &last_item, CURLFORM_COPYNAME, "file", CURLFORM_FILE, file_path_.c_str(), CURLFORM_END);
-  if (form_err != 0u) {
-    // Apparently there is not strerror for formadd.
-    LOG_ERROR << "curl_formadd error: " << form_err;
+  struct stat file_info {};
+  fd_ = fopen(file_path_.c_str(), "rb");
+  if (fd_ == nullptr) {
+    throw std::runtime_error("could not open file to be uploaded");
+  } else {
+    if (stat(file_path_.c_str(), &file_info) < 0) {
+      throw std::runtime_error("Could not get file information");
+    }
   }
+  curlEasySetoptWrapper(curl_handle_, CURLOPT_READDATA, fd_);
+  curlEasySetoptWrapper(curl_handle_, CURLOPT_POSTFIELDSIZE, file_info.st_size);
   curlEasySetoptWrapper(curl_handle_, CURLOPT_POST, 1);
-  curlEasySetoptWrapper(curl_handle_, CURLOPT_HTTPPOST, form_post_);
 
   curlEasySetoptWrapper(curl_handle_, CURLOPT_PRIVATE, this);  // Used by ostree_object_from_curl
-
   const CURLMcode err = curl_multi_add_handle(curl_multi_handle, curl_handle_);
   if (err != 0) {
     LOG_ERROR << "curl_multi_add_handle error:" << curl_multi_strerror(err);
@@ -316,13 +319,11 @@ void OSTreeObject::CurlDone(CURLM *curl_multi_handle, RequestPool &pool) {
     }
 
   } else if (current_operation_ == CurrentOp::kOstreeObjectUploading) {
-    curl_formfree(form_post_);
-    form_post_ = nullptr;
     // Sanity-check the handle's URL to make sure it contains the expected
     // object hash.
     if (url == nullptr || strstr(url, object_name_.c_str()) == nullptr) {
       UploadError(pool, rescode);
-    } else if (rescode == 200) {
+    } else if (rescode == 204) {
       LOG_TRACE << "OSTree upload successful";
       is_on_server_ = PresenceOnServer::kObjectPresent;
       last_operation_result_ = ServerResponse::kOk;
@@ -335,6 +336,7 @@ void OSTreeObject::CurlDone(CURLM *curl_multi_handle, RequestPool &pool) {
     } else {
       UploadError(pool, rescode);
     }
+    fclose(fd_);
   } else {
     LOG_ERROR << "Unknown operation: " << static_cast<int>(current_operation_);
     assert(0);

src/sota_tools/ostree_object.h

@@ -45,7 +45,7 @@ class OSTreeObject {
   void MakeTestRequest(const TreehubServer& push_target, CURLM* curl_multi_handle);
 
   /* Upload this object to the destination server. */
-  void Upload(const TreehubServer& push_target, CURLM* curl_multi_handle, RunMode mode);
+  void Upload(TreehubServer& push_target, CURLM* curl_multi_handle, RunMode mode);
 
   /* Process a completed curl transaction (presence check or upload). */
   void CurlDone(CURLM* curl_multi_handle, RequestPool& pool);
@@ -110,7 +110,7 @@ class OSTreeObject {
 
   std::stringstream http_response_;
   CURL* curl_handle_;
-  struct curl_httppost* form_post_;
+  FILE* fd_;
   std::list<parentref> parents_;
   std::list<OSTreeObject::ptr> children_;
 

src/sota_tools/ostree_object_test.cc

@@ -108,8 +108,6 @@ TEST(OstreeObject, UploadDryRun) {
   EXPECT_EQ(object->is_on_server_, PresenceOnServer::kObjectPresent);
   // This currently does not get reset.
   EXPECT_EQ(object->current_operation_, CurrentOp::kOstreeObjectPresenceCheck);
-  // This should not get allocated.
-  EXPECT_EQ(object->form_post_, nullptr);
 }
 
 /* Detect curl misconfiguration.
@@ -128,10 +126,6 @@ TEST(OstreeObject, UploadFail) {
   object->Upload(push_server, nullptr, RunMode::kDefault);
   EXPECT_EQ(object->is_on_server_, PresenceOnServer::kObjectStateUnknown);
   EXPECT_EQ(object->current_operation_, CurrentOp::kOstreeObjectUploading);
-  // This currently will get allocated and we need to free it.
-  EXPECT_NE(object->form_post_, nullptr);
-  curl_formfree(object->form_post_);
-  object->form_post_ = nullptr;
 }
 
 /* Upload missing OSTree objects to destination repository. */
@@ -182,10 +176,8 @@ TEST(OstreeObject, UploadSuccess) {
       EXPECT_GE(h->refcount_, 1);
       long rescode = 0;  // NOLINT(google-runtime-int)
       curl_easy_getinfo(h->curl_handle_, CURLINFO_RESPONSE_CODE, &rescode);
-      EXPECT_EQ(rescode, 200);
+      EXPECT_EQ(rescode, 204);
 
-      curl_formfree(h->form_post_);
-      h->form_post_ = nullptr;
       curl_multi_remove_handle(multi, h->curl_handle_);
       curl_easy_cleanup(h->curl_handle_);
       h->curl_handle_ = nullptr;

src/sota_tools/request_pool.cc

@@ -7,7 +7,7 @@
 
 #include "logging/logging.h"
 
-RequestPool::RequestPool(const TreehubServer& server, const int max_curl_requests, const RunMode mode)
+RequestPool::RequestPool(TreehubServer& server, const int max_curl_requests, const RunMode mode)
     : rate_controller_(max_curl_requests), running_requests_(0), server_(server), mode_(mode), stopped_(false) {
   curl_global_init(CURL_GLOBAL_DEFAULT);
   multi_ = curl_multi_init();

src/sota_tools/request_pool.h

@@ -11,7 +11,7 @@
 
 class RequestPool {
  public:
-  RequestPool(const TreehubServer& server, int max_curl_requests, RunMode mode);
+  RequestPool(TreehubServer& server, int max_curl_requests, RunMode mode);
   ~RequestPool();
   void AddQuery(const OSTreeObject::ptr& request);
   void AddUpload(const OSTreeObject::ptr& request);
@@ -42,7 +42,7 @@ class RequestPool {
   RateController rate_controller_;
   int running_requests_;
   int total_requests_made_{0};
-  const TreehubServer& server_;
+  TreehubServer& server_;
   CURLM* multi_;
   std::list<OSTreeObject::ptr> query_queue_;
   std::list<OSTreeObject::ptr> upload_queue_;

src/sota_tools/treehub_server.cc

@@ -13,18 +13,30 @@ TreehubServer::TreehubServer() {
   auth_header_.next = &force_header_;
   force_header_contents_ = "x-ats-ostree-force: true";
   force_header_.data = const_cast<char*>(force_header_contents_.c_str());
-  force_header_.next = nullptr;
+  force_header_.next = &content_type_header_;
+  content_type_header_.data = const_cast<char*>(content_type_header_contents_.c_str());
+  content_type_header_.next = nullptr;
 }
 
 void TreehubServer::SetToken(const string& token) {
   assert(auth_header_.next == &force_header_);
-  assert(force_header_.next == nullptr);
+  assert(force_header_.next == &content_type_header_);
+  assert(content_type_header_.next == nullptr);
 
   auth_header_contents_ = "Authorization: Bearer " + token;
   auth_header_.data = const_cast<char*>(auth_header_contents_.c_str());
   method_ = AuthMethod::kOauth2;
 }
 
+void TreehubServer::SetContentType(const string& content_type) {
+  assert(auth_header_.next == &force_header_);
+  assert(force_header_.next == &content_type_header_);
+  assert(content_type_header_.next == nullptr);
+
+  content_type_header_contents_ = content_type;
+  content_type_header_.data = const_cast<char*>(content_type_header_contents_.c_str());
+}
+
 void TreehubServer::SetCerts(const std::string& client_p12) {
   method_ = AuthMethod::kTls;
   client_p12_path_.PutContents(client_p12);

src/sota_tools/treehub_server.h

@@ -12,6 +12,7 @@ class TreehubServer {
  public:
   TreehubServer();
   void SetToken(const std::string &token);
+  void SetContentType(const std::string &content_type);
   void SetCerts(const std::string &client_p12);
   void SetAuthBasic(const std::string &username, const std::string &password);
 
@@ -39,6 +40,10 @@ class TreehubServer {
   // Don't modify force_header_contents_ without updating the pointer in
   // force_header_
   std::string force_header_contents_;
+  struct curl_slist content_type_header_ {};
+  // Don't modify content_type_header_contents_ without updating the pointer in
+  // content_type_header_
+  std::string content_type_header_contents_;
 };
 
 // vim: set tabstop=2 shiftwidth=2 expandtab:

tests/sota_tools/test-server-500

@@ -32,7 +32,7 @@ class OstreeRepo(object):
         if self.upload_count % 2 == 0:
             self.did_return_500.set()
             return 500
-        return 200
+        return 204
 
     def query(self, name):
         self.query_count += 1

tests/sota_tools/test-server-500_after_20

@@ -19,7 +19,7 @@ class OstreeRepo(object):
         self.upload_count += 1
         if self.upload_count >= 20:
             return 500
-        return 200
+        return 204
 
     def query(self, name):
         return 404

tests/sota_tools/test-server-error_every_10

@@ -19,7 +19,7 @@ class OstreeRepo(object):
         self.upload_count += 1
         if self.upload_count % 10 == 0:
             return status_code
-        return 200
+        return 204
 
     def query(self, name):
         return 404

tests/sota_tools/treehub_server.py

@@ -79,6 +79,16 @@ def do_POST(self):
             self.send_response_only(200)
             self.end_headers()
             return
+        elif ctype == 'application/octet-stream':
+            length = int(self.headers['content-length'])
+            body = self.rfile.read(length)
+            full_path = os.path.join(repo_path, self.path[1:])
+            os.system("mkdir -p %s" % os.path.dirname(full_path))
+            with open(full_path, "wb") as f:
+                f.write(body)
+            self.send_response_only(204)
+            self.end_headers()
+            return
 
         self.send_response_only(400)
         self.end_headers()