From c2e9a5f7c97719edc76d8040ed3b688681b9565d Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Wed, 27 Jan 2021 14:05:26 +0100 Subject: [PATCH 1/5] feat: add jsonschema validation for server options in values.yaml --- helm-chart/renku-notebooks/values.schema.json | 208 ++++++++++++++++++ 1 file changed, 208 insertions(+) create mode 100644 helm-chart/renku-notebooks/values.schema.json diff --git a/helm-chart/renku-notebooks/values.schema.json b/helm-chart/renku-notebooks/values.schema.json new file mode 100644 index 000000000..d8b9d9240 --- /dev/null +++ b/helm-chart/renku-notebooks/values.schema.json @@ -0,0 +1,208 @@ +{ + "$schema": "https://json-schema.org/draft-07/schema#", + "definitions": { + "informationAmount": { + "type": "string", + "pattern": "^(?:[1-9][0-9]*|[0-9]\\.[0-9]*)[EPTGMK][i]{0,1}$" + }, + "cpuRequest": { + "type": "number", + "exclusiveMinimum": 0.0, + "multipleOf": 0.001 + }, + "gpuRequest": { + "type": "integer", + "minimum": 0.0 + }, + "serverOption": { + "type": "object", + "properties": { + "order": { + "type": "integer", + "minimum": 1 + }, + "displayName": { + "type": "string" + } + }, + "required": [ + "order", + "displayName" + ] + }, + "serverOptionEnumStr": { + "allOf": [ + { + "$ref": "#/definitions/serverOption" + }, + { + "properties": { + "order": true, + "displayName": true, + "type": { + "type": "string", + "pattern": "^enum$" + }, + "default": { + "type": "string" + }, + "options": { + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1, + "uniqueItems": true + } + }, + "required": [ + "type", + "default", + "options" + ], + "additionalProperties": false + } + ] + }, + "serverOptionBool": { + "allOf": [ + { + "$ref": "#/definitions/serverOption" + }, + { + "properties": { + "order": true, + "displayName": true, + "type": { + "type": "string", + "pattern": "^boolean$" + }, + "default": { + "type": "boolean" + } + }, + "required": [ + "type", + "default" + ], + "additionalProperties": false + } + ] + }, + "serverOptionGpu": { + "allOf": [ + { + "$ref": "#/definitions/serverOption" + }, + { + "properties": { + "order": true, + "displayName": true, + "type": { + "type": "string", + "pattern": "^enum$" + }, + "default": { "$ref": "#/definitions/gpuRequest" }, + "options": { + "type": "array", + "items": { "$ref": "#/definitions/gpuRequest" }, + "minItems": 1, + "uniqueItems": true + } + }, + "required": [ + "type", + "default", + "options" + ], + "additionalProperties": false + } + ] + }, + "serverOptionCpu": { + "allOf": [ + { + "$ref": "#/definitions/serverOption" + }, + { + "properties": { + "order": true, + "displayName": true, + "type": { + "type": "string", + "pattern": "^enum$" + }, + "default": { "$ref": "#/definitions/cpuRequest" }, + "options": { + "type": "array", + "items": { "$ref": "#/definitions/cpuRequest" }, + "minItems": 1, + "uniqueItems": true + } + }, + "required": [ + "type", + "default", + "options" + ], + "additionalProperties": false + } + ] + }, + "serverOptionMemory": { + "allOf": [ + { + "$ref": "#/definitions/serverOptionEnumStr" + }, + { + "properties": { + "order": true, + "displayName": true, + "type": { + "type": "string", + "pattern": "^enum$" + }, + "default": { "$ref": "#/definitions/informationAmount" }, + "options": { + "type": "array", + "items": { "$ref": "#/definitions/informationAmount" }, + "minItems": 1, + "uniqueItems": true + } + }, + "required": [ + "type", + "default", + "options" + ], + "additionalProperties": false + } + ] + } + }, + "properties": { + "serverOptions": { + "description": "Options provided to the user in the UI when launching a server.", + "properties": { + "defaultUrl": { "$ref": "#/definitions/serverOptionEnumStr" }, + "cpu_request": { "$ref": "#/definitions/serverOptionCpu" }, + "mem_request": { "$ref": "#/definitions/serverOptionMemory" }, + "lfs_auto_fetch": { "$ref": "#/definitions/serverOptionBool" }, + "gpu_request": { "$ref": "#/definitions/serverOptionGpu" } + }, + "required": [ + "defaultUrl", + "cpu_request", + "mem_request", + "lfs_auto_fetch" + ], + "type": "object", + "additionalProperties": false + } + }, + "required": [ + "serverOptions" + ], + "title": "Values", + "type": "object" +} \ No newline at end of file From f3316c1332f35c75065786510de438fd3e38d2db Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Wed, 27 Jan 2021 18:38:38 +0100 Subject: [PATCH 2/5] feat: add specific schemas for server options in notebooks api --- renku_notebooks/api/custom_fields.py | 11 ++++ renku_notebooks/api/schemas.py | 83 +++++++++++++++++++++------- 2 files changed, 74 insertions(+), 20 deletions(-) diff --git a/renku_notebooks/api/custom_fields.py b/renku_notebooks/api/custom_fields.py index ae39aaa12..3a86d9ee0 100644 --- a/renku_notebooks/api/custom_fields.py +++ b/renku_notebooks/api/custom_fields.py @@ -1,6 +1,7 @@ from typing import List, Mapping, Any from marshmallow import fields from marshmallow.exceptions import ValidationError +import re class UnionField(fields.Field): @@ -48,3 +49,13 @@ def _deserialize( errors.append(error.messages) raise ValidationError(errors) + + +serverOptionCpuValue = fields.Number( + validate=lambda x: x > 0.0 and (x % 1 >= 0.001 or x % 1 == 0.0), required=True, +) +serverOptionMemoryValue = fields.String( + validate=lambda x: re.match(r"^(?:[1-9][0-9]*|[0-9]\.[0-9]*)[EPTGMK][i]{0,1}$", x) + is not None, + required=True, +) diff --git a/renku_notebooks/api/schemas.py b/renku_notebooks/api/schemas.py index 18307486f..1c788ed31 100644 --- a/renku_notebooks/api/schemas.py +++ b/renku_notebooks/api/schemas.py @@ -2,7 +2,18 @@ import collections from .. import config -from .custom_fields import UnionField +from .custom_fields import ( + serverOptionCpuValue, + serverOptionMemoryValue, +) + + +class LaunchNotebookRequestServerOptions(Schema): + defaultUrl = fields.String(required=True) + cpu_request = serverOptionCpuValue + mem_request = serverOptionMemoryValue + lfs_auto_fetch = fields.Bool(required=True) + gpu_request = fields.Integer(strict=True, validate=lambda x: x >= 0) class LaunchNotebookRequest(Schema): @@ -14,9 +25,7 @@ class LaunchNotebookRequest(Schema): commit_sha = fields.Str(required=True) notebook = fields.Str(missing=None) image = fields.Str(missing=None) - server_options = fields.Dict( - keys=fields.Str(), missing={}, data_key="serverOptions" - ) + server_options = fields.Nested(LaunchNotebookRequestServerOptions(), missing={}) def flatten_dict(d, parent_key="", sep="."): @@ -127,20 +136,54 @@ class FailedParsing(Schema): ) -class ServerOptionsOption(Schema): +class ServerOptionBase(Schema): + displayName = fields.Str(required=True) + order = fields.Int(required=True) + type = fields.String(validate=lambda x: x in ["boolean", "enum"], required=True,) + + +class ServerOptionCpu(ServerOptionBase): """The schema used to describe a single option for the server_options endpoint.""" - default = UnionField( - [ - fields.Str(required=True), - fields.Number(required=True), - fields.Bool(required=True), - ] + default = serverOptionCpuValue + options = fields.List( + serverOptionCpuValue, validate=lambda x: len(x) >= 1, required=True ) - displayName = fields.Str(required=True) - order = fields.Int(required=True) - type = fields.Str(required=True) - options = fields.List(UnionField([fields.Str(), fields.Number()])) + + +class ServerOptionMemory(ServerOptionBase): + """The schema used to describe a single option for the server_options endpoint.""" + + default = serverOptionMemoryValue + options = fields.List( + serverOptionMemoryValue, validate=lambda x: len(x) >= 1, required=True + ) + + +class ServerOptionGpu(ServerOptionBase): + """The schema used to describe a single option for the server_options endpoint.""" + + default = fields.Integer(strict=True, validate=lambda x: x >= 0, required=True) + options = fields.List( + fields.Integer(strict=True, validate=lambda x: x >= 0), + validate=lambda x: len(x) >= 1, + required=True, + ) + + +class ServerOptionString(ServerOptionBase): + """The schema used to describe a single option for the server_options endpoint.""" + + default = fields.String(required=True) + options = fields.List( + fields.String(), validate=lambda x: len(x) >= 1, required=True + ) + + +class ServerOptionBool(ServerOptionBase): + """The schema used to describe a single option for the server_options endpoint.""" + + default = fields.Bool(required=True) class ServerOptions(Schema): @@ -149,11 +192,11 @@ class ServerOptions(Schema): launching a jupyterhub server. """ - cpu_request = fields.Nested(ServerOptionsOption()) - defaultUrl = fields.Nested(ServerOptionsOption()) - gpu_request = fields.Nested(ServerOptionsOption()) - lfs_auto_fetch = fields.Nested(ServerOptionsOption()) - mem_request = fields.Nested(ServerOptionsOption()) + cpu_request = fields.Nested(ServerOptionCpu(), required=True) + defaultUrl = fields.Nested(ServerOptionString(), required=True) + gpu_request = fields.Nested(ServerOptionGpu()) + lfs_auto_fetch = fields.Nested(ServerOptionBool(), required=True) + mem_request = fields.Nested(ServerOptionMemory(), required=True) class ServerLogs(Schema): From 7a9f57555d51554006bd052c16cacfada9d83b60 Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Wed, 27 Jan 2021 18:54:37 +0100 Subject: [PATCH 3/5] fix: fix field name is schema --- renku_notebooks/api/schemas.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/renku_notebooks/api/schemas.py b/renku_notebooks/api/schemas.py index 1c788ed31..d327cacee 100644 --- a/renku_notebooks/api/schemas.py +++ b/renku_notebooks/api/schemas.py @@ -25,7 +25,9 @@ class LaunchNotebookRequest(Schema): commit_sha = fields.Str(required=True) notebook = fields.Str(missing=None) image = fields.Str(missing=None) - server_options = fields.Nested(LaunchNotebookRequestServerOptions(), missing={}) + server_options = fields.Nested( + LaunchNotebookRequestServerOptions(), missing={}, data_key="serverOptions" + ) def flatten_dict(d, parent_key="", sep="."): From b192dcc9b848a100bbda90006664d50af0f277ae Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Tue, 2 Feb 2021 21:38:57 +0100 Subject: [PATCH 4/5] fix: validate server options values are valid when launching notebook --- renku_notebooks/api/notebooks.py | 14 +++----------- renku_notebooks/api/schemas.py | 28 +++++++++++++++++++++++++++- renku_notebooks/util/misc.py | 11 +++++++++++ tests/test_notebook.py | 19 +++++++++++++++++++ 4 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 renku_notebooks/util/misc.py diff --git a/renku_notebooks/api/notebooks.py b/renku_notebooks/api/notebooks.py index cd2a104a7..7cc9b0f49 100644 --- a/renku_notebooks/api/notebooks.py +++ b/renku_notebooks/api/notebooks.py @@ -54,6 +54,7 @@ ServerOptions, FailedParsing, ) +from ..util.misc import read_server_options_file bp = Blueprint("notebooks_blueprint", __name__, url_prefix=config.SERVICE_PREFIX) @@ -140,7 +141,7 @@ def launch_notebook( # process the server options # server options from system configuration - server_options_defaults = _read_server_options_file() + server_options_defaults = read_server_options_file() # process the requested options and set others to defaults from config server_options.setdefault( @@ -351,7 +352,7 @@ def stop_server(user, forced, server_name): @authenticated def server_options(user): """Return a set of configurable server options.""" - server_options = _read_server_options_file() + server_options = read_server_options_file() # TODO: append image-specific options to the options json return jsonify(server_options) @@ -396,12 +397,3 @@ def server_logs(user, server_name): jsonify({"messages": {"error": "Cannot find server"}}), 404 ) return response - - -def _read_server_options_file(): - server_options_file = os.getenv( - "NOTEBOOKS_SERVER_OPTIONS_PATH", "/etc/renku-notebooks/server_options.json" - ) - with open(server_options_file) as f: - server_options = json.load(f) - return server_options diff --git a/renku_notebooks/api/schemas.py b/renku_notebooks/api/schemas.py index d327cacee..019f1ce06 100644 --- a/renku_notebooks/api/schemas.py +++ b/renku_notebooks/api/schemas.py @@ -1,4 +1,11 @@ -from marshmallow import Schema, fields, post_load, post_dump +from marshmallow import ( + Schema, + fields, + post_load, + post_dump, + validates_schema, + ValidationError, +) import collections from .. import config @@ -6,6 +13,7 @@ serverOptionCpuValue, serverOptionMemoryValue, ) +from ..util.misc import read_server_options_file class LaunchNotebookRequestServerOptions(Schema): @@ -15,6 +23,24 @@ class LaunchNotebookRequestServerOptions(Schema): lfs_auto_fetch = fields.Bool(required=True) gpu_request = fields.Integer(strict=True, validate=lambda x: x >= 0) + @validates_schema + def validate_server_options(self, data, **kwargs): + server_options = read_server_options_file() + for option in data.keys(): + if server_options.get(option, {}).get("type", None) == "boolean": + continue # boolean options are already validated by marshmallow + if server_options.get( # validate options that can have a set of values + option, {} + ).get("options", None) is None or data[option] not in server_options.get( + option, {} + ).get( + "options", [] + ): + raise ValidationError( + f"The value {data[option]} for sever option {option} is not valid, " + f"has to be one of {server_options.get(option, {}).get('options', [])}" + ) + class LaunchNotebookRequest(Schema): """Used to validate the requesting for launching a jupyterhub server""" diff --git a/renku_notebooks/util/misc.py b/renku_notebooks/util/misc.py new file mode 100644 index 000000000..3cbe59cf7 --- /dev/null +++ b/renku_notebooks/util/misc.py @@ -0,0 +1,11 @@ +import json +import os + + +def read_server_options_file(): + server_options_file = os.getenv( + "NOTEBOOKS_SERVER_OPTIONS_PATH", "/etc/renku-notebooks/server_options.json" + ) + with open(server_options_file) as f: + server_options = json.load(f) + return server_options diff --git a/tests/test_notebook.py b/tests/test_notebook.py index d3ec6f5ae..d3c2271cf 100644 --- a/tests/test_notebook.py +++ b/tests/test_notebook.py @@ -177,6 +177,25 @@ def test_users_with_no_developer_access_can_create_notebooks( assert response.status_code == 202 or response.status_code == 201 +def test_launching_notebook_with_invalid_server_options( + client, gitlab, make_all_images_valid, kubernetes_client_empty, +): + response = create_notebook( + client, + **{ + **DEFAULT_PAYLOAD, + "serverOptions": { + "cpu_request": 20, + "defaultUrl": "some_url", + "gpu_request": 20, + "lfs_auto_fetch": True, + "mem_request": "100G", + }, + }, + ) + assert response.status_code == 422 + + def test_getting_logs_for_nonexisting_notebook_returns_404( client, kubernetes_client_empty ): From 8f02bc1ba55a0cbc1a9c34778dbd35987454a33b Mon Sep 17 00:00:00 2001 From: Tasko Olevski Date: Wed, 3 Feb 2021 15:32:07 +0100 Subject: [PATCH 5/5] fix: simplify server options check logic --- renku_notebooks/api/schemas.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/renku_notebooks/api/schemas.py b/renku_notebooks/api/schemas.py index 019f1ce06..28da25cee 100644 --- a/renku_notebooks/api/schemas.py +++ b/renku_notebooks/api/schemas.py @@ -27,18 +27,15 @@ class LaunchNotebookRequestServerOptions(Schema): def validate_server_options(self, data, **kwargs): server_options = read_server_options_file() for option in data.keys(): - if server_options.get(option, {}).get("type", None) == "boolean": + if option not in server_options.keys(): + continue # presence of option keys are already handled by marshmallow + if server_options[option]["type"] == "boolean": continue # boolean options are already validated by marshmallow - if server_options.get( # validate options that can have a set of values - option, {} - ).get("options", None) is None or data[option] not in server_options.get( - option, {} - ).get( - "options", [] - ): + if data[option] not in server_options[option]["options"]: + # validate options that can have a set of values against allowed values raise ValidationError( f"The value {data[option]} for sever option {option} is not valid, " - f"has to be one of {server_options.get(option, {}).get('options', [])}" + f"it has to be one of {server_options[option]['options']}" )