diff --git a/CHANGELOG.md b/CHANGELOG.md index 56c81c42..efc97e20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +# v0.8.4 + +### Features +- [x] Add basic config in helm chart and CLI for Istio. +- [x] Add worker route to get pieces repository. + + +### Fixes +- [x] Add classic authorization to default route for get pieces repositories. + + # v0.8.3 ### Features diff --git a/helm/domino/Chart.yaml b/helm/domino/Chart.yaml index 1053d75d..fbd9da9f 100644 --- a/helm/domino/Chart.yaml +++ b/helm/domino/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: domino description: A Helm chart for Domino type: application -version: 0.1.9 -appVersion: 0.1.9 +version: 0.1.10 +appVersion: 0.1.10 home: https://github.com/Tauffer-Consulting/domino sources: - https://github.com/Tauffer-Consulting/domino diff --git a/helm/domino/templates/jobs/domino-migrations.yml b/helm/domino/templates/jobs/domino-migrations.yml index 0c3545c7..3f41cea1 100644 --- a/helm/domino/templates/jobs/domino-migrations.yml +++ b/helm/domino/templates/jobs/domino-migrations.yml @@ -8,6 +8,9 @@ spec: parallelism: 1 backoffLimit: 4 template: + metadata: + annotations: + sidecar.istio.io/inject: "false" spec: restartPolicy: OnFailure containers: diff --git a/rest/routers/piece_repository_router.py b/rest/routers/piece_repository_router.py index 878bd868..056ded72 100644 --- a/rest/routers/piece_repository_router.py +++ b/rest/routers/piece_repository_router.py @@ -120,8 +120,7 @@ def get_piece_repository_release_data( status.HTTP_500_INTERNAL_SERVER_ERROR: {'model': SomethingWrongError}, status.HTTP_403_FORBIDDEN: {'model': ForbiddenError}, }, - # TODO - I commented this to make it easier to test, but we should solve the auth service - # dependencies=[Depends(auth_service.workspace_access_authorizer)] + dependencies=[Depends(auth_service.workspace_access_authorizer)] ) def get_pieces_repositories( workspace_id: int, @@ -142,6 +141,39 @@ def get_pieces_repositories( raise HTTPException(status_code=e.status_code, detail=e.message) +@router.get( + path="/worker", + status_code=status.HTTP_200_OK, + responses={ + status.HTTP_200_OK: {'model': GetWorkspaceRepositoriesResponse}, + status.HTTP_500_INTERNAL_SERVER_ERROR: {'model': SomethingWrongError}, + status.HTTP_403_FORBIDDEN: {'model': ForbiddenError}, + }, +) +def get_pieces_repositories_worker( + workspace_id: int, + page: Optional[int] = 0, + page_size: Optional[int] = 100, + filters: ListRepositoryFilters = Depends(), +) -> GetWorkspaceRepositoriesResponse: + """ + Get pieces repositories for workspace. + This endpoint is used by the worker to get the repositories to be processed. + Is the same endpoint as the one above, but without the auth service. + The authorization is done by our service mesh Authorization Policy. + """ + try: + response = piece_repository_service.get_pieces_repositories( + workspace_id=workspace_id, + page=page, + page_size=page_size, + filters=filters + ) + return response + except (BaseException, ForbiddenException) as e: + raise HTTPException(status_code=e.status_code, detail=e.message) + + @router.delete( path="/{piece_repository_id}", status_code=status.HTTP_204_NO_CONTENT, diff --git a/rest/routers/secret_router.py b/rest/routers/secret_router.py index 8b3117d6..f0a4c535 100644 --- a/rest/routers/secret_router.py +++ b/rest/routers/secret_router.py @@ -70,7 +70,7 @@ def update_repository_secret( raise HTTPException(status_code=e.status_code, detail=e.message) @router.get( - path='/{piece_name}', + path='/{piece_name}/secrets-values', # using sufix /secrets-values only because istio does not support wildcards in paths status_code=200, responses={ status.HTTP_200_OK: {'model': List[GetSecretsByPieceResponse]}, @@ -79,13 +79,15 @@ def update_repository_secret( }, include_in_schema=False ) -#@auth_service.authorize_repository_workspace_access # TODO authorize only worker def get_piece_secrets( piece_repository_id: int, - piece_name: str, # TODO check what is better to use. query or path ? - #auth_context: AuthorizationContextData = Depends(auth_service.auth_wrapper) + piece_name: str, ) -> List[GetSecretsByPieceResponse]: - """Get secrets for a specific Piece from an piece repository, in a specific workspace""" + """ + Get secrets values for a specific Piece from an piece repository, in a specific workspace + This endpoint is not using authorization service because it is used by airflow to get secrets values + In production this endpoint should be blocked from external access using security strategies like authorization policies. + """ try: response = secret_service.get_piece_secrets( piece_repository_id=piece_repository_id, diff --git a/rest/tests/secret/fixtures.py b/rest/tests/secret/fixtures.py index a674d01f..48c51cb6 100644 --- a/rest/tests/secret/fixtures.py +++ b/rest/tests/secret/fixtures.py @@ -33,7 +33,7 @@ def patch_piece_secret(client: ApiTestClient, authorization_token: Dict, piece_r @pytest.fixture(scope="function") def get_secrets_by_piece_name(request, client: ApiTestClient, authorization_token: Dict, piece_repository: PieceRepository): return client.get( - f"/pieces-repositories/{piece_repository.id}/secrets/{request.param['piece_name']}", + f"/pieces-repositories/{piece_repository.id}/secrets/{request.param['piece_name']}/secrets-values", headers={"Authorization": authorization_token["header"]} ) @@ -72,5 +72,5 @@ def get_repository_secrets_mock_response(): # ) ] return mock_response - + diff --git a/rest/tests/secret/test_secret_router.py b/rest/tests/secret/test_secret_router.py index cb1887e1..9aa8b38e 100644 --- a/rest/tests/secret/test_secret_router.py +++ b/rest/tests/secret/test_secret_router.py @@ -9,7 +9,7 @@ "tests.secret.fixtures" ] @pytest.mark.usefixtures("register", "login", "add_piece_repository", "teardown_piece_repository") -class TestSecretRouter: +class TestSecretRouter: @staticmethod def test_get_repository_secrets(get_repository_secrets: Response, get_repository_secrets_mock_response: List): mock_response = get_repository_secrets_mock_response @@ -26,18 +26,18 @@ def test_get_repository_secrets(get_repository_secrets: Response, get_repository secret_content = json.loads(secret.model_dump_json()) mock_response_secrets_names.append(secret_content.get("name")) mock_response_secrets_names.sort() - + assert response.status_code == 200 mock_response_content = json.loads(mock_response[0].model_dump_json()) assert content[0].keys() == mock_response_content.keys() assert response_secrets_names == mock_response_secrets_names - + @staticmethod def test_patch_piece_secret(patch_piece_secret: Response): response = patch_piece_secret assert response.status_code == 204 - + @staticmethod @pytest.mark.parametrize("get_secrets_by_piece_name", [{"piece_name": "SimpleLogPiece"}], indirect=True) def test_get_secrets_by_piece_name(get_secrets_by_piece_name: Response): @@ -55,4 +55,3 @@ def test_get_secrets_by_piece_name(get_secrets_by_piece_name: Response): assert response.status_code == 200 assert content[0] == mock_response_content - \ No newline at end of file diff --git a/src/domino/VERSION b/src/domino/VERSION index fab77af2..fcbb5375 100644 --- a/src/domino/VERSION +++ b/src/domino/VERSION @@ -1 +1 @@ -0.8.3 \ No newline at end of file +0.8.4 \ No newline at end of file diff --git a/src/domino/cli/utils/platform.py b/src/domino/cli/utils/platform.py index f6180670..86b6bf0d 100644 --- a/src/domino/cli/utils/platform.py +++ b/src/domino/cli/utils/platform.py @@ -397,6 +397,22 @@ def create_platform(install_airflow: bool = True, use_gpu: bool = False) -> None "sshKeySecret": "airflow-ssh-secret" }, }, + "migrateDatabaseJob": { + "jobAnnotations": { + "sidecar.istio.io/inject": "false" + }, + "annotations": { + "sidecar.istio.io/inject": "false" + }, + }, + "createUserJob": { + "jobAnnotations": { + "sidecar.istio.io/inject": "false" + }, + "annotations": { + "sidecar.istio.io/inject": "false" + }, + }, **workers, **scheduler, } @@ -417,7 +433,7 @@ def create_platform(install_airflow: bool = True, use_gpu: bool = False) -> None "-f", str(fp.name), "airflow", "apache-airflow/airflow", - "--version", " 1.9.0", + "--version", " 1.11.0", ] subprocess.run(commands) diff --git a/src/domino/client/domino_backend_client.py b/src/domino/client/domino_backend_client.py index 4c308e10..0551a355 100644 --- a/src/domino/client/domino_backend_client.py +++ b/src/domino/client/domino_backend_client.py @@ -31,15 +31,7 @@ def health_check(self) -> requests.Response: return response def get_piece_secrets(self, piece_repository_id: int, piece_name: str) -> requests.Response: - resource = f"/pieces-repositories/{piece_repository_id}/secrets/{piece_name}" - response = self.request( - method='get', - resource=resource - ) - return response - - def get_piece_repository(self, piece_repository_id: int) -> requests.Response: - resource = f"/pieces-repositories/{piece_repository_id}" + resource = f"/pieces-repositories/{piece_repository_id}/secrets/{piece_name}/secrets-values" response = self.request( method='get', resource=resource @@ -47,14 +39,14 @@ def get_piece_repository(self, piece_repository_id: int) -> requests.Response: return response def get_piece_repositories_from_workspace_id(self, params: dict) -> requests.Response: - resource = "/pieces-repositories" + resource = "/pieces-repositories/worker" response = self.request( method='get', resource=resource, params=params ) return response - + def check_create_airflow_connection(self, conn_id: str, conn_type: str): """ This should check if a specific Airflow connection exists and create it if it doesn't. diff --git a/src/domino/task.py b/src/domino/task.py index b36daa6f..af1acd7a 100644 --- a/src/domino/task.py +++ b/src/domino/task.py @@ -103,6 +103,7 @@ def _set_operator(self) -> BaseOperator: # - https://www.astronomer.io/guides/templating/ # - good example: https://github.com/apache/airflow/blob/main/tests/system/providers/cncf/kubernetes/example_kubernetes.py # - commands HAVE to go in a list object: https://stackoverflow.com/a/55149915/11483674 + return DominoKubernetesPodOperator( dag_id=self.dag_id, task_id=self.task_id, @@ -115,11 +116,12 @@ def _set_operator(self) -> BaseOperator: workflow_shared_storage=self.workflow_shared_storage, container_resources=self.container_resources, # ----------------- Kubernetes ----------------- - namespace='default', # TODO - separate namespace by User or Workspace? + namespace='default', image=self.piece.get("source_image"), image_pull_policy='IfNotPresent', name=f"airflow-worker-pod-{self.task_id}", startup_timeout_seconds=600, + annotations={"sidecar.istio.io/inject": "false"}, # TODO - remove this when istio is working with airflow k8s pod # cmds=["/bin/bash"], # arguments=["-c", "sleep 120;"], cmds=["domino"],