-
Notifications
You must be signed in to change notification settings - Fork 0
Add: Services to system #883
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
eb166b1
363c92c
94ae725
1eeb1d5
371e37f
88cc71a
13b2265
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,182 @@ | ||
| """Rename services container to System for AD compatibility. | ||
| Revision ID: a1b2c3d4e5f6 | ||
| Revises: 6c858cc05da7 | ||
| Create Date: 2026-01-13 12:00:00.000000 | ||
| """ | ||
|
|
||
| from alembic import op | ||
| from dishka import AsyncContainer | ||
| from sqlalchemy import and_, exists, select | ||
| from sqlalchemy.ext.asyncio import AsyncConnection, AsyncSession | ||
|
|
||
| from entities import Attribute, Directory | ||
| from ldap_protocol.utils.queries import get_base_directories | ||
| from repo.pg.tables import queryable_attr as qa | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision: None | str = "a1b2c3d4e5f6" | ||
| down_revision: None | str = "6c858cc05da7" | ||
| branch_labels: None | list[str] = None | ||
| depends_on: None | list[str] = None | ||
|
|
||
|
|
||
| def upgrade(container: AsyncContainer) -> None: # noqa: ARG001 | ||
| """Upgrade: Rename 'services' container to 'System'.""" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] название System именно в upper кейсе? |
||
|
|
||
| async def _rename_services_to_system(connection: AsyncConnection) -> None: | ||
| session = AsyncSession(bind=connection) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. из контейнера берем теперь сесиию |
||
| await session.begin() | ||
|
|
||
| try: | ||
| base_directories = await get_base_directories(session) | ||
| if not base_directories: | ||
| await session.commit() | ||
| return | ||
|
Comment on lines
+33
to
+36
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. это можно наружу, зачем это внутри try
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. или может вообще уберем try? у нас это не очень распространенная практика |
||
|
|
||
| services_dirs = await session.scalars( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. services_dirs -> service_dirs |
||
| select(Directory).where(qa(Directory.name) == "services"), | ||
| ) | ||
|
|
||
| for services_dir in services_dirs: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. services_dir -> service_dir |
||
| system_exists = await session.scalar( | ||
| select(exists(Directory)).where( | ||
| and_( | ||
| qa(Directory.name) == "System", | ||
| qa(Directory.parent_id) == services_dir.parent_id, | ||
| ), | ||
| ), | ||
| ) | ||
|
Comment on lines
+43
to
+50
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
проверь работоспособность этого решения в реалтайме плз
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. можно создать любые директории-контейнеры и прочее руками, а затем сделать
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
можно в запрос встроить
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| if system_exists: | ||
| continue | ||
|
|
||
| services_dir.name = "System" | ||
| services_dir.path = [ | ||
| "ou=System" if p == "ou=services" else p | ||
| for p in services_dir.path | ||
| ] | ||
|
|
||
| await session.flush() | ||
| await _update_descendants(session, services_dir.id) | ||
|
|
||
| await _update_attributes(session, "ou=services", "ou=System") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. предлагаю строго привязывать к id той директории которую мы обновили, т.е. передавать в _update_attributes еще и directory_id, чтобы не сделать "лишних" изменений у тех директорий, которые в отбор не попали, но атрибуты которых попали под фильтр атрибутов. думаю, это повысит надежность миграции |
||
| await session.commit() | ||
|
|
||
| except Exception: | ||
| await session.rollback() | ||
| raise | ||
|
|
||
| async def _update_descendants( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. обе этих протектед функции предлагаю перенести вверх, чтобы сначала разработчик ознакамливался с ними, а только потом видел их использование в коде |
||
| session: AsyncSession, | ||
| parent_id: int, | ||
| ) -> None: | ||
| """Recursively update paths of all descendants.""" | ||
| children = await session.scalars( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. children -> child_dirs |
||
| select(Directory).where(qa(Directory.parent_id) == parent_id), | ||
| ) | ||
|
Comment on lines
+76
to
+78
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. предлагаю сделать "команда с отдельной строки" как у нас везде принято: |
||
|
|
||
| for child in children: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. child -> child_dir |
||
| child.path = [ | ||
| "ou=System" if p == "ou=services" else p for p in child.path | ||
| ] | ||
| await session.flush() | ||
| await _update_descendants(session, child.id) | ||
|
|
||
| async def _update_attributes( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. обе этих протектед функции предлагаю перенести вверх, чтобы сначала разработчик ознакамливался с ними, а только потом видел их использование в коде |
||
| session: AsyncSession, | ||
| old_value: str, | ||
| new_value: str, | ||
| ) -> None: | ||
| """Update attribute values containing old DN.""" | ||
| result = await session.execute( | ||
| select(Attribute).where( | ||
| Attribute.value.ilike(f"%{old_value}%"), # type: ignore | ||
| ), | ||
| ) | ||
|
Comment on lines
+93
to
+97
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. предлагаю сделать "команда с отдельной строки" как у нас везде принято: |
||
| attributes = result.scalars().all() | ||
|
|
||
| for attr in attributes: | ||
| if attr.value and old_value in attr.value: | ||
| attr.value = attr.value.replace(old_value, new_value) | ||
|
|
||
| await session.flush() | ||
|
|
||
| op.run_async(_rename_services_to_system) | ||
|
|
||
|
|
||
| def downgrade(container: AsyncContainer) -> None: # noqa: ARG001 | ||
| """Downgrade: Rename 'System' container back to 'services'.""" | ||
|
|
||
|
Comment on lines
+109
to
+111
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. проверь downgrade на аналогичные комментарии что и upgrade
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| async def _rename_system_to_services(connection: AsyncConnection) -> None: | ||
| session = AsyncSession(bind=connection) | ||
| await session.begin() | ||
|
|
||
| try: | ||
| base_directories = await get_base_directories(session) | ||
| if not base_directories: | ||
| await session.commit() | ||
| return | ||
|
|
||
| system_dirs = await session.scalars( | ||
| select(Directory).where(qa(Directory.name) == "System"), | ||
| ) | ||
|
|
||
| for system_dir in system_dirs: | ||
| system_dir.name = "services" | ||
| system_dir.path = [ | ||
| "ou=services" if p == "ou=System" else p | ||
| for p in system_dir.path | ||
| ] | ||
|
|
||
| await session.flush() | ||
| await _update_descendants_downgrade(session, system_dir.id) | ||
|
|
||
| await _update_attributes_downgrade( | ||
| session, | ||
| "ou=System", | ||
| "ou=services", | ||
| ) | ||
| await session.commit() | ||
|
|
||
| except Exception: | ||
| await session.rollback() | ||
| raise | ||
|
|
||
| async def _update_descendants_downgrade( | ||
| session: AsyncSession, | ||
| parent_id: int, | ||
| ) -> None: | ||
| """Recursively update paths of all descendants.""" | ||
| children = await session.scalars( | ||
| select(Directory).where(qa(Directory.parent_id) == parent_id), | ||
| ) | ||
|
|
||
| for child in children: | ||
| child.path = [ | ||
| "ou=services" if p == "ou=System" else p for p in child.path | ||
| ] | ||
| await session.flush() | ||
| await _update_descendants_downgrade(session, child.id) | ||
|
|
||
| async def _update_attributes_downgrade( | ||
| session: AsyncSession, | ||
| old_value: str, | ||
| new_value: str, | ||
| ) -> None: | ||
| """Update attribute values during downgrade.""" | ||
| result = await session.execute( | ||
| select(Attribute).where( | ||
| Attribute.value.ilike(f"%{old_value}%"), # type: ignore | ||
| ), | ||
| ) | ||
| attributes = result.scalars().all() | ||
|
|
||
| for attr in attributes: | ||
| if attr.value and old_value in attr.value: | ||
| attr.value = attr.value.replace(old_value, new_value) | ||
|
|
||
| await session.flush() | ||
|
|
||
| op.run_async(_rename_system_to_services) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,46 +1,89 @@ | ||
| """Kerberos update config. | ||
| """Kerberos configuration update script. | ||
|
|
||
| Copyright (c) 2025 MultiFactor | ||
| License: https://github.com/MultiDirectoryLab/MultiDirectory/blob/main/LICENSE | ||
| """ | ||
|
|
||
| from pathlib import Path | ||
|
|
||
| from loguru import logger | ||
| from sqlalchemy.ext.asyncio import AsyncSession | ||
|
|
||
| from config import Settings | ||
| from ldap_protocol.kerberos import AbstractKadmin | ||
| from ldap_protocol.kerberos.utils import get_services_container_dn | ||
| from ldap_protocol.utils.queries import get_base_directories | ||
|
|
||
| KRB5_CONF_PATH = Path("/etc/krb5kdc/krb5.conf") | ||
| KDC_CONF_PATH = Path("/etc/krb5kdc/kdc.conf") | ||
| STASH_FILE_PATH = Path("/etc/krb5kdc/krb5.d/stash.keyfile") | ||
|
|
||
|
|
||
| def _migrate_legacy_dns(content: str) -> str: | ||
| """Replace legacy DN formats with current ones. | ||
|
|
||
| :param content: File content to migrate. | ||
| :return: Migrated content. | ||
| """ | ||
| return content.replace("ou=services", "ou=System").replace( | ||
| "ou=users", | ||
| "cn=users", | ||
| ) | ||
|
|
||
|
|
||
| async def update_krb5_config( | ||
| kadmin: AbstractKadmin, | ||
| session: AsyncSession, | ||
| settings: Settings, | ||
| ) -> None: | ||
| """Update kerberos config.""" | ||
| if not (await kadmin.get_status(wait_for_positive=True)): | ||
| logger.error("kadmin_api is not running") | ||
| return | ||
| """Update Kerberos configuration files via direct write to shared volume. | ||
|
|
||
| base_dn_list = await get_base_directories(session) | ||
| base_dn = base_dn_list[0].path_dn | ||
| domain: str = base_dn_list[0].name | ||
| Renders krb5.conf and kdc.conf from templates and writes them directly | ||
| to the shared volume. Also migrates legacy DN formats in stash.keyfile | ||
| if present (ou=services -> ou=System, ou=users -> cn=users). | ||
|
|
||
| krbadmin = "cn=krbadmin,cn=users," + base_dn | ||
| services_container = "ou=services," + base_dn | ||
| :param session: Database session for fetching base directories. | ||
| :param settings: Application settings with template environment. | ||
| :raises Exception: If config rendering or writing fails. | ||
| """ | ||
| if not KRB5_CONF_PATH.parent.exists(): | ||
| logger.error( | ||
| f"Config directory {KRB5_CONF_PATH.parent} not found, " | ||
| "kerberos volume not mounted", | ||
| ) | ||
| return | ||
|
|
||
| krb5_template = settings.TEMPLATES.get_template("krb5.conf") | ||
| kdc_template = settings.TEMPLATES.get_template("kdc.conf") | ||
| base_dn_list = await get_base_directories(session) | ||
| if not base_dn_list: | ||
| logger.error("No base directories found") | ||
| return | ||
|
|
||
| kdc_config = await kdc_template.render_async(domain=domain) | ||
| base_dn = base_dn_list[0].path_dn | ||
| domain = base_dn_list[0].name | ||
| krbadmin = f"cn=krbadmin,cn=users,{base_dn}" | ||
| services_container = get_services_container_dn(base_dn) | ||
|
|
||
| krb5_config = await krb5_template.render_async( | ||
| krb5_config = await settings.TEMPLATES.get_template( | ||
| "krb5.conf", | ||
| ).render_async( | ||
| domain=domain, | ||
| krbadmin=krbadmin, | ||
| services_container=services_container, | ||
| ldap_uri=settings.KRB5_LDAP_URI, | ||
| mfa_push_url=settings.KRB5_MFA_PUSH_URL, | ||
| sync_password_url=settings.KRB5_SYNC_PASSWORD_URL, | ||
| ) | ||
| kdc_config = await settings.TEMPLATES.get_template( | ||
| "kdc.conf", | ||
| ).render_async( | ||
| domain=domain, | ||
| ) | ||
|
|
||
| KRB5_CONF_PATH.write_text(krb5_config, encoding="utf-8") | ||
| KDC_CONF_PATH.write_text(kdc_config, encoding="utf-8") | ||
|
|
||
| await kadmin.setup_configs(krb5_config, kdc_config) | ||
| if STASH_FILE_PATH.exists(): | ||
| stash_content = STASH_FILE_PATH.read_text(encoding="utf-8") | ||
| if "ou=services" in stash_content or "ou=users" in stash_content: | ||
| STASH_FILE_PATH.write_text( | ||
| _migrate_legacy_dns(stash_content), | ||
| encoding="utf-8", | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,7 +43,12 @@ | |
| from .ldap_structure import KRBLDAPStructureManager | ||
| from .schemas import AddRequests, KDCContext, KerberosAdminDnGroup, TaskStruct | ||
| from .template_render import KRBTemplateRenderer | ||
| from .utils import KerberosState, get_krb_server_state, set_state | ||
| from .utils import ( | ||
| KerberosState, | ||
| get_krb_server_state, | ||
| get_services_container_dn, | ||
| set_state, | ||
| ) | ||
|
|
||
|
|
||
| class KerberosService(AbstractService): | ||
|
|
@@ -140,7 +145,7 @@ def _build_kerberos_admin_dns(self, base_dn: str) -> KerberosAdminDnGroup: | |
| dataclass with DN for krbadmin, services_container, krbadmin_group. | ||
| """ | ||
| krbadmin = f"cn=krbadmin,cn=users,{base_dn}" | ||
| services_container = f"ou=services,{base_dn}" | ||
| services_container = get_services_container_dn(base_dn) | ||
| krbgroup = f"cn=krbadmin,cn=groups,{base_dn}" | ||
| return KerberosAdminDnGroup( | ||
| krbadmin_dn=krbadmin, | ||
|
|
@@ -288,7 +293,7 @@ async def _get_kdc_context(self) -> KDCContext: | |
| base_dn, domain = await self._get_base_dn() | ||
| krbadmin = f"cn=krbadmin,cn=users,{base_dn}" | ||
| krbgroup = f"cn=krbadmin,cn=groups,{base_dn}" | ||
| services_container = f"ou=services,{base_dn}" | ||
| services_container = get_services_container_dn(base_dn) | ||
|
Comment on lines
293
to
+296
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] можем сделать для krbadmin, krbgroup, ... аналогичный метод что и для services_container м? |
||
| return KDCContext( | ||
| base_dn=base_dn, | ||
| domain=domain, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,3 +131,8 @@ async def unlock_principal(name: str, session: AsyncSession) -> None: | |
| .filter_by(directory_id=subquery, name="krbprincipalexpiration") | ||
| .execution_options(synchronize_session=False), | ||
| ) | ||
|
|
||
|
|
||
| def get_services_container_dn(base_dn: str) -> str: | ||
| """Get System container DN for services.""" | ||
| return f"ou=System,{base_dn}" | ||
|
Comment on lines
+136
to
+138
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а это точно к керберосу относится а не к лдапу? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -424,6 +424,7 @@ services: | |
| - ./certs:/certs | ||
| - ./app:/app | ||
| - ldap_keytab:/LDAP_keytab/ | ||
| - kdc:/etc/krb5kdc/ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. а это нужная для этого ПР правка? не могу разобраться |
||
| env_file: local.env | ||
| command: python multidirectory.py --scheduler | ||
| tty: true | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
еще что не могу понять, а что если в лдап дереве нет директории с именем service ? мы же тогда просто выйдем из миграции, и кажется , что это не совсем правильно, наверное стоит создавать директорию System в таком случае
не знаю, стоит ли мне лезть в бизнеслогику, но не знаю в общем...