Skip to content

Conversation

@marrobi
Copy link
Member

@marrobi marrobi commented Feb 7, 2025

Fixes #1123

Add validation to API endpoints to ensure correct resourceType in template registration payloads.

  • Workspace Service Templates: Add validation in api_app/api/routes/workspace_service_templates.py to check if resourceType is WorkspaceService before registering the template. Return a 422 Unprocessable Entity error if the resourceType does not match.
  • Workspace Templates: Add validation in api_app/api/routes/workspace_templates.py to check if resourceType is Workspace before registering the template. Return a 422 Unprocessable Entity error if the resourceType does not match.
  • User Resource Templates: Add validation in api_app/api/routes/user_resource_templates.py to check if resourceType is UserResource before registering the template. Return a 422 Unprocessable Entity error if the resourceType does not match.
  • Shared Service Templates: Add validation in api_app/api/routes/shared_service_templates.py to check if resourceType is SharedService before registering the template. Return a 422 Unprocessable Entity error if the resourceType does not match.
  • Common Error Message: Add a common error message string INVALID_RESOURCE_TYPE in api_app/resources/strings.py for validation errors, including the expected and received resourceType.
  • Tests: Add tests in api_app/tests_ma/test_api/test_routes/test_workspace_service_templates.py, api_app/tests_ma/test_api/test_routes/test_workspace_templates.py, api_app/tests_ma/test_api/test_routes/test_user_resource_templates.py, and api_app/tests_ma/test_api/test_routes/test_shared_service_templates.py to cover validation for each template registration

For more details, open the Copilot Workspace session.

Fixes #1123

Add validation to API endpoints to ensure correct resourceType in template registration payloads.

* **Workspace Service Templates**: Add validation in `api_app/api/routes/workspace_service_templates.py` to check if `resourceType` is `WorkspaceService` before registering the template. Return a 422 Unprocessable Entity error if the `resourceType` does not match.
* **Workspace Templates**: Add validation in `api_app/api/routes/workspace_templates.py` to check if `resourceType` is `Workspace` before registering the template. Return a 422 Unprocessable Entity error if the `resourceType` does not match.
* **User Resource Templates**: Add validation in `api_app/api/routes/user_resource_templates.py` to check if `resourceType` is `UserResource` before registering the template. Return a 422 Unprocessable Entity error if the `resourceType` does not match.
* **Shared Service Templates**: Add validation in `api_app/api/routes/shared_service_templates.py` to check if `resourceType` is `SharedService` before registering the template. Return a 422 Unprocessable Entity error if the `resourceType` does not match.
* **Common Error Message**: Add a common error message string `INVALID_RESOURCE_TYPE` in `api_app/resources/strings.py` for validation errors, including the expected and received resourceType.
* **Tests**: Add tests in `api_app/tests_ma/test_api/test_routes/test_workspace_service_templates.py`, `api_app/tests_ma/test_api/test_routes/test_workspace_templates.py`, `api_app/tests_ma/test_api/test_routes/test_user_resource_templates.py`, and `api_app/tests_ma/test_api/test_routes/test_shared_service_templates.py` to cover validation for each template registration

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/microsoft/AzureTRE/issues/1123?shareId=XXXX-XXXX-XXXX-XXXX).
@github-actions
Copy link

github-actions bot commented Feb 7, 2025

Unit Test Results

612 tests   612 ✅  7s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit b72ca14.

♻️ This comment has been updated with latest results.

@marrobi marrobi marked this pull request as draft February 7, 2025 21:42
@marrobi
Copy link
Member Author

marrobi commented Feb 7, 2025

This PR uncovered a few test bugs, however we are inconsistent with our resourceType naming. In the CI and makefile we use workspace_service, in the Python API we use workspace-service`.

Going to leave this here, but should resolve this inconsistency. Suggest by amending the CI and makefile to use -.

image
vs
image

@marrobi
Copy link
Member Author

marrobi commented Feb 12, 2025

Propose we should handle the naming difference in the Python code, and deal with the script update later, add notes to the release, then revert the Python in the future.

@martinpeck martinpeck marked this pull request as ready for review March 20, 2025 15:38
@martinpeck martinpeck added the blocked Cannot progress at present label Mar 20, 2025
@martinpeck martinpeck self-assigned this Mar 20, 2025
@martinpeck
Copy link
Member

Marking as ready for review.
Can't be merged as-is.

Copy link
Member

@martinpeck martinpeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left a question/comment but LGTM either way.

@martinpeck martinpeck removed their assignment Apr 1, 2025
@marrobi
Copy link
Member Author

marrobi commented Apr 1, 2025

Propose we should handle the naming difference in the Python code, and deal with the script update later, add notes to the release, then revert the Python in the future.

@martinpeck what think of best way to sort this? The scripts send resources with _ vs expecting - , see /workspaces/marrobi-azuretre/.github/workflows/deploy_tre_reusable.yml for for example.

I don't want to break everyone's pipelines.

Could do a replace of _ for - here: https://github.com/marrobi/AzureTRE/blob/973a02d7691a1e9272197d9159bb1699ec0a8e4a/devops/scripts/register_bundle_with_api.sh#L102-L103

@martinpeck
Copy link
Member

@marrobi honestly, I'm a bit confused about the underscore/hyphen issue. Perhaps let's chat.

@martinpeck
Copy link
Member

If we did patch up the problem in that script, I would suggest we do that only as a temp measure, generate a warning saying "WARNING: Fix this cos we won't patch this for you later", and then remove that patch code in a later release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked Cannot progress at present

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validation Missing from API Endpoints

3 participants