lib/pull: Add `timestamp-check-from-rev`
authorJonathan Lebon <jonathan@jlebon.com>
Thu, 14 May 2020 17:44:32 +0000 (13:44 -0400)
committerJonathan Lebon <jonathan@jlebon.com>
Thu, 14 May 2020 18:00:42 +0000 (14:00 -0400)
The way `timestamp-check` works might be too restrictive in some
situations. Essentially, we need to support the case where users want to
pull an older commit than the current tip, but while still guaranteeing
that it is newer than some even older commit.

This will be used in Fedora CoreOS. For more information see:
https://github.com/coreos/rpm-ostree/pull/2094
https://github.com/coreos/fedora-coreos-tracker/issues/481

src/libostree/ostree-repo-pull-private.h
src/libostree/ostree-repo-pull.c
src/ostree/ot-builtin-pull.c
tests/pull-test.sh

index 5bc2a33a55f5f30d25bb87c6dd18e38f76b0be26..86d1ffeeea235b27bd67b30390b7d5fc32cefa4a 100644 (file)
@@ -114,6 +114,7 @@ typedef struct {
   guint             n_imported_content;
 
   gboolean          timestamp_check; /* Verify commit timestamps */
+  char             *timestamp_check_from_rev;
   int               maxdepth;
   guint64           max_metadata_size;
   guint64           start_time;
index 363db9e2285338e68c0f51d6c6baa7d1f015884c..291f3fe679b905f712c8e82af863b299a8491b8d 100644 (file)
@@ -1593,6 +1593,7 @@ scan_commit_object (OtPullData                 *pull_data,
                                      commit, error))
     return glnx_prefix_error (error, "Commit %s", checksum);
 
+  guint64 new_ts = ostree_commit_get_timestamp (commit);
   if (pull_data->timestamp_check)
     {
       /* We don't support timestamp checking while recursing right now */
@@ -1611,11 +1612,22 @@ scan_commit_object (OtPullData                 *pull_data,
             return glnx_prefix_error (error, "Reading %s for timestamp-check", ref->ref_name);
 
           guint64 orig_ts = ostree_commit_get_timestamp (orig_commit);
-          guint64 new_ts = ostree_commit_get_timestamp (commit);
           if (!_ostree_compare_timestamps (orig_rev, orig_ts, checksum, new_ts, error))
             return FALSE;
         }
     }
+  if (pull_data->timestamp_check_from_rev)
+    {
+      g_autoptr(GVariant) commit = NULL;
+      if (!ostree_repo_load_commit (pull_data->repo, pull_data->timestamp_check_from_rev,
+                                    &commit, NULL, error))
+        return glnx_prefix_error (error, "Reading %s for timestamp-check-from-rev",
+                                  pull_data->timestamp_check_from_rev);
+
+      guint64 ts = ostree_commit_get_timestamp (commit);
+      if (!_ostree_compare_timestamps (pull_data->timestamp_check_from_rev, ts, checksum, new_ts, error))
+        return FALSE;
+    }
 
   /* If we found a legacy transaction flag, assume all commits are partial */
   gboolean is_partial = commitstate_is_partial (pull_data, commitstate);
@@ -3270,6 +3282,7 @@ initiate_request (OtPullData                 *pull_data,
  *   * require-static-deltas (b): Require static deltas
  *   * override-commit-ids (as): Array of specific commit IDs to fetch for refs
  *   * timestamp-check (b): Verify commit timestamps are newer than current (when pulling via ref); Since: 2017.11
+ *   * timestamp-check-from-rev (s): Verify that all fetched commit timestamps are newer than timestamp of given rev; Since: 2020.4
  *   * metadata-size-restriction (t): Restrict metadata objects to a maximum number of bytes; 0 to disable.  Since: 2018.9
  *   * dry-run (b): Only print information on what will be downloaded (requires static deltas)
  *   * override-url (s): Fetch objects from this URL if remote specifies no metalink in options
@@ -3372,6 +3385,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
       (void) g_variant_lookup (options, "update-frequency", "u", &update_frequency);
       (void) g_variant_lookup (options, "localcache-repos", "^a&s", &opt_localcache_repos);
       (void) g_variant_lookup (options, "timestamp-check", "b", &pull_data->timestamp_check);
+      (void) g_variant_lookup (options, "timestamp-check-from-rev", "s", &pull_data->timestamp_check_from_rev);
       (void) g_variant_lookup (options, "max-metadata-size", "t", &pull_data->max_metadata_size);
       (void) g_variant_lookup (options, "append-user-agent", "s", &pull_data->append_user_agent);
       opt_n_network_retries_set =
@@ -4259,6 +4273,18 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
                             g_steal_pointer (&checksum));
     }
 
+  /* Resolve refs to a checksum if necessary */
+  if (pull_data->timestamp_check_from_rev &&
+      !ostree_validate_checksum_string (pull_data->timestamp_check_from_rev, NULL))
+    {
+      g_autofree char *from_rev = NULL;
+      if (!ostree_repo_resolve_rev (pull_data->repo, pull_data->timestamp_check_from_rev, FALSE,
+                                    &from_rev, error))
+        goto out;
+      g_free (pull_data->timestamp_check_from_rev);
+      pull_data->timestamp_check_from_rev = g_steal_pointer (&from_rev);
+    }
+
   g_hash_table_unref (requested_refs_to_fetch);
   requested_refs_to_fetch = g_steal_pointer (&updated_requested_refs_to_fetch);
   if (g_hash_table_size (requested_refs_to_fetch) == 1)
@@ -4604,6 +4630,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   g_clear_pointer (&pull_data->fetched_detached_metadata, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->summary_deltas_checksums, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->ref_original_commits, (GDestroyNotify) g_hash_table_unref);
+  g_free (pull_data->timestamp_check_from_rev);
   g_clear_pointer (&pull_data->verified_commits, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->ref_keyring_map, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->requested_content, (GDestroyNotify) g_hash_table_unref);
index 1fae0a38da99d2af564232c5bfbf2c7bfb61f626..1625ab4746633d4be303851e2204da705b6a935c 100644 (file)
@@ -37,6 +37,7 @@ static gboolean opt_require_static_deltas;
 static gboolean opt_untrusted;
 static gboolean opt_http_trusted;
 static gboolean opt_timestamp_check;
+static char* opt_timestamp_check_from_rev;
 static gboolean opt_bareuseronly_files;
 static char** opt_subpaths;
 static char** opt_http_headers;
@@ -72,6 +73,7 @@ static GOptionEntry options[] = {
    { "network-retries", 0, 0, G_OPTION_ARG_INT, &opt_network_retries, "Specifies how many times each download should be retried upon error (default: 5)", "N"},
    { "localcache-repo", 'L', 0, G_OPTION_ARG_FILENAME_ARRAY, &opt_localcache_repos, "Add REPO as local cache source for objects during this pull", "REPO" },
    { "timestamp-check", 'T', 0, G_OPTION_ARG_NONE, &opt_timestamp_check, "Require fetched commits to have newer timestamps", NULL },
+   { "timestamp-check-from-rev", 0, 0, G_OPTION_ARG_STRING, &opt_timestamp_check_from_rev, "Require fetched commits to have newer timestamps than given rev", NULL },
    /* let's leave this hidden for now; we just need it for tests */
    { "append-user-agent", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, &opt_append_user_agent, "Append string to user agent", NULL },
    { NULL }
@@ -313,6 +315,9 @@ ostree_builtin_pull (int argc, char **argv, OstreeCommandInvocation *invocation,
     if (opt_timestamp_check)
       g_variant_builder_add (&builder, "{s@v}", "timestamp-check",
                              g_variant_new_variant (g_variant_new_boolean (opt_timestamp_check)));
+    if (opt_timestamp_check_from_rev)
+      g_variant_builder_add (&builder, "{s@v}", "timestamp-check-from-rev",
+                             g_variant_new_variant (g_variant_new_string (opt_timestamp_check_from_rev)));
 
     if (override_commit_ids)
       g_variant_builder_add (&builder, "{s@v}", "override-commit-ids",
index a52adab9712f88ae310e636563342e4e38b7cd17..92bcf112c6f8647241e375cd49269dcb85fd813f 100644 (file)
@@ -318,7 +318,7 @@ ${CMD_PREFIX} ostree --repo=parentpullrepo rev-parse origin:main > main.txt
 assert_file_has_content main.txt ${rev}
 echo "ok pull specific commit"
 
-# test pull -T
+# test pull -T and --timestamp-check-from-rev
 cd ${test_tmpdir}
 repo_init --no-sign-verify
 ${CMD_PREFIX} ostree --repo=repo pull origin main
@@ -347,6 +347,28 @@ assert_file_has_content err.txt "Upgrade.*is chronologically older"
 assert_streq ${newrev} "$(${CMD_PREFIX} ostree --repo=repo rev-parse main)"
 # But we can pull it without timestamp checking
 ${CMD_PREFIX} ostree --repo=repo pull origin main
+# Now test --timestamp-check-from-rev. First, add two new commits with distinct
+# but newer timestamps.
+oldrev=${newrev2}
+middlerev=$(${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo commit ${COMMIT_ARGS} -b main --tree=ref=main)
+sleep 1
+latestrev=$(${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo commit ${COMMIT_ARGS} -b main --tree=ref=main)
+${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo summary -u
+# OK, let's pull the latest now.
+${CMD_PREFIX} ostree --repo=repo pull -T origin main
+assert_streq ${latestrev} "$(${CMD_PREFIX} ostree --repo=repo rev-parse main)"
+# Check we can't pull the middle commit by overrides with ts checking on
+if ${CMD_PREFIX} ostree --repo=repo pull -T origin main@${middlerev} 2>err.txt; then
+    fatal "pulled older commit override with timestamp checking enabled?"
+fi
+assert_file_has_content err.txt "Upgrade.*is chronologically older"
+# Check we can't pull an older commit by override if it's newer than --timestamp-check-from-rev
+if ${CMD_PREFIX} ostree --repo=repo pull --timestamp-check-from-rev=${latestrev} origin main@${middlerev} 2>err.txt; then
+    fatal "pulled older commit override with timestamp checking enabled?"
+fi
+assert_file_has_content err.txt "Upgrade.*is chronologically older"
+# But we can pull it with --timestamp-check-from-rev when starting from the oldrev
+${CMD_PREFIX} ostree --repo=repo pull --timestamp-check-from-rev=${oldrev} origin main@${middlerev}
 echo "ok pull timestamp checking"
 
 cd ${test_tmpdir}