Skip to content

Commit

Permalink
fix: check builder exists on create (#847)
Browse files Browse the repository at this point in the history
* feat: add parameter `BUILDER_ENABLED` in `settings.server`

Signed-off-by: Guilhem Barthés <[email protected]>

* chore: add `BUILDER_ENABLED` in chart

Signed-off-by: Guilhem Barthés <[email protected]>

* feat: add check for `BUILDER_ENABLED` on `function_create`

Signed-off-by: Guilhem Barthés <[email protected]>

* docs: changelog

Signed-off-by: Guilhem Barthés <[email protected]>

* fix: add var in charts

Signed-off-by: Guilhem Barthés <[email protected]>

* fix: add decorator on test

Signed-off-by: Guilhem Barthés <[email protected]>

* fix: `INSTALLED_APPS` not defined due to import order

Signed-off-by: Guilhem Barthés <[email protected]>

---------

Signed-off-by: Guilhem Barthés <[email protected]>
  • Loading branch information
guilhem-barthes authored Mar 6, 2024
1 parent b650487 commit 26e38bc
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Ensure that we always use `safezip` and `tarsafe` to avoid path traversal attack ([#845](https://github.com/Substra/substra-backend/pull/845))
- Add timeout on all OIDC requests (value set with `OIDC_TIMOUT`, and default to `HTTP_CLIENT_TIMEOUT_SECONDS`) ([#846](https://github.com/Substra/substra-backend/pull/846))
- Check if builder is enabled during function registration ([#847](https://github.com/Substra/substra-backend/pull/847))

### Changed

Expand Down
41 changes: 41 additions & 0 deletions backend/api/tests/views/test_views_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import tempfile
from unittest import mock

import pytest
from django.conf import settings
from django.test import override_settings
from django.urls import reverse
from django.utils.http import urlencode
Expand Down Expand Up @@ -36,6 +38,7 @@ class FunctionViewTests(APITestCase):
client_class = AuthenticatedClient

def setUp(self):
settings.BUILDER_ENABLED = True
if not os.path.exists(MEDIA_ROOT):
os.makedirs(MEDIA_ROOT)

Expand Down Expand Up @@ -646,3 +649,41 @@ def test_function_update(self):
with mock.patch.object(OrchestratorClient, "update_function", side_effect=error):
response = self.client.put(url, data=data, format="json")
self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR)


@pytest.mark.django_db
def test_function_create_no_builder(authenticated_client):
settings.BUILDER_ENABLED = False

function_path = os.path.join(FIXTURE_PATH, "function.zip")
description_path = os.path.join(FIXTURE_PATH, "description.md")
data = {
"json": json.dumps(
{
"name": "Logistic regression",
"metric_key": "some key",
"permissions": {
"public": True,
"authorized_ids": ["MyOrg1MSP"],
},
"inputs": {
"datasamples": {"kind": "ASSET_DATA_SAMPLE", "optional": False, "multiple": True},
"opener": {"kind": "ASSET_DATA_MANAGER", "optional": False, "multiple": False},
"model": {"kind": "ASSET_MODEL", "optional": True, "multiple": False},
},
"outputs": {
"model": {"kind": "ASSET_MODEL", "multiple": False},
},
}
),
"file": open(function_path, "rb"),
"description": open(description_path, "rb"),
}

url = reverse("api:function-list")
response = authenticated_client.post(url, data=data, format="multipart")

assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY

data["file"].close()
data["description"].close()
4 changes: 4 additions & 0 deletions backend/api/views/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ class FunctionViewSet(
FunctionViewSetConfig, mixins.RetrieveModelMixin, mixins.ListModelMixin, mixins.CreateModelMixin, GenericViewSet
):
def create(self, request, *args, **kwargs):
if not settings.BUILDER_ENABLED:
return ApiResponse(
{"detail": "No builder available on this organization"}, status=status.HTTP_422_UNPROCESSABLE_ENTITY
)
return create(request, self.basename, lambda data: self.get_success_headers(data))

def update(self, request, *args, **kwargs):
Expand Down
6 changes: 6 additions & 0 deletions backend/backend/settings/server/common.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import os

from ..deps.ledger import *
from ..deps.orchestrator import *

BUILDER_ENABLED = to_bool(os.getenv("BUILDER_ENABLED", "false"))
3 changes: 1 addition & 2 deletions backend/backend/settings/server/dev.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from ..deps.ledger import *
from ..deps.orchestrator import *
from ..dev import *
from .common import *

INSTALLED_APPS += ["organization_register"]
3 changes: 1 addition & 2 deletions backend/backend/settings/server/prod.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from ..deps.ledger import *
from ..deps.orchestrator import *
from ..prod import *
from .common import *

INSTALLED_APPS += ["organization_register"]
5 changes: 5 additions & 0 deletions charts/substra-backend/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
# Changelog

## [24.3.2] - 2024-03-06

### Added

- Env var `BUILDER_ENABLED` in server

## [24.3.1] - 2024-03-06

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion charts/substra-backend/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: v2
name: substra-backend
home: https://github.com/Substra
version: 24.3.1
version: 24.3.2
appVersion: 0.43.0
kubeVersion: ">= 1.19.0-0"
description: Main package for Substra
Expand Down
2 changes: 2 additions & 0 deletions charts/substra-backend/templates/configmap-settings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ data:
DEFAULT_DOMAIN: {{ .Values.server.defaultDomain | quote }}
COMMON_HOST_DOMAIN: {{ .Values.server.commonHostDomain | quote }}

BUILDER_ENABLED: {{ .Values.builder.enabled | quote }}

COMPUTE_POD_RUN_AS_USER: {{ .Values.worker.computePod.securityContext.runAsUser | quote}}
COMPUTE_POD_RUN_AS_GROUP: {{ .Values.worker.computePod.securityContext.runAsGroup | quote }}
COMPUTE_POD_FS_GROUP: {{ .Values.worker.computePod.securityContext.fsGroup | quote }}
Expand Down

0 comments on commit 26e38bc

Please sign in to comment.