Skip to content

Commit

Permalink
Refine object store templates backend.
Browse files Browse the repository at this point in the history
  • Loading branch information
jmchilton committed Apr 24, 2024
1 parent 05a8fa4 commit 7dfd63a
Show file tree
Hide file tree
Showing 15 changed files with 139 additions and 67 deletions.
8 changes: 5 additions & 3 deletions client/src/api/schema/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12500,7 +12500,7 @@ export interface components {
/** Device */
device?: string | null;
/** Id */
id: number;
id: number | string;
/** Name */
name?: string | null;
/** Object Store Id */
Expand All @@ -12519,6 +12519,8 @@ export interface components {
* @enum {string}
*/
type: "s3" | "azure_blob" | "disk" | "generic_s3";
/** Uuid */
uuid: string;
/** Variables */
variables: {
[key: string]: (string | boolean | number) | undefined;
Expand Down Expand Up @@ -20529,7 +20531,7 @@ export interface operations {
header?: {
"run-as"?: string | null;
};
/** @description The model ID for a persisted UserObjectStore object. */
/** @description The identifier used to index a persisted UserObjectStore object. */
path: {
user_object_store_id: string;
};
Expand All @@ -20556,7 +20558,7 @@ export interface operations {
header?: {
"run-as"?: string | null;
};
/** @description The model ID for a persisted UserObjectStore object. */
/** @description The identifier used to index a persisted UserObjectStore object. */
path: {
user_object_store_id: string;
};
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/ObjectStore/Instances/CreateForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ interface CreateFormProps {
}
const error = ref<string | null>(null);
const props = defineProps<CreateFormProps>();
const title = "Create a new object store for your data";
const title = "Create a new storage location for your data";
const submitTitle = "Submit";
const inputs = computed(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async function onCreated(objectStore: UserConcreteObjectStore) {

<template>
<div>
<LoadingSpan v-if="!template" message="Loading object store templates" />
<LoadingSpan v-if="!template" message="Loading storage location templates" />
<CreateForm v-else :template="template" @created="onCreated"></CreateForm>
</div>
</template>
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const inputs = computed(() => {
return form;
});
const title = computed(() => `Edit Object Store ${instance.value?.name} Settings`);
const title = computed(() => `Edit Storage Location ${instance.value?.name} Settings`);
const hasSecrets = computed(() => instance.value?.secrets && instance.value?.secrets.length > 0);
const submitTitle = "Update Settings";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ interface Props {
template: ObjectStoreTemplateSummary;
}
const props = defineProps<Props>();
const title = computed(() => `Update Object Store ${props.objectStore?.name} Secrets`);
const title = computed(() => `Update Storage Location ${props.objectStore?.name} Secrets`);
async function onUpdate(secretName: string, secretValue: string) {
const payload = {
Expand Down
6 changes: 3 additions & 3 deletions client/src/components/User/UserPreferences.vue
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@
:preferred-object-store-id="currentUser.preferred_object_store_id"
:user-id="userId">
</UserPreferredObjectStore>
<UserPreferredObjectStore
<UserPreferencesElement
v-if="hasTemplates"
id="manage-object-stores"
class="manage-object-stores"
icon="fa-hdd"
title="Manage Your Object Stores"
description="Add, remove, or update your personal object store configured."
title="Manage Your Storage Locations"
description="Add, remove, or update your personally configured storage locations."
to="/object_store_instances/index" />
<UserDeletion
v-if="isConfigLoaded && !config.single_user && config.enable_account_interface"
Expand Down
1 change: 1 addition & 0 deletions client/src/stores/objectStoreInstancesStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const TEST_INSTANCE = {
quota: { enabled: false },
private: false,
id: 4,
uuid: "112f889f-72d7-4619-a8e8-510a8c685aa7",
};

describe("Object Store Instances Store", () => {
Expand Down
1 change: 1 addition & 0 deletions lib/galaxy/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ def _configure_object_store(self, **kwds):
umask=self.config.umask,
object_store_cache_size=self.config.object_store_cache_size,
object_store_cache_path=self.config.object_store_cache_path,
user_object_store_index_by=self.config.user_object_store_index_by,
)
self._register_singleton(UserObjectStoresAppConfig, app_config)
user_object_store_resolver = self._register_abstract_singleton(
Expand Down
13 changes: 13 additions & 0 deletions lib/galaxy/config/schemas/config_schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,19 @@ mapping:
desc: |
Configured Object Store templates embedded into Galaxy's config.
user_object_store_index_by:
type: str
default: 'uuid'
required: false
enum: ['uuid', 'id']
desc: |
Configure URIs for user object stores to use either the object ID ('id')
or UUIDs ('uuid'). Either is fine really, Galaxy doesn't typically expose
database objects by 'id' but there isn't any obvious disadvantage to doing
it in this case and it keeps user exposed URIs much smaller. The default of
UUID feels a little more like a typical way to do this within Galaxy though.
Do not change this value once user object stores have been created.
enable_mulled_containers:
type: bool
default: true
Expand Down
108 changes: 74 additions & 34 deletions lib/galaxy/managers/object_store_instances.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@

import logging
from typing import (
Any,
Dict,
List,
Optional,
Union,
)
from uuid import uuid4

from pydantic import BaseModel

Expand Down Expand Up @@ -78,7 +80,8 @@ class UpgradeInstancePayload(BaseModel):


class UserConcreteObjectStoreModel(ConcreteObjectStoreModel):
id: int
id: Union[int, str]
uuid: str
type: ObjectStoreTemplateType
template_id: str
template_version: int
Expand All @@ -92,16 +95,19 @@ class UserConcreteObjectStoreModel(ConcreteObjectStoreModel):
class ObjectStoreInstancesManager:
_catalog: ConfiguredObjectStoreTemplates
_sa_session: galaxy_scoped_session
_app_config: UserObjectStoresAppConfig

def __init__(
self,
catalog: ConfiguredObjectStoreTemplates,
sa_session: galaxy_scoped_session,
vault: Vault,
app_config: UserObjectStoresAppConfig,
):
self._catalog = catalog
self._sa_session = sa_session
self._app_vault = vault
self._app_config = app_config

@property
def summaries(self) -> ObjectStoreTemplateSummaries:
Expand All @@ -123,10 +129,10 @@ def _upgrade_instance(
) -> UserConcreteObjectStoreModel:
persisted_object_store = self._get(trans, id)
catalog = self._catalog
template = catalog.find_template_by(persisted_object_store.object_store_template_id, payload.template_version)
persisted_object_store.object_store_template_version = template.version
persisted_object_store.object_store_template_definition = template.model_dump()
old_variables = persisted_object_store.object_store_template_variables or {}
template = catalog.find_template_by(persisted_object_store.template_id, payload.template_version)
persisted_object_store.template_version = template.version
persisted_object_store.template_definition = template.model_dump()
old_variables = persisted_object_store.template_variables or {}
updated_variables = payload.variables
actual_variables: OBJECT_STORE_TEMPLATE_CONFIGURATION_VARIABLES_TYPE = {}
for variable in template.variables or []:
Expand All @@ -136,11 +142,11 @@ def _upgrade_instance(
if updated_value:
actual_variables[variable_name] = updated_value

persisted_object_store.object_store_template_variables = actual_variables
old_secrets = persisted_object_store.object_store_template_secrets or []
persisted_object_store.template_variables = actual_variables
old_secrets = persisted_object_store.template_secrets or []
new_secrets = payload.secrets

recorded_secrets = persisted_object_store.object_store_template_secrets or []
recorded_secrets = persisted_object_store.template_secrets or []

user_vault = trans.user_vault
upgraded_template_secrets = []
Expand All @@ -154,23 +160,23 @@ def _upgrade_instance(
continue

secret_value = new_secrets[secret_name]
key = user_vault_key(persisted_object_store, secret_name)
key = user_vault_key(persisted_object_store, secret_name, self._app_config)
user_vault.write_secret(key, secret_value)
if secret_name not in recorded_secrets:
recorded_secrets.append(secret_name)

secrets_to_delete: List[str] = []
for recorded_secret in recorded_secrets:
if recorded_secret not in upgraded_template_secrets:
key = user_vault_key(persisted_object_store, recorded_secret)
key = user_vault_key(persisted_object_store, recorded_secret, self._app_config)
log.info(f"deleting {key} from user vault")
user_vault.delete_secret(key)
secrets_to_delete.append(recorded_secret)

for secret_to_delete in secrets_to_delete:
recorded_secrets.remove(secret_to_delete)

persisted_object_store.object_store_template_secrets = recorded_secrets
persisted_object_store.template_secrets = recorded_secrets
self._save(persisted_object_store)
rval = self._to_model(trans, persisted_object_store)
return rval
Expand All @@ -188,7 +194,7 @@ def _update_instance(
persisted_object_store.description = payload.description
if payload.variables is not None:
# maybe just record the valid variables according to template like in upgrade
persisted_object_store.object_store_template_variables = payload.variables
persisted_object_store.template_variables = payload.variables
self._save(persisted_object_store)
return self._to_model(trans, persisted_object_store)

Expand All @@ -197,7 +203,7 @@ def _update_instance_secret(
) -> UserConcreteObjectStoreModel:
persisted_object_store = self._get(trans, id)
user_vault = trans.user_vault
key = user_vault_key(persisted_object_store, payload.secret_name)
key = user_vault_key(persisted_object_store, payload.secret_name, self._app_config)
user_vault.write_secret(key, payload.secret_value)
return self._to_model(trans, persisted_object_store)

Expand All @@ -212,10 +218,11 @@ def create_instance(
persisted_object_store = UserObjectStore()
persisted_object_store.user_id = trans.user.id
assert persisted_object_store.user_id
persisted_object_store.object_store_template_definition = template.model_dump()
persisted_object_store.object_store_template_id = template.id
persisted_object_store.object_store_template_version = template.version
persisted_object_store.object_store_template_variables = payload.variables
persisted_object_store.uuid = uuid4().hex
persisted_object_store.template_definition = template.model_dump()
persisted_object_store.template_id = template.id
persisted_object_store.template_version = template.version
persisted_object_store.template_variables = payload.variables
persisted_object_store.name = payload.name
persisted_object_store.description = payload.description
self._save(persisted_object_store)
Expand All @@ -231,13 +238,13 @@ def create_instance(
recorded_secrets = []
try:
for secret, value in payload.secrets.items():
key = user_vault_key(persisted_object_store, secret)
key = user_vault_key(persisted_object_store, secret, self._app_config)
user_vault.write_secret(key, value)
recorded_secrets.append(secret)
except Exception:
self._sa_session.delete(persisted_object_store)
raise
persisted_object_store.object_store_template_secrets = recorded_secrets
persisted_object_store.template_secrets = recorded_secrets
self._save(persisted_object_store)
return self._to_model(trans, persisted_object_store)

Expand All @@ -255,13 +262,25 @@ def _save(self, persisted_object_store: UserObjectStore) -> None:
self._sa_session.commit()

def _get(self, trans: ProvidesUserContext, id: Union[str, int]) -> UserObjectStore:
user_object_store = self._sa_session.query(UserObjectStore).get(int(id))
filter = self._index_filter(id)
user_object_store = self._sa_session.query(UserObjectStore).filter(filter).one_or_none()
if user_object_store is None:
raise RequestParameterInvalidException(f"Failed to fetch object store for id {id}")
if user_object_store.user != trans.user:
raise ItemOwnershipException()
return user_object_store

def _index_filter(self, id: Union[str, int]):
index_by = self._app_config.user_object_store_index_by
index_filter: Any
if index_by == "id":
id_as_int = int(id)
index_filter = UserObjectStore.__table__.c.id == id_as_int
else:
id_as_str = str(id)
index_filter = UserObjectStore.__table__.c.uuid == id_as_str
return index_filter

def _to_model(self, trans, persisted_object_store: UserObjectStore) -> UserConcreteObjectStoreModel:
quota = QuotaModel(source=None, enabled=False)
object_store_type = persisted_object_store.template.configuration.type
Expand All @@ -274,42 +293,58 @@ def _to_model(self, trans, persisted_object_store: UserObjectStore) -> UserConcr
object_store_type in ["azure_blob", "s3"],
)
# These shouldn't be null but sometimes can be?
secrets = persisted_object_store.object_store_template_secrets or []
secrets = persisted_object_store.template_secrets or []
uos_id: str
response_id: Union[id, str]
if self._app_config.user_object_store_index_by == "id":
uos_id = str(persisted_object_store.id)
response_id = persisted_object_store.id
else:
uos_id = str(persisted_object_store.uuid)
response_id = uos_id
object_store_id = f"user_objects://{uos_id}"

return UserConcreteObjectStoreModel(
id=persisted_object_store.id,
id=response_id,
uuid=str(persisted_object_store.uuid),
type=object_store_type,
template_id=persisted_object_store.object_store_template_id,
template_version=persisted_object_store.object_store_template_version,
variables=persisted_object_store.object_store_template_variables,
template_id=persisted_object_store.template_id,
template_version=persisted_object_store.template_version,
variables=persisted_object_store.template_variables,
secrets=secrets,
name=persisted_object_store.name,
description=persisted_object_store.description,
object_store_id=f"user_objects://{persisted_object_store.id}",
object_store_id=object_store_id,
private=True,
quota=quota,
badges=badges,
)


def user_vault_key(user_object_store: UserObjectStore, secret: str) -> str:
uos_id = user_object_store.id
def user_vault_key(user_object_store: UserObjectStore, secret: str, app_config: UserObjectStoresAppConfig) -> str:
if app_config.user_object_store_index_by == "id":
uos_id = str(user_object_store.id)
else:
uos_id = str(user_object_store.uuid)
assert uos_id
user_vault_id_prefix = f"object_store_config/{uos_id}"
key = f"{user_vault_id_prefix}/{secret}"
return key


def recover_secrets(user_object_store: UserObjectStore, vault: Vault) -> Dict[str, str]:
def recover_secrets(
user_object_store: UserObjectStore, vault: Vault, app_config: UserObjectStoresAppConfig
) -> Dict[str, str]:
user: User = user_object_store.user
user_vault = UserVaultWrapper(vault, user)
secrets: Dict[str, str] = {}
# now we could recover the list of secrets to fetch from...
# ones recorded as written in the persisted object, the ones
# expected in the catalog, or the ones expected in the definition
# persisted.
persisted_secret_names = user_object_store.object_store_template_secrets or []
persisted_secret_names = user_object_store.template_secrets or []
for secret in persisted_secret_names:
vault_key = user_vault_key(user_object_store, secret)
vault_key = user_vault_key(user_object_store, secret, app_config)
secret_value = user_vault.read_secret(vault_key)
# assert secret_value
if secret_value is not None:
Expand All @@ -325,8 +360,13 @@ def __init__(self, sa_session: galaxy_scoped_session, vault: Vault, app_config:

def resolve_object_store_uri_config(self, uri: str) -> ObjectStoreConfiguration:
user_object_store_id = uri.split("://", 1)[1]
id_filter = UserObjectStore.__table__.c.id == user_object_store_id
user_object_store: UserObjectStore = self._sa_session.query(UserObjectStore).filter(id_filter).one()
secrets = recover_secrets(user_object_store, self._vault)
index_by = self._app_config.user_object_store_index_by
index_filter: Any
if index_by == "id":
index_filter = UserObjectStore.__table__.c.id == user_object_store_id
else:
index_filter = UserObjectStore.__table__.c.uuid == user_object_store_id
user_object_store: UserObjectStore = self._sa_session.query(UserObjectStore).filter(index_filter).one()
secrets = recover_secrets(user_object_store, self._vault, self._app_config)
object_store_configuration = user_object_store.object_store_configuration(secrets=secrets)
return object_store_configuration
Loading

0 comments on commit 7dfd63a

Please sign in to comment.