From 1a2e4e87abff2b0a2c735e5fd6a0323ae0c42d02 Mon Sep 17 00:00:00 2001 From: Ivan Zubenko Date: Sat, 1 Feb 2025 15:31:45 +0100 Subject: [PATCH 1/2] remove CORS configuration --- charts/platform-api/templates/deployment.yml | 4 -- charts/platform-api/values.yaml | 3 - platform_api/api.py | 21 +------ platform_api/config.py | 6 -- platform_api/config_factory.py | 9 --- setup.cfg | 6 +- tests/integration/conftest.py | 8 +-- tests/integration/test_api.py | 66 +------------------- tests/unit/test_config.py | 5 -- 9 files changed, 7 insertions(+), 121 deletions(-) diff --git a/charts/platform-api/templates/deployment.yml b/charts/platform-api/templates/deployment.yml index b26ce4ab9..c02235779 100644 --- a/charts/platform-api/templates/deployment.yml +++ b/charts/platform-api/templates/deployment.yml @@ -88,10 +88,6 @@ spec: value: http://localhost:8080/api/v1 - name: NP_ENFORCER_RETENTION_DELAY_DAYS value: {{ .Values.enforcerRetentionDelayDays | quote }} - {{- if .Values.cors.origins }} - - name: NP_CORS_ORIGINS - value: {{ .Values.cors.origins | join "," | quote}} - {{- end }} - name: NP_DB_POSTGRES_DSN {{- if .Values.postgres.dsn }} {{ toYaml .Values.postgres.dsn | indent 10 }} diff --git a/charts/platform-api/values.yaml b/charts/platform-api/values.yaml index c7a8148ce..ea8d712fe 100644 --- a/charts/platform-api/values.yaml +++ b/charts/platform-api/values.yaml @@ -33,9 +33,6 @@ oauth: successRedirectUrl: https://staging.neu.ro/oauth/login headlessCallbackUrl: https://staging.neu.ro/oauth/show-code -cors: - origins: [] - postgres: dsn: {} diff --git a/platform_api/api.py b/platform_api/api.py index 031ef5a5c..dbd2d8f46 100644 --- a/platform_api/api.py +++ b/platform_api/api.py @@ -6,7 +6,6 @@ from typing import Any import aiohttp.web -import aiohttp_cors from aiohttp.web import HTTPUnauthorized from aiohttp.web_urldispatcher import AbstractRoute from aiohttp_security import check_permission @@ -39,7 +38,7 @@ EnergySchedulePeriod, VolumeConfig, ) -from .config import Config, CORSConfig +from .config import Config from .config_client import ConfigClient from .config_factory import EnvironConfigFactory from .handlers import JobsHandler @@ -540,8 +539,6 @@ async def _init_app(app: aiohttp.web.Application) -> AsyncIterator[None]: app.add_subapp("/api/v1", api_v1_app) - _setup_cors(app, config.cors) - app.on_response_prepare.append(add_version_to_header) if config.zipkin: @@ -550,22 +547,6 @@ async def _init_app(app: aiohttp.web.Application) -> AsyncIterator[None]: return app -def _setup_cors(app: aiohttp.web.Application, config: CORSConfig) -> None: - if not config.allowed_origins: - return - - logger.info(f"Setting up CORS with allowed origins: {config.allowed_origins}") - default_options = aiohttp_cors.ResourceOptions( - allow_credentials=True, expose_headers="*", allow_headers="*" - ) - cors = aiohttp_cors.setup( - app, defaults={origin: default_options for origin in config.allowed_origins} - ) - for route in app.router.routes(): - logger.debug(f"Setting up CORS for {route}") - cors.add(route) - - def setup_tracing(config: Config) -> None: if config.zipkin: setup_zipkin_tracer( diff --git a/platform_api/config.py b/platform_api/config.py index 5bd8f0dd3..60a5fa4f1 100644 --- a/platform_api/config.py +++ b/platform_api/config.py @@ -218,11 +218,6 @@ def retention_delay(self) -> timedelta: return timedelta(days=self.retention_delay_days) -@dataclass(frozen=True) -class CORSConfig: - allowed_origins: Sequence[str] = () - - @dataclass(frozen=True) class JobsSchedulerConfig: # Minimal time that preepmtible job is guaranteed to run before suspended @@ -271,7 +266,6 @@ class Config: oauth: Optional[OAuthConfig] = None jobs: JobsConfig = JobsConfig() - cors: CORSConfig = CORSConfig() scheduler: JobsSchedulerConfig = JobsSchedulerConfig() diff --git a/platform_api/config_factory.py b/platform_api/config_factory.py index 0893be991..e4e4a83cc 100644 --- a/platform_api/config_factory.py +++ b/platform_api/config_factory.py @@ -12,7 +12,6 @@ from .config import ( AuthConfig, Config, - CORSConfig, DatabaseConfig, JobPolicyEnforcerConfig, JobsConfig, @@ -65,7 +64,6 @@ def create(self) -> Config: job_policy_enforcer=self.create_job_policy_enforcer(), scheduler=self.create_job_scheduler(), notifications=self.create_notifications(), - cors=self.create_cors(), config_url=config_url, admin_url=admin_url, admin_public_url=admin_public_url, @@ -227,13 +225,6 @@ def create_notifications(self) -> NotificationsConfig: token = self._environ.get("NP_NOTIFICATIONS_TOKEN", "") return NotificationsConfig(url=url, token=token) - def create_cors(self) -> CORSConfig: - origins: Sequence[str] = CORSConfig.allowed_origins - origins_str = self._environ.get("NP_CORS_ORIGINS", "").strip() - if origins_str: - origins = origins_str.split(",") - return CORSConfig(allowed_origins=origins) - def create_postgres(self) -> PostgresConfig: try: postgres_dsn = to_async_postgres_dsn(self._environ["NP_DB_POSTGRES_DSN"]) diff --git a/setup.cfg b/setup.cfg index 56614d57c..70daf8cfb 100644 --- a/setup.cfg +++ b/setup.cfg @@ -22,8 +22,7 @@ install_requires = cryptography==41.0.5 aiorwlock==1.3.0 neuro-notifications-client==22.6.2 - neuro-logging==21.12.2 - aiohttp-cors==0.7.0 + neuro-logging==21.12.0 asyncpg==0.28.0 sqlalchemy==1.4.25 alembic==1.9.4 @@ -142,9 +141,6 @@ ignore_missing_imports = true [mypy-_pytest.*] ignore_missing_imports = true -[mypy-aiohttp_cors] -ignore_missing_imports = true - [mypy-asyncpg.*] ignore_missing_imports = true diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index af8063d35..e8d755d09 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -32,7 +32,6 @@ from platform_api.config import ( AuthConfig, Config, - CORSConfig, DatabaseConfig, JobPolicyEnforcerConfig, JobsConfig, @@ -495,11 +494,11 @@ async def wait_pod_scheduled( cpu = resource_requests.get("cpu") memory = resource_requests.get("memory") print( - f' {pod["metadata"]["name"]:40s}', + f" {pod['metadata']['name']:40s}", f"{str(cpu):5s}", f"{str(memory):10s}", - f'{pod["status"]["phase"]:9s}', - f'{str(pod["spec"].get("nodeName"))}', + f"{pod['status']['phase']:9s}", + f"{str(pod['spec'].get('nodeName'))}", ) pytest.fail("Pod unscheduled") @@ -906,7 +905,6 @@ def _factory(**kwargs: Any) -> Config: jobs=jobs_config, job_policy_enforcer=job_policy_enforcer, notifications=notifications_config, - cors=CORSConfig(allowed_origins=["https://neu.ro"]), config_url=config_url, admin_url=admin_url, admin_public_url=admin_url, diff --git a/tests/integration/test_api.py b/tests/integration/test_api.py index 20e6e167b..819a1eedc 100644 --- a/tests/integration/test_api.py +++ b/tests/integration/test_api.py @@ -92,67 +92,6 @@ async def test_ping_includes_version( assert response.status == HTTPOk.status_code, await response.text() assert "platform-api" in response.headers["X-Service-Version"] - async def test_ping_unknown_origin( - self, api: ApiConfig, client: aiohttp.ClientSession - ) -> None: - async with client.get( - api.ping_url, headers={"Origin": "http://unknown"} - ) as response: - assert response.status == HTTPOk.status_code, await response.text() - assert "Access-Control-Allow-Origin" not in response.headers - - async def test_ping_allowed_origin( - self, api: ApiConfig, client: aiohttp.ClientSession - ) -> None: - async with client.get( - api.ping_url, headers={"Origin": "https://neu.ro"} - ) as resp: - assert resp.status == HTTPOk.status_code, await resp.text() - assert resp.headers["Access-Control-Allow-Origin"] == "https://neu.ro" - assert resp.headers["Access-Control-Allow-Credentials"] == "true" - assert resp.headers["Access-Control-Expose-Headers"] - - async def test_ping_options_no_headers( - self, api: ApiConfig, client: aiohttp.ClientSession - ) -> None: - async with client.options(api.ping_url) as resp: - assert resp.status == HTTPForbidden.status_code, await resp.text() - assert await resp.text() == ( - "CORS preflight request failed: " - "origin header is not specified in the request" - ) - - async def test_ping_options_unknown_origin( - self, api: ApiConfig, client: aiohttp.ClientSession - ) -> None: - async with client.options( - api.ping_url, - headers={ - "Origin": "http://unknown", - "Access-Control-Request-Method": "GET", - }, - ) as resp: - assert resp.status == HTTPForbidden.status_code, await resp.text() - assert await resp.text() == ( - "CORS preflight request failed: " - "origin 'http://unknown' is not allowed" - ) - - async def test_ping_options( - self, api: ApiConfig, client: aiohttp.ClientSession - ) -> None: - async with client.options( - api.ping_url, - headers={ - "Origin": "https://neu.ro", - "Access-Control-Request-Method": "GET", - }, - ) as resp: - assert resp.status == HTTPOk.status_code, await resp.text() - assert resp.headers["Access-Control-Allow-Origin"] == "https://neu.ro" - assert resp.headers["Access-Control-Allow-Credentials"] == "true" - assert resp.headers["Access-Control-Allow-Methods"] == "GET" - async def test_config_unauthorized( self, api: ApiConfig, client: aiohttp.ClientSession ) -> None: @@ -2955,8 +2894,7 @@ async def test_create_job_with_secret_env_wrong_scheme_fail( assert resp.status == HTTPBadRequest.status_code, await resp.text() msg = await resp.json() err = ( - "Invalid URI scheme: " - f"\\\\*'{wrong_scheme}\\\\*' != \\\\*'secret\\\\*'" + f"Invalid URI scheme: \\\\*'{wrong_scheme}\\\\*' != \\\\*'secret\\\\*'" ) assert re.search(err, msg["error"]), msg @@ -3149,7 +3087,7 @@ async def test_create_job_volume_wrong_cluster_name( url = api.jobs_base_url job_submit["container"]["volumes"] = [ { - "src_storage_uri": "storage://wrong-cluster/" f"{regular_user.name}", + "src_storage_uri": f"storage://wrong-cluster/{regular_user.name}", "dst_path": "/var/storage", "read_only": False, } diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 56aceb792..2afce47b5 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -386,8 +386,6 @@ def test_create_defaults(self) -> None: "https://dev.neu.ro/oauth/show-code" ) - assert not config.cors.allowed_origins - assert config.zipkin is None assert config.sentry is None @@ -424,7 +422,6 @@ def test_create_custom(self, cert_authority_path: str) -> None: "NP_NOTIFICATIONS_URL": "http://notifications:8080", "NP_NOTIFICATIONS_TOKEN": "token", "NP_ENFORCER_PLATFORM_API_URL": "http://platformapi:8080/api/v1", - "NP_CORS_ORIGINS": "https://domain1.com,http://do.main", "NP_ZIPKIN_URL": "https://zipkin:9411", "NP_SENTRY_DSN": "https://sentry", "NP_SENTRY_CLUSTER_NAME": "test", @@ -460,8 +457,6 @@ def test_create_custom(self, cert_authority_path: str) -> None: assert config.auth.service_token == "token" assert config.auth.service_name == "servicename" - assert config.cors.allowed_origins == ["https://domain1.com", "http://do.main"] - assert config.zipkin assert config.sentry From 82562936e5f9480c9b7426d426b7c8197fdb306a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 1 Feb 2025 14:33:23 +0000 Subject: [PATCH 2/2] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- platform_api/utils/asyncio.py | 4 ++-- tests/integration/api.py | 2 +- tests/integration/test_config_client.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/platform_api/utils/asyncio.py b/platform_api/utils/asyncio.py index 6b56d2082..42b53a26e 100644 --- a/platform_api/utils/asyncio.py +++ b/platform_api/utils/asyncio.py @@ -9,7 +9,7 @@ async def run_and_log_exceptions( - coros: Union[Coroutine[Any, Any, Any], Iterable[Coroutine[Any, Any, Any]]] + coros: Union[Coroutine[Any, Any, Any], Iterable[Coroutine[Any, Any, Any]]], ) -> None: try: # Check is iterable @@ -49,7 +49,7 @@ async def __aexit__( def asyncgeneratorcontextmanager( - func: Callable[..., T_co] + func: Callable[..., T_co], ) -> Callable[..., AbstractAsyncContextManager[T_co]]: @functools.wraps(func) def wrapper(*args: Any, **kwargs: Any) -> AbstractAsyncContextManager[T_co]: diff --git a/tests/integration/api.py b/tests/integration/api.py index efde7908f..57a9afac9 100644 --- a/tests/integration/api.py +++ b/tests/integration/api.py @@ -433,6 +433,6 @@ def _factory(cluster_name: Optional[str] = None) -> dict[str, Any]: @pytest.fixture async def job_submit( - job_request_factory: Callable[[], dict[str, Any]] + job_request_factory: Callable[[], dict[str, Any]], ) -> dict[str, Any]: return job_request_factory() diff --git a/tests/integration/test_config_client.py b/tests/integration/test_config_client.py index ec7eb6328..207faa90f 100644 --- a/tests/integration/test_config_client.py +++ b/tests/integration/test_config_client.py @@ -64,7 +64,7 @@ async def handle(request: aiohttp.web.Request) -> aiohttp.web.Response: @asynccontextmanager async def create_config_api( - cluster_configs_payload: list[dict[str, Any]] + cluster_configs_payload: list[dict[str, Any]], ) -> AsyncIterator[URL]: app = await create_config_app(cluster_configs_payload) runner = ApiRunner(app, port=8082)