Fill out missing file source management tests.

lib/galaxy/files/templates/examples/testing_multi_version_with_secrets.yml

@@ -0,0 +1,34 @@
+# a template with multiple versions that could be coherent for unit/integration
+# testing upgrading object stores templates
+- id: secure_disk
+  version: 0
+  name: Secure Disk
+  description: Secure Disk Bound to You
+  secrets:
+    sec1:
+      help: This is my test secret.
+  configuration:
+    type: posix
+    root: '/data/secure/{{ user.username }}/{{ secrets.sec1 }}/aftersec'
+- id: secure_disk
+  version: 1
+  name: Secure Disk
+  description: Secure Disk Bound to You
+  secrets:
+    sec1:
+      help: This is my test secret.
+    sec2:
+      help: This is my test secret 2.
+  configuration:
+    type: posix
+    root: '/data/secure/{{ user.username }}/{{ secrets.sec1 }}/{{ secrets.sec2 }}'
+- id: secure_disk
+  version: 2
+  name: Secure Disk
+  description: Secure Disk Bound to You
+  secrets:
+    sec2:
+      help: This is my test secret 2.
+  configuration:
+    type: posix
+    root: '/data/secure/{{ user.username }}/newbar/{{ secrets.sec2 }}'

test/unit/app/managers/test_user_file_sources.py

@@ -3,15 +3,25 @@ from typing import (
     cast,
     List,
     Optional,
+    Type,
 )
 
+from yaml import safe_load
+
+from galaxy.exceptions import (
+    RequestParameterInvalidException,
+    RequestParameterMissingException,
+)
 from galaxy.files import FileSourcesUserContext
 from galaxy.files.templates import ConfiguredFileSourceTemplates
+from galaxy.files.templates.examples import get_example
 from galaxy.managers.file_source_instances import (
     CreateInstancePayload,
     FileSourceInstancesManager,
+    ModifyInstancePayload,
     UpdateInstancePayload,
     UpdateInstanceSecretPayload,
+    UpgradeInstancePayload,
     USER_FILE_SOURCES_SCHEME,
     UserDefinedFileSourcesConfig,
     UserDefinedFileSourcesImpl,
@@ -165,6 +175,16 @@ SIMPLE_VAULT_CREATE_PAYLOAD = CreateInstancePayload(
     variables={},
     secrets={"sec1": "foosec"},
 )
+UPGRADE_EXAMPLE = get_example("testing_multi_version_with_secrets.yml")
+UPGRADE_INITIAL_PAYLOAD = CreateInstancePayload(
+    name="My Upgradable Disk",
+    template_id="secure_disk",
+    template_version=0,
+    secrets={
+        "sec1": "moocow",
+    },
+    variables={},
+)
 
 
 class TestFileSourcesTestCase(BaseTestCase):
@@ -206,14 +226,15 @@ class TestFileSourcesTestCase(BaseTestCase):
         user_file_source = self._create_user_file_source()
         assert user_file_source.uri_root in self._uri_roots()
         hide = UpdateInstancePayload(hidden=True)
-        self.manager.modify_instance(self.trans, user_file_source.uuid, hide)
+        self._modify(user_file_source, hide)
+        assert user_file_source.uri_root not in self._uri_roots()
 
     def test_find_best_match_not_filters_hidden(self, tmp_path):
         self.init_user_in_database()
         self._init_managers(tmp_path)
         user_file_source = self._create_user_file_source()
         hide = UpdateInstancePayload(hidden=True)
-        self.manager.modify_instance(self.trans, user_file_source.uuid, hide)
+        self._modify(user_file_source, hide)
         match = self.file_sources.find_best_match(user_file_source.uri_root)
         assert match
 
@@ -221,8 +242,8 @@ class TestFileSourcesTestCase(BaseTestCase):
         self.init_user_in_database()
         self._init_managers(tmp_path)
         user_file_source = self._create_user_file_source()
-        hide = UpdateInstancePayload(active=False)
-        self.manager.modify_instance(self.trans, user_file_source.uuid, hide)
+        deactivate = UpdateInstancePayload(active=False)
+        self._modify(user_file_source, deactivate)
         match = self.file_sources.find_best_match(user_file_source.uri_root)
         assert not match
 
@@ -301,7 +322,7 @@ class TestFileSourcesTestCase(BaseTestCase):
         )
 
         # assert response includes updated variable as well as a fresh show()
-        response = self.manager.modify_instance(self.trans, user_file_source.uuid, update)
+        response = self._modify(user_file_source, update)
         assert response.variables
         assert response.variables["var1"] == "newval"
 
@@ -316,7 +337,7 @@ class TestFileSourcesTestCase(BaseTestCase):
         assert not user_file_source_showed.hidden
 
         hide = UpdateInstancePayload(hidden=True)
-        self.manager.modify_instance(self.trans, user_file_source.uuid, hide)
+        self._modify(user_file_source, hide)
 
         user_file_source_showed = self.manager.show(self.trans, user_file_source.uuid)
         assert user_file_source_showed.hidden
@@ -330,7 +351,7 @@ class TestFileSourcesTestCase(BaseTestCase):
         assert not user_file_source_showed.purged
 
         deactivate = UpdateInstancePayload(active=False)
-        self.manager.modify_instance(self.trans, user_file_source.uuid, deactivate)
+        self._modify(user_file_source, deactivate)
 
         user_file_source_showed = self.manager.show(self.trans, user_file_source.uuid)
         assert user_file_source_showed.hidden
@@ -350,9 +371,88 @@ class TestFileSourcesTestCase(BaseTestCase):
         user_file_source = self._create_instance(SIMPLE_VAULT_CREATE_PAYLOAD)
         self._assert_secret_is(user_file_source, "sec1", "foosec")
         update = UpdateInstanceSecretPayload(secret_name="sec1", secret_value="newvalue")
-        self.manager.modify_instance(self.trans, user_file_source.uuid, update)
+        self._modify(user_file_source, update)
         self._assert_secret_is(user_file_source, "sec1", "newvalue")
 
+    def test_cannot_update_invalid_secret(self, tmp_path):
+        self._init_managers(tmp_path, simple_vault_template(tmp_path))
+        user_file_source = self._create_instance(SIMPLE_VAULT_CREATE_PAYLOAD)
+        update = UpdateInstanceSecretPayload(secret_name="undefinedsec", secret_value="newvalue")
+        self._assert_modify_throws_exception(user_file_source, update, RequestParameterInvalidException)
+
+    def test_upgrade(self, tmp_path):
+        user_file_source = self._init_upgrade_test_case(tmp_path)
+        assert "sec1" in user_file_source.secrets
+        assert "sec2" not in user_file_source.secrets
+        self._assert_secret_is(user_file_source, "sec1", "moocow")
+        self._assert_secret_absent(user_file_source, "foobarxyz")
+        self._assert_secret_absent(user_file_source, "sec2")
+        upgrade_to_1 = UpgradeInstancePayload(
+            template_version=1,
+            secrets={
+                "sec1": "moocow",
+                "sec2": "aftersec2",
+            },
+            variables={},
+        )
+        user_file_source = self._modify(user_file_source, upgrade_to_1)
+        assert "sec1" in user_file_source.secrets
+        assert "sec2" in user_file_source.secrets
+        self._assert_secret_is(user_file_source, "sec1", "moocow")
+        self._assert_secret_is(user_file_source, "sec2", "aftersec2")
+        upgrade_to_2 = UpgradeInstancePayload(
+            template_version=2,
+            secrets={},
+            variables={},
+        )
+
+        user_file_source = self._modify(user_file_source, upgrade_to_2)
+        assert "sec1" not in user_file_source.secrets
+        assert "sec2" in user_file_source.secrets
+        self._assert_secret_absent(user_file_source, "sec1")
+        self._assert_secret_is(user_file_source, "sec2", "aftersec2")
+
+    def test_upgrade_does_not_allow_extra_variables(self, tmp_path):
+        user_object_store = self._init_upgrade_test_case(tmp_path)
+        upgrade_to_1 = UpgradeInstancePayload(
+            template_version=1,
+            variables={
+                "extra_variable": "moocow",
+            },
+            secrets={
+                "sec1": "moocow",
+                "sec2": "aftersec2",
+            },
+        )
+
+        self._assert_modify_throws_exception(user_object_store, upgrade_to_1, RequestParameterInvalidException)
+
+    def test_upgrade_does_not_allow_extra_secrets(self, tmp_path):
+        user_object_store = self._init_upgrade_test_case(tmp_path)
+        upgrade_to_1 = UpgradeInstancePayload(
+            template_version=1,
+            variables={},
+            secrets={
+                "sec1": "moocow",
+                "sec2": "aftersec2",
+                "extrasec": "moo345",
+            },
+        )
+
+        self._assert_modify_throws_exception(user_object_store, upgrade_to_1, RequestParameterInvalidException)
+
+    def test_upgrade_fails_if_new_secrets_absent(self, tmp_path):
+        user_object_store = self._init_upgrade_test_case(tmp_path)
+        upgrade_to_1 = UpgradeInstancePayload(
+            template_version=1,
+            variables={},
+            secrets={
+                "sec1": "moocow",
+            },
+        )
+
+        self._assert_modify_throws_exception(user_object_store, upgrade_to_1, RequestParameterMissingException)
+
     def test_status_valid(self, tmp_path):
         self.init_user_in_database()
         self._init_managers(tmp_path)
@@ -433,6 +533,14 @@ class TestFileSourcesTestCase(BaseTestCase):
         assert "Input should be a valid boolean" in status.template_settings.message
         assert status.connection is None
 
+    def _init_upgrade_test_case(self, tmp_path) -> UserFileSourceModel:
+        example_yaml_str = UPGRADE_EXAMPLE
+        example_yaml_str.replace("/data", str(tmp_path))
+        config = safe_load(example_yaml_str)
+        self._init_managers(tmp_path, config)
+        user_file_source = self._create_instance(UPGRADE_INITIAL_PAYLOAD)
+        return user_file_source
+
     def _init_and_create_simple(self, tmp_path) -> UserFileSourceModel:
         self._init_managers(tmp_path)
         user_file_source = self._create_user_file_source()
@@ -462,6 +570,19 @@ class TestFileSourcesTestCase(BaseTestCase):
         # deleting vs never inserted...
         assert sec_val in ["", None]
 
+    def _assert_modify_throws_exception(
+        self, user_file_source: UserFileSourceModel, modify: ModifyInstancePayload, exception_type: Type
+    ):
+        exception_thrown = False
+        try:
+            self._modify(user_file_source, modify)
+        except exception_type:
+            exception_thrown = True
+        assert exception_thrown
+
+    def _modify(self, user_file_source: UserFileSourceModel, modify: ModifyInstancePayload) -> UserFileSourceModel:
+        return self.manager.modify_instance(self.trans, user_file_source.uuid, modify)
+
     def _create_instance(self, create_payload: CreateInstancePayload) -> UserFileSourceModel:
         user_file_source = self.manager.create_instance(self.trans, create_payload)
         return user_file_source
@@ -475,7 +596,10 @@ class TestFileSourcesTestCase(BaseTestCase):
         self.app.setup_test_vault()
         if config_dict is None:
             config_dict = home_directory_template(tmp_path)
-        templates_config = Config([config_dict])
+        if isinstance(config_dict, dict):
+            templates_config = Config([config_dict])
+        else:
+            templates_config = Config(config_dict)
         templates = ConfiguredFileSourceTemplates.from_app_config(
             templates_config,
             vault_configured=True,

test/unit/files/test_template_models.py

@@ -299,6 +299,7 @@ def test_examples_parse():
     _assert_example_parses("templating_override.yml")
     _assert_example_parses("admin_secrets.yml")
     _assert_example_parses("admin_secrets_with_defaults.yml")
+    _assert_example_parses("testing_multi_version_with_secrets.yml")
 
 
 def _assert_example_parses(filename: str):