repo: Add add-remotes-config-dir option

This option allows a repo to explicitly opt out of adding new remotes in
a remotes configuration directory. This currently defaults to true for
system repos and false for non-system repos to maintain legacy behavior
that non-system repos don't add remotes in a configuration directory.
That would be problematic for flatpak, which specifies a remotes config
dir but adds remotes in ways that are incompatible with it.

So, what this really does is allow system repos to control whether they
want to add remotes in the config dir or not. That's important if your
flatpak repo is the system repo like at Endless.

Closes: #1134

Closes: #1155
Approved by: cgwalters

5 files changed, 63 insertions, 6 deletions

diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml
index 8ae6ad42..388b9e62 100644
--- a/man/ostree.repo-config.xml
+++ b/man/ostree.repo-config.xml
@@ -122,6 +122,27 @@ Boston, MA 02111-1307, USA.
         keep free. The default value is 3.</para></listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><varname>add-remotes-config-dir</varname></term>
+        <listitem>
+          <para>
+            Boolean value controlling whether new remotes will be added
+            in the remotes configuration directory. Defaults to
+            <literal>true</literal> for system ostree repositories. When
+            this is <literal>false</literal>, remotes will be added in
+            the repository's <filename>config</filename> file.
+          </para>
+          <para>
+            This only applies to repositories that use a remotes
+            configuration directory such as system ostree repositories,
+            which use <filename>/etc/ostree/remotes.d</filename>.
+            Non-system repositories do not use a remotes configuration
+            directory unless one is specified when the repository is
+            opened.
+          </para>
+        </listitem>
+      </varlistentry>
+
     </variablelist>
   </refsect1>
 
diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h
index fdcd8f3f..865cb120 100644
--- a/src/libostree/ostree-repo-private.h
+++ b/src/libostree/ostree-repo-private.h
@@ -154,6 +154,7 @@ struct OstreeRepo {
   gboolean generate_sizes;
   guint64 tmp_expiry_seconds;
   gchar *collection_id;
+  gboolean add_remotes_config_dir; /* Add new remotes in remotes.d dir */
 
   OstreeRepo *parent_repo;
 };
diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index c0d4a8e7..b529854e 100644
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -1010,11 +1010,12 @@ impl_repo_remote_add (OstreeRepo     *self,
 
   remote = ostree_remote_new (name);
 
-  /* Only add repos in remotes.d for system repos since that was the
-   * legacy behavior and non-system repos would not expect it.
+  /* Only add repos in remotes.d if the repo option
+   * add-remotes-config-dir is true. This is the default for system
+   * repos.
    */
   g_autoptr(GFile) etc_ostree_remotes_d = get_remotes_d_dir (self, sysroot);
-  if (etc_ostree_remotes_d && ostree_repo_is_system (self))
+  if (etc_ostree_remotes_d && self->add_remotes_config_dir)
     {
       g_autoptr(GError) local_error = NULL;
 
@@ -2231,6 +2232,17 @@ reload_core_config (OstreeRepo          *self,
         }
     }
 
+  /* By default, only add remotes in a remotes config directory for
+   * system repos. This is to preserve legacy behavior for non-system
+   * repos that specify a remotes config dir (flatpak).
+   */
+  { gboolean is_system = ostree_repo_is_system (self);
+
+    if (!ot_keyfile_get_boolean_with_default (self->config, "core", "add-remotes-config-dir",
+                                              is_system, &self->add_remotes_config_dir, error))
+      return FALSE;
+  }
+
   return TRUE;
 }
 
diff --git a/tests/admin-test.sh b/tests/admin-test.sh
index 805a7130..f5d33a6f 100644
--- a/tests/admin-test.sh
+++ b/tests/admin-test.sh
@@ -19,7 +19,7 @@
 
 set -euo pipefail
 
-echo "1..$((21 + ${extra_admin_tests:-0}))"
+echo "1..$((22 + ${extra_admin_tests:-0}))"
 
 function validate_bootloader() {
     cd ${test_tmpdir};
@@ -292,6 +292,15 @@ assert_not_file_has_content sysroot/ostree/repo/config remote-test-nonphysical
 assert_file_has_content ${deployment}/etc/ostree/remotes.d/remote-test-nonphysical.conf testos-repo
 echo "ok remote add nonphysical sysroot"
 
+# Test that setting add-remotes-config-dir to false adds a remote in the
+# config file rather than the remotes config dir even though this is a
+# "system" repo.
+${CMD_PREFIX} ostree --repo=sysroot/ostree/repo config set core.add-remotes-config-dir false
+${CMD_PREFIX} ostree --sysroot=${deployment} remote add --set=gpg-verify=false remote-test-config-dir file://$(pwd)/testos-repo
+assert_not_has_file ${deployment}/etc/ostree/remotes.d/remote-test-config-dir.conf testos-repo
+assert_file_has_content sysroot/ostree/repo/config remote-test-config-dir
+echo "ok remote add nonphysical sysroot add-remotes-config-dir false"
+
 if env OSTREE_SYSROOT_DEBUG="${OSTREE_SYSROOT_DEBUG},test-fifreeze" \
        ${CMD_PREFIX} ostree admin deploy --os=testos testos:testos/buildmaster/x86_64-runtime 2>err.txt; then
     fatal "fifreeze-test exited successfully?"
diff --git a/tests/test-remotes-config-dir.js b/tests/test-remotes-config-dir.js
index 9de8f01a..11d77348 100755
--- a/tests/test-remotes-config-dir.js
+++ b/tests/test-remotes-config-dir.js
@@ -32,7 +32,7 @@ function assertNotEquals(a, b) {
 	throw new Error("assertion failed " + JSON.stringify(a) + " != " + JSON.stringify(b));
 }
 
-print('1..3')
+print('1..4')
 
 let remotesDir = Gio.File.new_for_path('remotes.d');
 remotesDir.make_directory(null);
@@ -56,7 +56,8 @@ assertNotEquals(remotes.indexOf('foo'), -1);
 
 print("ok read-remotes-config-dir");
 
-// Adding a remote should not go in the remotes.d dir
+// Adding a remote should not go in the remotes.d dir unless this is a
+// system repo or add-remotes-config-dir is set to true
 repo.remote_add('bar', 'http://bar', null, null);
 remotes = repo.remote_list()
 assertNotEquals(remotes.indexOf('bar'), -1);
@@ -71,3 +72,16 @@ assertEquals(remotes.indexOf('foo'), -1);
 assertEquals(remotesDir.get_child('foo.conf').query_exists(null), false);
 
 print("ok delete-in-remotes-config-dir");
+
+// Set add-remotes-config-dir to true and check that a remote gets added
+// in the config dir
+let repoConfig = repo.copy_config();
+repoConfig.set_boolean('core', 'add-remotes-config-dir', true);
+repo.write_config(repoConfig);
+repo.reload_config(null);
+repo.remote_add('baz', 'http://baz', null, null);
+remotes = repo.remote_list()
+assertNotEquals(remotes.indexOf('baz'), -1);
+assertEquals(remotesDir.get_child('baz.conf').query_exists(null), true);
+
+print("ok add-in-remotes-config-dir");