Merge branch 'main' into 517-avd-workspace-service

.devcontainer/Dockerfile

@@ -75,7 +75,7 @@ COPY ["e2e_tests/requirements.txt", "/tmp/pip-tmp/e2e_tests/"]
 RUN pip3 --disable-pip-version-check --no-cache-dir install -r /tmp/pip-tmp/requirements.txt && rm -rf /tmp/pip-tmp
 
 # Install azure-cli
-ARG AZURE_CLI_VERSION=2.29.2-1~buster
+ARG AZURE_CLI_VERSION=2.36.0-1~buster
 COPY .devcontainer/scripts/azure-cli.sh /tmp/
 RUN export AZURE_CLI_VERSION=${AZURE_CLI_VERSION} \
     && /tmp/azure-cli.sh

.github/linters/.tflint.hcl

@@ -0,0 +1,8 @@
+config {
+  module = true
+  force = false
+}
+
+plugin "azurerm" {
+    enabled = true
+}

.github/scripts/build.js

@@ -17,19 +17,6 @@ async function getCommandFromComment({ core, context, github }) {
   const runId = context.runId;
   const prAuthorUsername = context.payload.issue.user.login;
 
-  // only allow actions for users with write access
-  if (!await userHasWriteAccessToRepo({ core, github }, commentUsername, repoOwner, repoName)) {
-    core.notice("Command: none - user doesn't have write permission]");
-    await github.rest.issues.createComment({
-      owner: repoOwner,
-      repo: repoName,
-      issue_number: prNumber,
-      body: `Sorry, @${commentUsername}, only users with write access to the repo can run pr-bot commands.`
-    });
-    logAndSetOutput(core, "command", "none");
-    return "none";
-  }
-
   // Determine PR SHA etc
   const ciGitRef = getRefForPr(prNumber);
   logAndSetOutput(core, "ciGitRef", ciGitRef);
@@ -65,7 +52,20 @@ async function getCommandFromComment({ core, context, github }) {
   let command = "none";
   const trimmedFirstLine = commentFirstLine.trim();
   if (trimmedFirstLine[0] === "/") {
-    const parts = trimmedFirstLine.split(' ').filter(p=>p !== '');
+    // only allow actions for users with write access
+    if (!await userHasWriteAccessToRepo({ core, github }, commentUsername, repoOwner, repoName)) {
+      core.notice("Command: none - user doesn't have write permission]");
+      await github.rest.issues.createComment({
+        owner: repoOwner,
+        repo: repoName,
+        issue_number: prNumber,
+        body: `Sorry, @${commentUsername}, only users with write access to the repo can run pr-bot commands.`
+      });
+      logAndSetOutput(core, "command", "none");
+      return "none";
+    }
+
+    const parts = trimmedFirstLine.split(' ').filter(p => p !== '');
     const commandText = parts[0];
     switch (commandText) {
       case "/test":

.github/scripts/build.test.js

@@ -53,27 +53,49 @@ describe('getCommandFromComment', () => {
   }
 
   describe('with non-contributor', () => {
-    test(`for '/test' should return 'none'`, async () => {
-      const context = createCommentContext({
-        username: 'non-contributor',
-        body: '/test',
+    describe(`for '/test`, () => {
+      test(`should return 'none'`, async () => {
+        const context = createCommentContext({
+          username: 'non-contributor',
+          body: '/test',
+        });
+        const command = await getCommandFromComment({ core, context, github });
+        expect(outputFor(mockCoreSetOutput, 'command')).toBe('none');
       });
-      const command = await getCommandFromComment({ core, context, github });
-      expect(outputFor(mockCoreSetOutput, 'command')).toBe('none');
-    });
 
-    test(`should add a comment indicating that the user cannot run commands`, async () => {
-      const context = createCommentContext({
-        username: 'non-contributor',
-        body: '/test',
-        pullRequestNumber: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
+      test(`should add a comment indicating that the user cannot run commands`, async () => {
+        const context = createCommentContext({
+          username: 'non-contributor',
+          body: '/test',
+          pullRequestNumber: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
+        });
+        await getCommandFromComment({ core, context, github });
+        expect(mockGithubRestIssuesCreateComment).toHaveComment({
+          owner: 'someOwner',
+          repo: 'someRepo',
+          issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
+          bodyMatcher: /Sorry, @non-contributor, only users with write access to the repo can run pr-bot commands./
+        });
       });
-      await getCommandFromComment({ core, context, github });
-      expect(mockGithubRestIssuesCreateComment).toHaveComment({
-        owner: 'someOwner',
-        repo: 'someRepo',
-        issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
-        bodyMatcher: /Sorry, @non-contributor, only users with write access to the repo can run pr-bot commands./
+    });
+    describe(`for 'non-command`, () => {
+      test(`should return 'none'`, async () => {
+        const context = createCommentContext({
+          username: 'non-contributor',
+          body: 'non-command',
+        });
+        const command = await getCommandFromComment({ core, context, github });
+        expect(outputFor(mockCoreSetOutput, 'command')).toBe('none');
+      });
+
+      test(`should not add a comment`, async () => {
+        const context = createCommentContext({
+          username: 'non-contributor',
+          body: 'non-command',
+          pullRequestNumber: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
+        });
+        await getCommandFromComment({ core, context, github });
+        expect(mockGithubRestIssuesCreateComment).not.toHaveBeenCalled();
       });
     });
 

.github/workflows/build_validation_develop.yml

@@ -42,14 +42,17 @@ jobs:
         if: ${{ steps.filter.outputs.terraform == 'true' }}
         run: |
           find . -type d -name 'terraform' -not -path '*cnab*' -print0 \
-          | xargs -0 -I{} sh -c 'echo "***** Validating: {} *****"; \
+          | xargs -0 -I{} sh -c 'echo "***** Validating: {} *****"; \https://github.com/github/super-linter/issues/2433
           terraform -chdir={} init -backend=false; terraform -chdir={} validate'
 
       - name: Lint code base
         # the slim image is 2GB smaller and we don't use the extra stuff
         # Moved this after the Terraform checks above due something similar to this issue: https://github.com/github/super-linter/issues/2433
-        uses: github/super-linter/slim@v4
+        uses: github/super-linter/slim@v4.9.3
         env:
+          # Until https://github.com/github/super-linter/commit/ec0662756da93f1e3aad4df049712df7d764d143 is released
+          # we need to set the correct plugin directory (which is incorrectly set to github/home/.tflint.d/plugins by default)
+          TFLINT_PLUGIN_DIR: "/root/.tflint.d/plugins"
           VALIDATE_ALL_CODEBASE: false
           DEFAULT_BRANCH: main
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
@@ -61,6 +64,7 @@ jobs:
           JAVA_FILE_NAME: checkstyle.xml
           VALIDATE_BASH: true
           VALIDATE_BASH_EXEC: true
+          VALIDATE_DOCKERFILE_HADOLINT: true
           # https://github.com/microsoft/AzureTRE/issues/1723 tracks re-instating VALIDATE_GITHUB_ACTIONS
           # Note: in the meantime, the `.github/scripts/run-test.sh` script includes the `actionlint` checks)
           # VALIDATE_GITHUB_ACTIONS: true

Makefile

@@ -241,6 +241,7 @@ lint:
 		-e VALIDATE_BASH=true \
 		-e VALIDATE_BASH_EXEC=true \
 		-e VALIDATE_GITHUB_ACTIONS=true \
+		-e VALIDATE_DOCKERFILE_HADOLINT=true \
 		-v $${LOCAL_WORKSPACE_FOLDER}:/tmp/lint \
 		github/super-linter:slim-v4
 
@@ -319,6 +320,11 @@ shared_service_bundle = $(MAKE) bundle-build DIR=./templates/shared_services/$(1
 	&& $(MAKE) bundle-publish DIR=./templates/shared_services/$(1)/ \
 	&& $(MAKE) bundle-register DIR="./templates/shared_services/$(1)" BUNDLE_TYPE=shared_service
 
+user_resource_bundle = $(MAKE) bundle-build DIR=./templates/workspace_services/$(1)/user_resources/$(2)/ \
+	&& $(MAKE) bundle-publish DIR=./templates/workspace_services/$(1)/user_resources/$(2) \
+	&& $(MAKE) bundle-register DIR="./templates/workspace_services/$(1)/user_resources/$(2)" BUNDLE_TYPE=user_resource WORKSPACE_SERVICE_NAME=tre-service-$(1)
+
+
 deploy-shared-service:
 	@# NOTE: ACR_NAME below comes from the env files, so needs the double '$$'. Others are set on command execution and don't
 	$(call target_title, "Deploying ${DIR} shared service") \
@@ -352,7 +358,8 @@ prepare-for-e2e:
 	&& $(call workspace_service_bundle,gitea) \
 	&& $(call workspace_service_bundle,innereye) \
 	&& $(call shared_service_bundle,sonatype-nexus) \
-	&& $(call shared_service_bundle,gitea)
+	&& $(call shared_service_bundle,gitea) \
+	&& $(call user_resource_bundle,guacamole,guacamole-dev-vm)
 
 test-e2e-smoke:
 	$(call target_title, "Running E2E smoke tests") && \

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.2.27"
+__version__ = "0.3.7"

api_app/api/routes/api.py

@@ -95,7 +95,7 @@ async def swagger_ui_redirect():
 
 def get_scope(workspace) -> str:
     # Cope with the fact that scope id can have api:// at the front.
-    return f"api://{workspace['properties']['scope_id'].lstrip('api://')}/user_impersonation"
+    return f"api://{workspace.properties['scope_id'].lstrip('api://')}/user_impersonation"
 
 
 @workspace_swagger_router.get("/workspaces/{workspace_id}/openapi.json", include_in_schema=False, name="openapi_definitions")

api_app/api/routes/resource_helpers.py

@@ -91,11 +91,6 @@ async def send_custom_action_message(resource: Resource, resource_repo: Resource
         raise HTTPException(status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail=strings.SERVICE_BUS_GENERAL_ERROR_MESSAGE)
 
 
-def check_for_etag(etag: str):
-    if etag is None or len(etag) == 0:
-        raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=strings.ETAG_REQUIRED)
-
-
 def get_current_template_by_name(template_name: str, template_repo: ResourceTemplateRepository, resource_type: ResourceType, parent_service_template_name: str = "", is_update: bool = False) -> dict:
     try:
         template = template_repo.get_current_template(template_name, resource_type, parent_service_template_name)

api_app/api/routes/shared_services.py

@@ -14,7 +14,7 @@
 from models.schemas.shared_service import SharedServiceInCreate, SharedServicesInList, SharedServiceInResponse
 from models.schemas.resource import ResourcePatch
 from resources import strings
-from .workspaces import save_and_deploy_resource, check_for_etag, construct_location_header
+from .workspaces import save_and_deploy_resource, construct_location_header
 from azure.cosmos.exceptions import CosmosAccessConditionFailedError
 from .resource_helpers import send_custom_action_message, send_uninstall_message, send_resource_request_message
 from services.authentication import get_current_admin_user, get_current_tre_user_or_tre_admin
@@ -63,8 +63,7 @@ async def create_shared_service(response: Response, shared_service_input: Shared
                               response_model=OperationInResponse,
                               name=strings.API_UPDATE_SHARED_SERVICE,
                               dependencies=[Depends(get_current_admin_user), Depends(get_shared_service_by_id_from_path)])
-async def patch_shared_service(shared_service_patch: ResourcePatch, response: Response, user=Depends(get_current_admin_user), shared_service_repo=Depends(get_repository(SharedServiceRepository)), shared_service=Depends(get_shared_service_by_id_from_path), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), etag: str = Header(None)) -> SharedServiceInResponse:
-    check_for_etag(etag)
+async def patch_shared_service(shared_service_patch: ResourcePatch, response: Response, user=Depends(get_current_admin_user), shared_service_repo=Depends(get_repository(SharedServiceRepository)), shared_service=Depends(get_shared_service_by_id_from_path), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), etag: str = Header(...)) -> SharedServiceInResponse:
     try:
         patched_shared_service, resource_template = shared_service_repo.patch_shared_service(shared_service, shared_service_patch, etag, resource_template_repo, user)
         operation = await send_resource_request_message(

api_app/api/routes/workspaces.py

@@ -25,7 +25,7 @@
 from services.authentication import extract_auth_information
 from services.azure_resource_status import get_azure_resource_status
 from azure.cosmos.exceptions import CosmosAccessConditionFailedError
-from .resource_helpers import get_user_role_assignments, save_and_deploy_resource, construct_location_header, send_uninstall_message, check_for_etag, \
+from .resource_helpers import get_user_role_assignments, save_and_deploy_resource, construct_location_header, send_uninstall_message, \
     send_custom_action_message, send_resource_request_message
 from models.domain.request_action import RequestAction
 
@@ -73,7 +73,7 @@ async def create_workspace(workspace_create: WorkspaceInCreate, response: Respon
     try:
         # TODO: This requires Directory.ReadAll ( Application.Read.All ) to be enabled in the Azure AD application to enable a users workspaces to be listed. This should be made optional.
         auth_info = extract_auth_information(workspace_create.properties)
-        workspace, resource_template = workspace_repo.create_workspace_item(workspace_create, auth_info)
+        workspace, resource_template = workspace_repo.create_workspace_item(workspace_create, auth_info, user.id)
     except (ValidationError, ValueError) as e:
         logging.error(f"Failed to create workspace model instance: {e}")
         raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
@@ -91,8 +91,7 @@ async def create_workspace(workspace_create: WorkspaceInCreate, response: Respon
 
 
 @workspaces_core_router.patch("/workspaces/{workspace_id}", status_code=status.HTTP_202_ACCEPTED, response_model=OperationInResponse, name=strings.API_UPDATE_WORKSPACE, dependencies=[Depends(get_current_admin_user)])
-async def patch_workspace(workspace_patch: ResourcePatch, response: Response, user=Depends(get_current_admin_user), workspace=Depends(get_workspace_by_id_from_path), workspace_repo=Depends(get_repository(WorkspaceRepository)), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), etag: str = Header(None)) -> OperationInResponse:
-    check_for_etag(etag)
+async def patch_workspace(workspace_patch: ResourcePatch, response: Response, user=Depends(get_current_admin_user), workspace=Depends(get_workspace_by_id_from_path), workspace_repo=Depends(get_repository(WorkspaceRepository)), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), etag: str = Header(...)) -> OperationInResponse:
     try:
         patched_workspace, resource_template = workspace_repo.patch_workspace(workspace, workspace_patch, etag, resource_template_repo, user)
         operation = await send_resource_request_message(
@@ -194,8 +193,7 @@ async def create_workspace_service(response: Response, workspace_service_input:
 
 
 @workspace_services_workspace_router.patch("/workspaces/{workspace_id}/workspace-services/{service_id}", status_code=status.HTTP_202_ACCEPTED, response_model=OperationInResponse, name=strings.API_UPDATE_WORKSPACE_SERVICE, dependencies=[Depends(get_current_workspace_owner_or_researcher_user), Depends(get_workspace_by_id_from_path)])
-async def patch_workspace_service(workspace_service_patch: ResourcePatch, response: Response, user=Depends(get_current_workspace_owner_or_researcher_user), workspace_service_repo=Depends(get_repository(WorkspaceServiceRepository)), workspace_service=Depends(get_workspace_service_by_id_from_path), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), etag: str = Header(None)) -> OperationInResponse:
-    check_for_etag(etag)
+async def patch_workspace_service(workspace_service_patch: ResourcePatch, response: Response, user=Depends(get_current_workspace_owner_or_researcher_user), workspace_service_repo=Depends(get_repository(WorkspaceServiceRepository)), workspace_service=Depends(get_workspace_service_by_id_from_path), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), etag: str = Header(...)) -> OperationInResponse:
     try:
         patched_workspace_service, resource_template = workspace_service_repo.patch_workspace_service(workspace_service, workspace_service_patch, etag, resource_template_repo, user)
         operation = await send_resource_request_message(
@@ -337,8 +335,7 @@ async def delete_user_resource(response: Response, user=Depends(get_current_work
 
 
 @user_resources_workspace_router.patch("/workspaces/{workspace_id}/workspace-services/{service_id}/user-resources/{resource_id}", status_code=status.HTTP_202_ACCEPTED, response_model=OperationInResponse, name=strings.API_UPDATE_USER_RESOURCE, dependencies=[Depends(get_workspace_by_id_from_path), Depends(get_workspace_service_by_id_from_path)])
-async def patch_user_resource(user_resource_patch: ResourcePatch, response: Response, user=Depends(get_current_workspace_owner_or_researcher_user), user_resource=Depends(get_user_resource_by_id_from_path), workspace_service=Depends(get_workspace_service_by_id_from_path), user_resource_repo=Depends(get_repository(UserResourceRepository)), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), etag: str = Header(None)) -> OperationInResponse:
-    check_for_etag(etag)
+async def patch_user_resource(user_resource_patch: ResourcePatch, response: Response, user=Depends(get_current_workspace_owner_or_researcher_user), user_resource=Depends(get_user_resource_by_id_from_path), workspace_service=Depends(get_workspace_service_by_id_from_path), user_resource_repo=Depends(get_repository(UserResourceRepository)), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), etag: str = Header(...)) -> OperationInResponse:
     validate_user_is_workspace_owner_or_resource_owner(user, user_resource)
 
     try:
@@ -362,10 +359,11 @@ async def patch_user_resource(user_resource_patch: ResourcePatch, response: Resp
 
 # user resource actions
 @user_resources_workspace_router.post("/workspaces/{workspace_id}/workspace-services/{service_id}/user-resources/{resource_id}/invoke-action", status_code=status.HTTP_202_ACCEPTED, response_model=OperationInResponse, name=strings.API_INVOKE_ACTION_ON_USER_RESOURCE)
-async def invoke_action_on_user_resource(response: Response, action: str, user_resource=Depends(get_user_resource_by_id_from_path), workspace_service=Depends(get_workspace_service_by_id_from_path), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), user=Depends(get_current_workspace_owner_or_researcher_user)) -> OperationInResponse:
+async def invoke_action_on_user_resource(response: Response, action: str, user_resource=Depends(get_user_resource_by_id_from_path), workspace_service=Depends(get_workspace_service_by_id_from_path), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), user_resource_repo=Depends(get_repository(UserResourceRepository)), operations_repo=Depends(get_repository(OperationRepository)), user=Depends(get_current_workspace_owner_or_researcher_user)) -> OperationInResponse:
     validate_user_is_workspace_owner_or_resource_owner(user, user_resource)
     operation = await send_custom_action_message(
         resource=user_resource,
+        resource_repo=user_resource_repo,
         custom_action=action,
         resource_type=ResourceType.UserResource,
         operations_repo=operations_repo,