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 22, 2024
1 parent 77de715 commit c4d6021
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 51 deletions.
4 changes: 2 additions & 2 deletions client/src/api/schema/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20669,7 +20669,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 @@ -20696,7 +20696,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
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
82 changes: 51 additions & 31 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 @@ -92,16 +94,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 +128,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 +141,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 +159,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 +193,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 +202,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 +217,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 +237,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 Down Expand Up @@ -274,42 +280,51 @@ 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
if self._app_config.user_object_store_index_by == "id":
uos_id = str(persisted_object_store.id)
else:
uos_id = str(persisted_object_store.uuid)
object_store_id = f"user_objects://{uos_id}"
return UserConcreteObjectStoreModel(
id=persisted_object_store.id,
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 = 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 +340,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
15 changes: 8 additions & 7 deletions lib/galaxy/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10889,37 +10889,38 @@ class UserObjectStore(Base, RepresentById):

id: Mapped[int] = mapped_column(Integer, primary_key=True)
user_id: Mapped[Optional[int]] = mapped_column(Integer, ForeignKey("galaxy_user.id"), index=True)
uuid: Mapped[Union[UUID, str]] = mapped_column(UUIDType(), index=True)
create_time: Mapped[datetime] = mapped_column(DateTime, default=now)
update_time: Mapped[datetime] = mapped_column(DateTime, default=now, onupdate=now, index=True)
# user specified name of the instance they've created
name: Mapped[str] = mapped_column(String(255), index=True)
# user specified description of the instance they've created
description: Mapped[Optional[str]] = mapped_column(Text)
# the template store id
object_store_template_id: Mapped[str] = mapped_column(String(255), index=True)
template_id: Mapped[str] = mapped_column(String(255), index=True)
# the template store version (0, 1, ...)
object_store_template_version: Mapped[int] = mapped_column(Integer, index=True)
template_version: Mapped[int] = mapped_column(Integer, index=True)
# Full template from object_store_templates.yml catalog.
# For tools we just store references, so here we could easily just use
# the id/version and not record the definition... as the templates change
# over time this choice has some big consequences despite being easy to swap
# implementations.
object_store_template_definition: Mapped[Optional[OBJECT_STORE_TEMPLATE_DEFINITION_TYPE]] = mapped_column(JSONType)
template_definition: Mapped[Optional[OBJECT_STORE_TEMPLATE_DEFINITION_TYPE]] = mapped_column(JSONType)
# Big JSON blob of the variable name -> value mapping defined for the store's
# variables by the user.
object_store_template_variables: Mapped[Optional[OBJECT_STORE_TEMPLATE_CONFIGURATION_VARIABLES_TYPE]] = (
template_variables: Mapped[Optional[OBJECT_STORE_TEMPLATE_CONFIGURATION_VARIABLES_TYPE]] = (
mapped_column(JSONType)
)
# Track a list of secrets that were defined for this object store at creation
object_store_template_secrets: Mapped[Optional[OBJECT_STORE_TEMPLATE_CONFIGURATION_SECRET_NAMES_TYPE]] = (
template_secrets: Mapped[Optional[OBJECT_STORE_TEMPLATE_CONFIGURATION_SECRET_NAMES_TYPE]] = (
mapped_column(JSONType)
)

user = relationship("User", back_populates="object_stores")

@property
def template(self) -> ObjectStoreTemplate:
return ObjectStoreTemplate(**self.object_store_template_definition or {})
return ObjectStoreTemplate(**self.template_definition or {})

def object_store_configuration(self, secrets: Dict[str, Any]) -> ObjectStoreConfiguration:
user = self.user
Expand All @@ -10928,7 +10929,7 @@ def object_store_configuration(self, secrets: Dict[str, Any]) -> ObjectStoreConf
"email": user.email,
"id": user.id,
}
variables: OBJECT_STORE_TEMPLATE_CONFIGURATION_VARIABLES_TYPE = self.object_store_template_variables or {}
variables: OBJECT_STORE_TEMPLATE_CONFIGURATION_VARIABLES_TYPE = self.template_variables or {}
return template_to_configuration(
self.template,
variables=variables,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
Text,
)

from galaxy.model.custom_types import JSONType
from galaxy.model.custom_types import (
JSONType,
UUIDType,
)
from galaxy.model.migrations.util import (
create_table,
drop_table,
Expand All @@ -37,15 +40,16 @@ def upgrade():
table_name,
Column("id", Integer, primary_key=True),
Column("user_id", Integer, ForeignKey("galaxy_user.id"), nullable=False, index=True),
Column("uuid", UUIDType, nullable=False, index=True),
Column("name", String(255), index=True),
Column("description", Text, index=True),
Column("create_time", DateTime),
Column("update_time", DateTime),
Column("object_store_template_id", String(255), index=True),
Column("object_store_template_version", Integer, index=True),
Column("object_store_template_definition", JSONType),
Column("object_store_template_variables", JSONType),
Column("object_store_template_secrets", JSONType),
Column("template_id", String(255), index=True),
Column("template_version", Integer, index=True),
Column("template_definition", JSONType),
Column("template_variables", JSONType),
Column("template_secrets", JSONType),
)


Expand Down
17 changes: 13 additions & 4 deletions lib/galaxy/objectstore/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
Any,
Dict,
List,
Literal,
NamedTuple,
Optional,
Set,
Expand Down Expand Up @@ -1360,10 +1361,17 @@ def validate_selected_object_store_id(self, user, object_store_id: Optional[str]
if not user:
return "Supplied object store id is not accessible"
rest_of_uri = object_store_id.split("://", 1)[1]
user_object_store_id = int(rest_of_uri)
for user_object_store in user.object_stores:
if user_object_store.id == user_object_store_id:
return None
index_by = self.config.user_object_store_index_by
if index_by == "id":
user_object_store_id = int(rest_of_uri)
for user_object_store in user.object_stores:
if user_object_store.id == user_object_store_id:
return None
else:
user_object_store_uuid = rest_of_uri
for user_object_store in user.object_stores:
if str(user_object_store.uuid) == user_object_store_uuid:
return None
return "Supplied object store id was not found"
if object_store_id not in self.object_store_ids_allowing_selection():
return "Supplied object store id is not an allowed object store selection"
Expand Down Expand Up @@ -1624,6 +1632,7 @@ def build_object_store_from_config(
class UserObjectStoresAppConfig(BaseModel):
object_store_cache_path: str
object_store_cache_size: int
user_object_store_index_by: Literal['uuid', 'id']
jobs_directory: str
new_file_path: str
umask: int
Expand Down
Loading

0 comments on commit c4d6021

Please sign in to comment.