Add environment option for workspaces and external graders (#4852)

* Add `environment` option for workspaces * Add `environment` option for external graders * Fix optional flag for typechecker * Add `external_grading_environment` to pg schema * Change `environment` from array to object * Fix pg schema * Fix sync fallback * Apply suggestions from code review Co-authored-by: Nathan Walters <nathan@prairielearn.com> * Add `environment` to grader host * Apply suggestions from code review Co-authored-by: Nathan Walters <nathan@prairielearn.com> Co-authored-by: Nathan Walters <nathan@prairielearn.com>
PrairieLearn · Oct 6, 2021 · 2727ae4 · 2727ae4
1 parent 9e940a6
commit 2727ae4
Show file tree

Hide file tree

Showing 15 changed files with 46 additions and 6 deletions.
diff --git a/database/tables/questions.pg b/database/tables/questions.pg
@@ -7,6 +7,7 @@ columns
     external_grading_enable_networking: boolean
     external_grading_enabled: boolean default false
     external_grading_entrypoint: text
+    external_grading_environment: jsonb not null default '{}'::jsonb
     external_grading_files: text[] default ARRAY[]::text[]
     external_grading_image: text
     external_grading_timeout: integer
@@ -27,6 +28,7 @@ columns
     uuid: uuid
     workspace_args: text
     workspace_enable_networking: boolean
+    workspace_environment: jsonb not null default '{}'::jsonb
     workspace_graded_files: text[] default ARRAY[]::text[]
     workspace_home: text
     workspace_image: text

diff --git a/docs/externalGrading.md b/docs/externalGrading.md
@@ -70,6 +70,8 @@ External grading configuration is done on a per-question basis. All configuratio
 
 * `enableNetworking`: Allows the container to access the public internet. This is disabled by default to make secure, isolated execution the default behavior.
 
+* `environment`: Environment variables to set inside the grading container. Set variables using `{"VAR": "value", ...}`, and unset variables using `{"VAR": null}` (no quotes around `null`). This property is optional.
+
 Here's an example of a complete `externalGradingOptions` portion of a question's `info.json`:
 
 ```json

diff --git a/docs/workspaces/index.md b/docs/workspaces/index.md
@@ -65,6 +65,7 @@ The question's `info.json` should set the `singleVariant` and `workspaceOptions`
     * `syncIgnore` (optional, default none): list of files or directories that will be excluded from sync
     * `rewriteUrl` (optional, default true): if true, the URL will be rewritten such that the workspace container will see all requests as originating from /
     * `enableNetworking` (optional, default false): whether the workspace should be allowed to connect to the public internet. This is disabled by default to make secure, isolated execution the default behavior. This restriction is not enforced when running PrairieLearn in local development mode. It is strongly recommended to use the default (no networking) for exam questions, because network access can be used to enable cheating. Only enable networking for homework questions, and only if it is strictly required, for example for downloading data from the internet.
+    * `environment` (optional, default `{}`): environment variables to set inside the workspace container. Set variables using `{"VAR": "value", ...}`, and unset variables using `{"VAR": null}` (no quotes around `null`).
 
 #### `info.json` for ungraded workspace
 

diff --git a/grader_host/index.js b/grader_host/index.js
@@ -361,6 +361,7 @@ function runJob(info, callback) {
                 entrypoint,
                 timeout,
                 enableNetworking,
+                environment,
             },
         },
         initFiles: {
@@ -372,6 +373,7 @@ function runJob(info, callback) {
     let jobTimeout = timeout || 30;
     let globalJobTimeout = jobTimeout * 2;
     let jobEnableNetworking = enableNetworking || false;
+    let jobEnvironment = environment || {};
 
     let jobFailed = false;
     let globalJobTimeoutCleared = false;
@@ -394,6 +396,8 @@ function runJob(info, callback) {
         (callback) => {
             docker.createContainer({
                 Image: runImage,
+                // Convert {key: 'value'} to ['key=value'] and {key: null} to ['key'] for Docker API
+                Env: Object.entries(jobEnvironment).map(([k, v]) => v === null ? k : `${k}=${v}`),
                 AttachStdout: true,
                 AttachStderr: true,
                 Tty: true,

diff --git a/lib/externalGraderLocal.js b/lib/externalGraderLocal.js
@@ -89,6 +89,8 @@ class Grader {
             (callback) => {
                 docker.createContainer({
                     Image: question.external_grading_image,
+                    // Convert {key: 'value'} to ['key=value'] and {key: null} to ['key'] for Docker API
+                    Env: Object.entries(question.external_grading_environment).map(([k, v]) => v === null ? k : `${k}=${v}`),
                     AttachStdout: true,
                     AttachStderr: true,
                     Tty: true,

diff --git a/lib/externalGraderSqs.js b/lib/externalGraderSqs.js
@@ -118,6 +118,7 @@ function sendJobToQueue(jobId, question, config, callback) {
                 csrfToken: csrf.generateToken({url: '/pl/webhooks/grading'}, config.secretKey),
                 timeout: question.external_grading_timeout || config.externalGradingDefaultTimeout,
                 enableNetworking: question.external_grading_enable_networking || false,
+                environment: question.external_grading_environment || {},
             };
             const params = {
                 QueueUrl: QUEUE_URL,

diff --git a/migrations/243_questions__workspace_environment__add.sql b/migrations/243_questions__workspace_environment__add.sql
@@ -0,0 +1 @@
+ALTER TABLE questions ADD COLUMN workspace_environment jsonb not null default '{}'::jsonb;
diff --git a/migrations/244_questions__external_grading_environment__add.sql b/migrations/244_questions__external_grading_environment__add.sql
@@ -0,0 +1 @@
+ALTER TABLE questions ADD COLUMN external_grading_environment jsonb not null default '{}'::jsonb;
diff --git a/schemas/schemas/infoQuestion.json b/schemas/schemas/infoQuestion.json
@@ -136,6 +136,10 @@
                 "enableNetworking": {
                     "description": "Whether the grading containers should have network access. Access is disabled by default.",
                     "type": "boolean"
+                },
+                "environment": {
+                    "description": "Environment variables to set inside the grading container.",
+                    "type": "object"
                 }
             }
         },
@@ -269,6 +273,10 @@
                 "enableNetworking": {
                     "description": "Whether the workspace should have network access. Access is disabled by default.",
                     "type": "boolean"
+                },
+                "environment": {
+                    "description": "Environment variables to set inside the workspace container.",
+                    "type": "object"
                 }
             }
         }

diff --git a/sprocs/sync_questions.sql b/sprocs/sync_questions.sql
@@ -128,6 +128,7 @@ BEGIN
         external_grading_entrypoint = src.data->>'external_grading_entrypoint',
         external_grading_timeout = (src.data->>'external_grading_timeout')::integer,
         external_grading_enable_networking = (src.data->>'external_grading_enable_networking')::boolean,
+        external_grading_environment = (src.data->>'external_grading_environment')::jsonb,
         dependencies = (src.data->>'dependencies')::jsonb,
         workspace_image = src.data->>'workspace_image',
         workspace_port = (src.data->>'workspace_port')::integer,
@@ -137,6 +138,7 @@ BEGIN
         workspace_sync_ignore = jsonb_array_to_text_array(src.data->'workspace_sync_ignore'),
         workspace_url_rewrite = (src.data->>'workspace_url_rewrite')::boolean,
         workspace_enable_networking = (src.data->>'workspace_enable_networking')::boolean,
+        workspace_environment = (src.data->>'workspace_environment')::jsonb,
         sync_errors = NULL,
         sync_warnings = src.warnings
     FROM

diff --git a/sync/course-db.js b/sync/course-db.js
@@ -243,6 +243,7 @@ const FILE_UUID_REGEX = /"uuid":\s*"([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4
  * @property {string[]} serverFilesCourse
  * @property {number} timeout
  * @property {boolean} enableNetworking
+ * @property {Record<string, string | null>} environment
  */
 
 /**
@@ -255,6 +256,7 @@ const FILE_UUID_REGEX = /"uuid":\s*"([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4
  * @property {string[]} syncIgnore
  * @property {string} rewriteUrl
  * @property {boolean} enableNetworking
+ * @property {Record<string, string | null>} environment
  */
 
  /**

diff --git a/sync/fromDisk/questions.js b/sync/fromDisk/questions.js
@@ -36,6 +36,7 @@ function getParamsForQuestion(q) {
         external_grading_entrypoint: (q.externalGradingOptions && q.externalGradingOptions.entrypoint),
         external_grading_timeout: (q.externalGradingOptions && q.externalGradingOptions.timeout),
         external_grading_enable_networking: (q.externalGradingOptions && q.externalGradingOptions.enableNetworking),
+        external_grading_environment: q.externalGradingOptions?.environment ?? {},
         dependencies: q.dependencies || {},
         workspace_image: (q.workspaceOptions && q.workspaceOptions.image),
         workspace_port: (q.workspaceOptions && q.workspaceOptions.port),
@@ -45,6 +46,7 @@ function getParamsForQuestion(q) {
         workspace_sync_ignore: (q.workspaceOptions && q.workspaceOptions.syncIgnore),
         workspace_url_rewrite: (q.workspaceOptions && q.workspaceOptions.rewriteUrl),
         workspace_enable_networking: (q.workspaceOptions && q.workspaceOptions.enableNetworking),
+        workspace_environment: q.workspaceOptions?.environment ?? {},
     };
 }
 

diff --git a/testCourse/questions/workspace/info.json b/testCourse/questions/workspace/info.json
@@ -4,7 +4,8 @@
     "topic": "Workspace",
     "tags": [
         "su20",
-        "nnytko2"
+        "nnytko2",
+        "tyang15"
     ],
     "type": "v3",
     "workspaceOptions": {
@@ -14,6 +15,10 @@
         "gradedFiles": [
             "starter_code.h",
             "starter_code.c"
-        ]
+        ],
+        "environment": {
+            "FOO": "bar baz",
+            "ILL": "ini"
+        }
     }
 }
diff --git a/tests/sync/util.js b/tests/sync/util.js
@@ -147,6 +147,7 @@ const syncFromDisk = require('../../sync/syncFromDisk');
  * @property {string[]=} serverFilesCourse
  * @property {number=} timeout
  * @property {boolean=} enableNetworking
+ * @property {Record<string, string | null>=} environment
  */
 
 /**
@@ -159,6 +160,7 @@ const syncFromDisk = require('../../sync/syncFromDisk');
  * @property {string[]} syncIgnore
  * @property {string} rewriteUrl
  * @property {boolean=} enableNetworking
+ * @property {Record<string, string | null>=} environment
  */
 
  /**

diff --git a/workspace_host/interface.js b/workspace_host/interface.js
@@ -623,6 +623,11 @@ const _checkServerAsync = util.promisify(_checkServer);
  */
 async function _getWorkspaceSettingsAsync(workspace_id) {
     const result = await sqldb.queryOneRowAsync(sql.select_workspace_settings, { workspace_id });
+    const workspace_environment = result.rows[0].workspace_environment || {};
+
+    // Set base URL needed by certain workspaces (e.g., jupyterlab, rstudio)
+    workspace_environment['WORKSPACE_BASE_URL'] = `/pl/workspace/${workspace_id}/container/`;
+
     const settings = {
         workspace_image: result.rows[0].workspace_image,
         workspace_port: result.rows[0].workspace_port,
@@ -631,6 +636,8 @@ async function _getWorkspaceSettingsAsync(workspace_id) {
         workspace_args: result.rows[0].workspace_args || '',
         workspace_sync_ignore: result.rows[0].workspace_sync_ignore || [],
         workspace_enable_networking: !!result.rows[0].workspace_enable_networking,
+        // Convert {key: 'value'} to ['key=value'] and {key: null} to ['key'] for Docker API
+        workspace_environment: Object.entries(workspace_environment).map(([k, v]) => v === null ? k : `${k}=${v}`),
     };
 
     if (config.cacheImageRegistry) {
@@ -984,7 +991,7 @@ function _createContainer(workspace, callback) {
     debug(`Exposed port: ${workspace.settings.workspace_port}`);
     debug(`Networking enabled: ${workspace.settings.workspace_enable_networking}`);
     debug(`Network mode: ${networkMode}`);
-    debug(`Env vars: WORKSPACE_BASE_URL=/pl/workspace/${workspace.id}/container/`);
+    debug(`Env vars: ${workspace.settings.workspace_environment}`);
     debug(`User binding: ${config.workspaceJobsDirectoryOwnerUid}:${config.workspaceJobsDirectoryOwnerGid}`);
     debug(`Port binding: ${workspace.settings.workspace_port}:${workspace.launch_port}`);
     debug(`Volume mount: ${workspacePath}:${containerPath}`);
@@ -1021,9 +1028,7 @@ function _createContainer(workspace, callback) {
                 ExposedPorts: {
                     [`${workspace.settings.workspace_port}/tcp`]: {},
                 },
-                Env: [
-                    `WORKSPACE_BASE_URL=/pl/workspace/${workspace.id}/container/`,
-                ],
+                Env: workspace.settings.workspace_environment,
                 User: `${config.workspaceJobsDirectoryOwnerUid}:${config.workspaceJobsDirectoryOwnerGid}`,
                 HostConfig: {
                     PortBindings: {