diff --git a/CHANGELOG.md b/CHANGELOG.md index d52d77954a..e153c9ee86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ENHANCEMENTS: * API: Replace HTTP_422_UNPROCESSABLE_ENTITY response with HTTP_422_UNPROCESSABLE_CONTENT as per RFC 9110 ([#4742](https://github.com/microsoft/AzureTRE/issues/4742)) * Change Group.ReadWrite.All permission to Group.Create for AUTO_WORKSPACE_GROUP_CREATION ([#4772](https://github.com/microsoft/AzureTRE/issues/4772)) * Make workspace shared storage quota updateable ([#4314](https://github.com/microsoft/AzureTRE/issues/4314)) +* Allow new template properties to be specified during template upgrades. Remove Template properties that no longer exist. * Update Porter, AzureCLI, Terraform and its providers across the solution ([#4799](https://github.com/microsoft/AzureTRE/issues/4799)) BUG FIXES: diff --git a/api_app/_version.py b/api_app/_version.py index 5461be1190..4f3cae6929 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.25.7" +__version__ = "0.25.8" diff --git a/api_app/db/repositories/resources.py b/api_app/db/repositories/resources.py index efa8af841e..bbf3f30eba 100644 --- a/api_app/db/repositories/resources.py +++ b/api_app/db/repositories/resources.py @@ -111,6 +111,16 @@ async def validate_input_against_template(self, template_name: str, resource_inp return parse_obj_as(ResourceTemplate, template) + def _get_all_property_keys_from_template(self, resource_template: ResourceTemplate) -> set: + properties = set(resource_template.properties.keys()) + if "allOf" in resource_template: + for condition in resource_template.allOf: + if "then" in condition and "properties" in condition["then"]: + properties.update(condition["then"]["properties"].keys()) + if "else" in condition and "properties" in condition["else"]: + properties.update(condition["else"]["properties"].keys()) + return properties + async def patch_resource(self, resource: Resource, resource_patch: ResourcePatch, resource_template: ResourceTemplate, etag: str, resource_template_repo: ResourceTemplateRepository, resource_history_repo: ResourceHistoryRepository, user: User, resource_action: str, force_version_update: bool = False) -> Tuple[Resource, ResourceTemplate]: await resource_history_repo.create_resource_history_item(resource) # now update the resource props @@ -123,10 +133,20 @@ async def patch_resource(self, resource: Resource, resource_patch: ResourcePatch if resource_patch.templateVersion is not None: await self.validate_template_version_patch(resource, resource_patch, resource_template_repo, resource_template, force_version_update) + new_template = await resource_template_repo.get_template_by_name_and_version(resource.templateName, resource_patch.templateVersion, resource.resourceType) + + old_properties = self._get_all_property_keys_from_template(resource_template) + new_properties = self._get_all_property_keys_from_template(new_template) + + properties_to_remove = old_properties - new_properties + for prop in properties_to_remove: + if prop in resource.properties: + del resource.properties[prop] + resource.templateVersion = resource_patch.templateVersion if resource_patch.properties is not None and len(resource_patch.properties) > 0: - self.validate_patch(resource_patch, resource_template_repo, resource_template, resource_action) + await self.validate_patch(resource_patch, resource_template_repo, resource_template, resource_action) # if we're here then we're valid - update the props + persist resource.properties.update(resource_patch.properties) @@ -183,16 +203,39 @@ async def validate_template_version_patch(self, resource: Resource, resource_pat except EntityDoesNotExist: raise TargetTemplateVersionDoesNotExist(f"Template '{resource_template.name}' not found for resource type '{resource_template.resourceType}' with target template version '{resource_patch.templateVersion}'") - def validate_patch(self, resource_patch: ResourcePatch, resource_template_repo: ResourceTemplateRepository, resource_template: ResourceTemplate, resource_action: str): + async def validate_patch(self, resource_patch: ResourcePatch, resource_template_repo: ResourceTemplateRepository, resource_template: ResourceTemplate, resource_action: str): # get the enriched (combined) template enriched_template = resource_template_repo.enrich_template(resource_template, is_update=True) - # validate the PATCH data against a cut down version of the full template. + # get the old template properties for comparison during upgrades + old_template_properties = set(enriched_template["properties"].keys()) + + # get the schema for the target version if upgrade is happening + if resource_patch.templateVersion is not None: + # fetch the template for the target version + target_template = await resource_template_repo.get_template_by_name_and_version(resource_template.name, resource_patch.templateVersion, resource_template.resourceType) + enriched_template = resource_template_repo.enrich_template(target_template, is_update=True) + + # validate the PATCH data against the target schema. update_template = copy.deepcopy(enriched_template) update_template["required"] = [] update_template["properties"] = {} for prop_name, prop in enriched_template["properties"].items(): - if (resource_action == RESOURCE_ACTION_INSTALL or prop.get("updateable", False) is True): + # Allow property if: + # 1. Installing (all properties allowed) + # 2. Property is explicitly updateable + # 3. Upgrading and the property is NEW (not in old template) and being added in this patch + if ( + resource_action == RESOURCE_ACTION_INSTALL + or prop.get("updateable", False) is True + or ( + resource_patch.templateVersion is not None + and resource_patch.templateVersion != resource_template.version + and resource_patch.properties is not None + and prop_name in resource_patch.properties + and prop_name not in old_template_properties + ) + ): update_template["properties"][prop_name] = prop self._validate_resource_parameters(resource_patch.dict(), update_template) diff --git a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py index 60b8569c53..e734b36f33 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_resource_repository.py @@ -4,6 +4,7 @@ import pytest import pytest_asyncio from mock import patch, MagicMock +from pydantic import parse_obj_as from jsonschema.exceptions import ValidationError from resources import strings @@ -140,6 +141,58 @@ def sample_nested_template() -> ResourceTemplate: ).dict(exclude_none=True) +def sample_resource_template_with_new_property(version: str = "0.2.0") -> dict: + """ + Returns a template similar to sample_resource_template but with an additional + 'new_property' that is not updateable. Useful for testing template upgrades. + """ + return ResourceTemplate( + id="123", + name="tre-user-resource", + description="description", + version=version, + resourceType=ResourceType.UserResource, + current=True, + required=['os_image', 'title'], + properties={ + 'title': { + 'type': 'string', + 'title': 'Title of the resource' + }, + 'os_image': { + 'type': 'string', + 'title': 'Windows image', + 'description': 'Select Windows image to use for VM', + 'enum': [ + 'Windows 10', + 'Server 2019 Data Science VM' + ], + 'updateable': False + }, + 'vm_size': { + 'type': 'string', + 'title': 'VM Size', + 'description': 'Select Windows image to use for VM', + 'enum': [ + 'small', + 'large' + ], + 'updateable': True + }, + 'new_property': { + 'type': 'string', + 'title': 'New non-updateable property', + 'enum': [ + 'value1', + 'value2' + ], + 'updateable': False + } + }, + actions=[] + ).dict(exclude_none=True) + + @pytest.mark.asyncio @patch("db.repositories.resources.ResourceRepository._get_enriched_template") @patch("db.repositories.resources.ResourceRepository._validate_resource_parameters", return_value=None) @@ -370,10 +423,11 @@ async def test_patch_resource_preserves_property_history(_, __, ___, resource_re resource_repo.update_item_with_etag.assert_called_with(expected_resource, etag) +@pytest.mark.asyncio @patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') -def test_validate_patch_with_good_fields_passes(template_repo, resource_repo): +async def test_validate_patch_with_good_fields_passes(template_repo, resource_repo): """ - Make sure that patch is NOT valid when non-updateable fields are included + Make sure that patch is valid when updateable fields are included """ template_repo.enrich_template = MagicMock(return_value=sample_resource_template()) @@ -381,11 +435,12 @@ def test_validate_patch_with_good_fields_passes(template_repo, resource_repo): # check it's valid when updating a single updateable prop patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'large'}) - resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) +@pytest.mark.asyncio @patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') -def test_validate_patch_with_bad_fields_fails(template_repo, resource_repo): +async def test_validate_patch_with_bad_fields_fails(template_repo, resource_repo): """ Make sure that patch is NOT valid when non-updateable fields are included """ @@ -396,14 +451,121 @@ def test_validate_patch_with_bad_fields_fails(template_repo, resource_repo): # check it's invalid when sending an unexpected field patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'large', 'unexpected_field': 'surprise!'}) with pytest.raises(ValidationError): - resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL) + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) - # check it's invalid when sending a bad value + # check it's invalid when sending a bad value (new install) patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'huge'}) with pytest.raises(ValidationError): - resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL) + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL) + + # check it's invalid when sending a bad value (update) + patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'huge'}) + with pytest.raises(ValidationError): + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) # check it's invalid when trying to update a non-updateable field patch = ResourcePatch(isEnabled=True, properties={'vm_size': 'large', 'os_image': 'linux'}) with pytest.raises(ValidationError): - resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_INSTALL) + await resource_repo.validate_patch(patch, template_repo, template, strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.get_template_by_name_and_version') +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_allows_new_non_updateable_property_during_upgrade(enrich_template_mock, get_template_mock, resource_repo): + """ + Test that during a template upgrade, new properties (not in old version) can be specified + even if they are marked as updateable: false in the new template version + """ + # Old template has os_image and vm_size + old_template = sample_resource_template() + old_template['version'] = '0.1.0' + + # New template adds a new property 'new_property' that is not updateable + new_template = sample_resource_template_with_new_property(version='0.2.0') + + # Mock the template repository + template_repo = MagicMock() + template_repo.get_template_by_name_and_version = AsyncMock(return_value=parse_obj_as(ResourceTemplate, new_template)) + template_repo.enrich_template = MagicMock(side_effect=[old_template, new_template]) + + # Patch includes the new property during upgrade - this should be ALLOWED + patch = ResourcePatch(templateVersion='0.2.0', properties={'new_property': 'value1'}) + + # This should NOT raise a ValidationError + await resource_repo.validate_patch(patch, template_repo, parse_obj_as(ResourceTemplate, old_template), strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.get_template_by_name_and_version') +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_rejects_existing_non_updateable_property_during_upgrade(enrich_template_mock, get_template_mock, resource_repo): + """ + Test that during a template upgrade, existing non-updateable properties still cannot be modified + """ + # Old template has os_image (non-updateable) and vm_size (updateable) + old_template = sample_resource_template() + + # New template is the same but version 0.2.0 + new_template = copy.deepcopy(old_template) + + # Mock the template repository + template_repo = MagicMock() + template_repo.get_template_by_name_and_version = AsyncMock(return_value=parse_obj_as(ResourceTemplate, new_template)) + template_repo.enrich_template = MagicMock(side_effect=[old_template, new_template]) + + # Try to update existing non-updateable property during upgrade - this should FAIL + patch = ResourcePatch(templateVersion='0.2.0', properties={'os_image': 'Windows 10'}) + + with pytest.raises(ValidationError): + await resource_repo.validate_patch(patch, template_repo, parse_obj_as(ResourceTemplate, old_template), strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.get_template_by_name_and_version') +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_allows_updateable_property_during_upgrade(enrich_template_mock, get_template_mock, resource_repo): + """ + Test that during a template upgrade, updateable properties can still be modified + """ + # Old template 0.1.0 + old_template = sample_resource_template() + + # New template 0.2.0 + new_template = copy.deepcopy(old_template) + + # Mock the template repository + template_repo = MagicMock() + template_repo.get_template_by_name_and_version = AsyncMock(return_value=parse_obj_as(ResourceTemplate, new_template)) + template_repo.enrich_template = MagicMock(side_effect=[old_template, new_template]) + + # Update existing updateable property during upgrade - this should work + patch = ResourcePatch(templateVersion='0.2.0', properties={'vm_size': 'large'}) + + # This should NOT raise a ValidationError + await resource_repo.validate_patch(patch, template_repo, parse_obj_as(ResourceTemplate, old_template), strings.RESOURCE_ACTION_UPDATE) + + +@pytest.mark.asyncio +@patch('db.repositories.resources.ResourceTemplateRepository.get_template_by_name_and_version') +@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template') +async def test_validate_patch_allows_mix_of_new_and_updateable_properties_during_upgrade(enrich_template_mock, get_template_mock, resource_repo): + """ + Test that during upgrade, you can specify both new non-updateable properties and existing updateable properties + """ + # Old template + old_template = sample_resource_template() + + # New template adds new_property + new_template = sample_resource_template_with_new_property(version='0.2.0') + + # Mock the template repository + template_repo = MagicMock() + template_repo.get_template_by_name_and_version = AsyncMock(return_value=parse_obj_as(ResourceTemplate, new_template)) + template_repo.enrich_template = MagicMock(side_effect=[old_template, new_template]) + + # Patch with both new non-updateable property and existing updateable property + patch = ResourcePatch(templateVersion='0.2.0', properties={'new_property': 'value1', 'vm_size': 'large'}) + + # This should NOT raise a ValidationError + await resource_repo.validate_patch(patch, template_repo, parse_obj_as(ResourceTemplate, old_template), strings.RESOURCE_ACTION_UPDATE) diff --git a/ui/app/package-lock.json b/ui/app/package-lock.json index b0fbb60cf3..56d958e735 100644 --- a/ui/app/package-lock.json +++ b/ui/app/package-lock.json @@ -1,12 +1,12 @@ { "name": "tre-ui", - "version": "0.8.19", + "version": "0.8.21", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "tre-ui", - "version": "0.8.19", + "version": "0.8.21", "dependencies": { "@azure/msal-browser": "^2.35.0", "@azure/msal-react": "^1.5.12", diff --git a/ui/app/package.json b/ui/app/package.json index 3400a0c5f3..2ed0b4bef2 100644 --- a/ui/app/package.json +++ b/ui/app/package.json @@ -1,6 +1,6 @@ { "name": "tre-ui", - "version": "0.8.20", + "version": "0.8.21", "private": true, "type": "module", "dependencies": { diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx new file mode 100644 index 0000000000..ac034b69e5 --- /dev/null +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.test.tsx @@ -0,0 +1,704 @@ +import React from "react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, fireEvent, waitFor, createPartialFluentUIMock } from "../../test-utils"; +import { ConfirmUpgradeResource } from "./ConfirmUpgradeResource"; +import { Resource, AvailableUpgrade } from "../../models/resource"; +import { ResourceType } from "../../models/resourceType"; +import { WorkspaceContext } from "../../contexts/WorkspaceContext"; +import { CostResource } from "../../models/costs"; + +// Mock dependencies +const mockApiCall = vi.fn(); +const mockDispatch = vi.fn(); + +// Mock template schemas +const mockCurrentTemplateSchema = { + properties: { + display_name: { type: "string" }, + resource_key: { type: "string" }, + existing_property: { type: "string" }, + }, + required: ["display_name"], +}; + +const mockNewTemplateSchema = { + properties: { + display_name: { type: "string" }, + resource_key: { type: "string" }, + new_property: { type: "string", default: "default_value" }, + }, + required: ["display_name"], + uiSchema: {}, +}; + +vi.mock("../../hooks/useAuthApiCall", () => ({ + useAuthApiCall: () => mockApiCall, + HttpMethod: { Patch: "PATCH", Get: "GET" }, + ResultType: { JSON: "JSON" }, +})); + +vi.mock("../../hooks/customReduxHooks", () => ({ + useAppDispatch: () => mockDispatch, +})); + +vi.mock("../shared/notifications/operationsSlice", () => ({ + addUpdateOperation: vi.fn(), + default: (state: any = { items: [] }) => state +})); + +// Mock FluentUI components using centralized mocks +vi.mock("@fluentui/react", async () => { + const actual = await vi.importActual("@fluentui/react"); + return { + ...actual, + ...createPartialFluentUIMock([ + 'Dialog', + 'DialogFooter', + 'DialogType', + 'PrimaryButton', + 'DefaultButton', + 'Dropdown', + 'Spinner', + 'MessageBar', + 'MessageBarType', + 'Icon' + ]), + }; +}); + +vi.mock("./ExceptionLayout", () => ({ + ExceptionLayout: ({ e }: any) => ( +
{e.userMessage}
+ ), +})); + +const mockAvailableUpgrades: AvailableUpgrade[] = [ + { version: "1.1.0", forceUpdateRequired: false }, + { version: "1.2.0", forceUpdateRequired: false }, + { version: "2.0.0", forceUpdateRequired: true }, +]; + +const mockResource: Resource = { + id: "test-resource-id", + resourceType: ResourceType.WorkspaceService, + templateName: "test-template", + templateVersion: "1.0.0", + resourcePath: "/workspaces/test-workspace/workspace-services/test-resource-id", + resourceVersion: 1, + isEnabled: true, + properties: { + display_name: "Test Resource", + }, + _etag: "test-etag", + updatedWhen: Date.now(), + deploymentStatus: "deployed", + availableUpgrades: mockAvailableUpgrades, + history: [], + user: { + id: "test-user-id", + name: "Test User", + email: "test@example.com", + roleAssignments: [], + roles: ["workspace_researcher"], + }, +}; + +const mockWorkspaceContext = { + costs: [] as CostResource[], + workspace: { + id: "test-workspace-id", + isEnabled: true, + resourcePath: "/workspaces/test-workspace-id", + resourceVersion: 1, + resourceType: ResourceType.Workspace, + templateName: "base", + templateVersion: "1.0.0", + availableUpgrades: [], + deploymentStatus: "deployed", + updatedWhen: Date.now(), + history: [], + _etag: "test-etag", + properties: { + display_name: "Test Workspace", + }, + user: { + id: "test-user-id", + name: "Test User", + email: "test@example.com", + roleAssignments: [], + roles: ["workspace_owner"], + }, + workspaceURL: "https://workspace.example.com", + }, + workspaceApplicationIdURI: "test-app-id-uri", + roles: ["workspace_owner"], + setCosts: vi.fn(), + setRoles: vi.fn(), + setWorkspace: vi.fn(), +}; + +const renderWithWorkspaceContext = (component: React.ReactElement) => { + return render( + + {component} + + ); +}; + +describe("ConfirmUpgradeResource Component", () => { + const mockOnDismiss = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + // Mock API call to return templates for GET requests + mockApiCall.mockImplementation((url, method) => { + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: { id: "operation-id", status: "running" } }); + }); + }); + + it("renders upgrade dialog with correct title and content", () => { + renderWithWorkspaceContext( + + ); + + expect(screen.getByTestId("dialog-title")).toHaveTextContent( + "Upgrade Template Version?" + ); + expect(screen.getByTestId("dialog-subtext")).toHaveTextContent( + "Are you sure you want upgrade the template version of Test Resource from version 1.0.0?" + ); + }); + + it("shows warning message about irreversible upgrade", () => { + renderWithWorkspaceContext( + + ); + + expect(screen.getByTestId("message-bar")).toBeInTheDocument(); + expect(screen.getByText("Upgrading the template version is irreversible.")).toBeInTheDocument(); + }); + + it("renders dropdown with available upgrade versions", () => { + renderWithWorkspaceContext( + + ); + + const dropdown = screen.getByTestId("dropdown"); + expect(dropdown).toBeInTheDocument(); + + // Check that non-major upgrades are included (force update required = false) + expect(screen.getByText("1.1.0")).toBeInTheDocument(); + expect(screen.getByText("1.2.0")).toBeInTheDocument(); + + // Major upgrade (force update required = true) should not be included in regular dropdown + expect(screen.queryByText("2.0.0")).not.toBeInTheDocument(); + }); + + it("disables upgrade button when no version is selected", () => { + renderWithWorkspaceContext( + + ); + + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).toBeDisabled(); + }); + + it("enables upgrade button when version is selected", async () => { + renderWithWorkspaceContext( + + ); + + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load and button to become enabled + await waitFor(() => { + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).not.toBeDisabled(); + }); + }); + + it("calls API with selected version on upgrade", async () => { + const mockOperation = { id: "operation-id", status: "running" }; + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.resolve({ operation: mockOperation }); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: mockOperation }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect(screen.queryByText("Loading new template schema...")).not.toBeInTheDocument(); + }); + + // Click upgrade + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + await waitFor(() => { + expect(mockApiCall).toHaveBeenCalledWith( + mockResource.resourcePath, + "PATCH", + mockWorkspaceContext.workspaceApplicationIdURI, + expect.objectContaining({ + templateVersion: "1.1.0", + properties: expect.any(Object), + }), + "JSON", + undefined, + undefined, + mockResource._etag + ); + }); + + expect(mockDispatch).toHaveBeenCalled(); + expect(mockOnDismiss).toHaveBeenCalled(); + }); + + it("shows loading spinner during API call", async () => { + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return new Promise((resolve) => + setTimeout( + () => { + resolve({ operation: { id: "operation-id", status: "running" } }); + }, + 100 + ) + ); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ + operation: { id: "operation-id", status: "running" }, + }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Click upgrade and check for loading spinner + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + expect(screen.getByTestId("spinner")).toBeInTheDocument(); + expect(screen.getByText("Sending request...")).toBeInTheDocument(); + }); + + it("displays error when API call fails", async () => { + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.reject(new Error("Network error")); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.reject(new Error("Network error")); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Click upgrade + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + await waitFor(() => { + expect(screen.getByTestId("exception-layout")).toBeInTheDocument(); + expect(screen.getByText("Failed to upgrade resource")).toBeInTheDocument(); + }); + }); + + it("uses workspace auth for workspace service resources", async () => { + const mockOperation = { id: "operation-id", status: "running" }; + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.resolve({ operation: mockOperation }); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: mockOperation }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Click upgrade + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + await waitFor(() => { + expect(mockApiCall).toHaveBeenCalledWith( + expect.any(String), + "PATCH", + mockWorkspaceContext.workspaceApplicationIdURI, // should use workspace auth + expect.any(Object), + "JSON", + undefined, + undefined, + expect.any(String) + ); + }); + }); + + it("does not use workspace auth for shared service resources", async () => { + const sharedServiceResource = { + ...mockResource, + resourceType: ResourceType.SharedService, + }; + const mockOperation = { id: "operation-id", status: "running" }; + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.resolve({ operation: mockOperation }); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: mockOperation }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Click upgrade + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + + await waitFor(() => { + expect(mockApiCall).toHaveBeenCalledWith( + expect.any(String), + "PATCH", + undefined, // should not use workspace auth + expect.any(Object), + "JSON", + undefined, + undefined, + expect.any(String) + ); + }); + }); + + it("filters out major upgrades from dropdown options", () => { + const resourceWithMajorUpgrade = { + ...mockResource, + availableUpgrades: [ + { version: "1.1.0", forceUpdateRequired: false }, + { version: "2.0.0", forceUpdateRequired: true }, + { version: "1.2.0", forceUpdateRequired: false }, + ], + }; + + renderWithWorkspaceContext( + + ); + + // Minor updates should be available + expect(screen.getByText("1.1.0")).toBeInTheDocument(); + expect(screen.getByText("1.2.0")).toBeInTheDocument(); + + // Major update should not be available in dropdown + expect(screen.queryByText("2.0.0")).not.toBeInTheDocument(); + }); + + it("displays form when new properties need to be added", async () => { + renderWithWorkspaceContext( + + ); + + // Select a version that has new properties + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Should show info message about new properties + expect(screen.getByText("You must specify values for new properties:")).toBeInTheDocument(); + + // The form input for new_property should be rendered + expect(screen.getByDisplayValue("default_value")).toBeInTheDocument(); + }); + + it("displays warning about removed properties", async () => { + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Should show warning about removed properties + expect(screen.getByText(/Warning: The following properties are no longer present/)).toBeInTheDocument(); + expect(screen.getByText(/existing_property/)).toBeInTheDocument(); + }); + + it("disables upgrade button when required new properties are cleared", async () => { + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Find the input field and clear it + const inputField = screen.getByDisplayValue("default_value"); + fireEvent.change(inputField, { target: { value: "" } }); + + // Button should now be disabled because the required property is empty + await waitFor(() => { + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).toBeDisabled(); + }); + }); + + it("enables upgrade button when all new properties are filled in", async () => { + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Find the new_property input field and fill it + const inputField = screen.getByDisplayValue("default_value"); + fireEvent.change(inputField, { target: { value: "filled_value" } }); + + // Button should now be enabled + await waitFor(() => { + const upgradeButton = screen.getByTestId("primary-button"); + expect(upgradeButton).not.toBeDisabled(); + }); + }); + + it("includes new property values in upgrade API call", async () => { + const mockOperation = { id: "operation-id", status: "running" }; + mockApiCall.mockImplementation((url, method) => { + if (method === "PATCH") { + return Promise.resolve({ operation: mockOperation }); + } + // Handle GET requests for schemas + if (method === "GET" && url.includes("?version=")) { + if (url.includes("version=1.0.0")) { + return Promise.resolve(mockCurrentTemplateSchema); + } else { + return Promise.resolve(mockNewTemplateSchema); + } + } + return Promise.resolve({ operation: mockOperation }); + }); + + renderWithWorkspaceContext( + + ); + + // Select a version + const dropdown = screen.getByTestId("dropdown"); + fireEvent.change(dropdown, { target: { value: "1.1.0" } }); + + // Wait for schema to load + await waitFor(() => { + expect( + screen.queryByText("Loading new template schema...") + ).not.toBeInTheDocument(); + }); + + // Fill in the new property + const inputField = screen.getByDisplayValue("default_value"); + fireEvent.change(inputField, { target: { value: "custom_value" } }); + + // Click upgrade + await waitFor(() => { + const upgradeButton = screen.getByTestId("primary-button"); + fireEvent.click(upgradeButton); + }); + + // Verify the API call includes the new property value + await waitFor(() => { + expect(mockApiCall).toHaveBeenCalledWith( + mockResource.resourcePath, + "PATCH", + mockWorkspaceContext.workspaceApplicationIdURI, + expect.objectContaining({ + templateVersion: "1.1.0", + properties: expect.objectContaining({ + new_property: "custom_value", + }), + }), + "JSON", + undefined, + undefined, + mockResource._etag + ); + }); + }); +}); diff --git a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx index 8967b08e01..b2dbf2b4eb 100644 --- a/ui/app/src/components/shared/ConfirmUpgradeResource.tsx +++ b/ui/app/src/components/shared/ConfirmUpgradeResource.tsx @@ -8,9 +8,12 @@ import { MessageBar, MessageBarType, Icon, + Stack, } from "@fluentui/react"; -import React, { useContext, useState } from "react"; +import React, { useContext, useState, useEffect, useRef } from "react"; import { AvailableUpgrade, Resource } from "../../models/resource"; +import { ApiEndpoint } from "../../models/apiEndpoints"; +import { WorkspaceService } from "../../models/workspaceService"; import { HttpMethod, ResultType, @@ -23,24 +26,113 @@ import { LoadingState } from "../../models/loadingState"; import { ExceptionLayout } from "./ExceptionLayout"; import { useAppDispatch } from "../../hooks/customReduxHooks"; import { addUpdateOperation } from "../shared/notifications/operationsSlice"; +import Form from "@rjsf/fluent-ui"; +import validator from "@rjsf/validator-ajv8"; interface ConfirmUpgradeProps { resource: Resource; onDismiss: () => void; } +// Utility to get all property keys from template schema's properties object recursively, flattening nested if needed +const getAllPropertyKeys = (properties: any, prefix = ""): string[] => { + if (!properties) return []; + let keys: string[] = []; + for (const [key, value] of Object.entries(properties)) { + if (value && typeof value === "object" && 'properties' in value) { + // recur for nested properties + keys = keys.concat(getAllPropertyKeys(value["properties"], prefix + key + ".")); + } else { + keys.push(prefix + key); + } + } + return keys; +}; + +// Utility to build a reduced schema with only given keys and their nested schema (depth 1), including required +const buildReducedSchema = (fullSchema: any, keys: string[]): any => { + if (!fullSchema || !fullSchema.properties) return null; + const reducedProperties: any = {}; + const required: string[] = []; + + keys.forEach((key) => { + // Only allow top-level property keys (no nested with dots) for simplicity here + const topKey = key.split('.')[0]; + if (fullSchema.properties[topKey]) { + if (!reducedProperties[topKey]) { + reducedProperties[topKey] = fullSchema.properties[topKey]; + if (fullSchema.required && fullSchema.required.includes(topKey)) { + required.push(topKey); + } + } + } + }); + + return { + type: "object", + properties: reducedProperties, + required: required.length > 0 ? required : undefined, + }; +}; + +// Utility to collect direct property keys referenced inside conditional schemas +const collectConditionalKeys = (entry: any): string[] => { + const keys: string[] = []; + if (!entry) return keys; + const collect = (schemaPart: any) => { + if (schemaPart && schemaPart.properties) { + keys.push(...Object.keys(schemaPart.properties)); + } + }; + collect(entry.if); + collect(entry.then); + collect(entry.else); + return [...new Set(keys)]; +}; + +// Extract conditional blocks that reference any of the new properties. +const extractConditionalBlocks = (schema: any, newKeys: string[]) => { + const conditionalEntries: any[] = []; + if (!schema) return { allOf: [] }; + const allOf = schema.allOf || []; + allOf.forEach((entry: any) => { + if (entry && entry.if) { + const conditionalKeys = collectConditionalKeys(entry); + // include entry if any conditionalKey matches a new key (top-level match) + if (conditionalKeys.some((k) => newKeys.some((nk) => nk.split('.')[0] === k))) { + conditionalEntries.push(entry); + } + } + }); + return { allOf: conditionalEntries }; +}; + export const ConfirmUpgradeResource: React.FunctionComponent< ConfirmUpgradeProps > = (props: ConfirmUpgradeProps) => { const apiCall = useAuthApiCall(); const [selectedVersion, setSelectedVersion] = useState(""); - const [apiError, setApiError] = useState({} as APIError); + const [apiError, setApiError] = useState(null); const [requestLoadingState, setRequestLoadingState] = useState( LoadingState.Ok, ); const workspaceCtx = useContext(WorkspaceContext); const dispatch = useAppDispatch(); + const [newPropertiesToFill, setNewPropertiesToFill] = useState([]); + const [newPropertyValues, setNewPropertyValues] = useState({}); + const [loadingSchema, setLoadingSchema] = useState(false); + const [newTemplateSchema, setNewTemplateSchema] = useState(null); + const [removedProperties, setRemovedProperties] = useState([]); + + // Cache for current template to avoid refetching the same template repeatedly while selecting versions + const currentTemplateRef = useRef(null); + + // Invalidate cache if the resource's template name or current template version changes + useEffect(() => { + currentTemplateRef.current = null; + }, [props.resource.templateName, props.resource.templateVersion]); + const upgradeProps = { type: DialogType.normal, title: `Upgrade Template Version?`, @@ -60,10 +152,128 @@ export const ConfirmUpgradeResource: React.FunctionComponent< props.resource.resourceType === ResourceType.WorkspaceService || props.resource.resourceType === ResourceType.UserResource; + // Fetch new template schema and identify new properties missing in current resource + useEffect(() => { + if (!selectedVersion) { + setNewPropertiesToFill([]); + setNewPropertyValues({}); + setNewTemplateSchema(null); + setRemovedProperties([]); + return; + } + + // Construct API paths for templates of specified resourceType + let templateListPath; + // Usually, the GET path would be `${templateGetPath}/${selectedTemplate}`, but there's an exception for user resources + let templateGetPath; + + // let workspaceApplicationIdURI = undefined; + switch (props.resource.resourceType) { + case ResourceType.Workspace: + templateListPath = ApiEndpoint.WorkspaceTemplates; + templateGetPath = templateListPath; + break; + case ResourceType.WorkspaceService: + templateListPath = ApiEndpoint.WorkspaceServiceTemplates; + templateGetPath = templateListPath; + break; + case ResourceType.SharedService: + templateListPath = ApiEndpoint.SharedServiceTemplates; + templateGetPath = templateListPath; + break; + case ResourceType.UserResource: + if (props.resource.properties.parentWorkspaceService) { + // If we are upgrading a user resource, parent resource must have a workspaceId + const workspaceId = (props.resource.properties.parentWorkspaceService as WorkspaceService) + .workspaceId; + templateListPath = `${ApiEndpoint.Workspaces}/${workspaceId}/${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; + templateGetPath = `${ApiEndpoint.WorkspaceServiceTemplates}/${props.resource.properties.parentWorkspaceService.templateName}/${ApiEndpoint.UserResourceTemplates}`; + break; + } else { + throw Error( + "Parent workspace service must be passed as prop when creating user resource.", + ); + } + default: + throw Error("Unsupported resource type."); + } + + const fetchNewTemplateSchema = async () => { + setLoadingSchema(true); + setApiError(null); + try { + let fetchUrl = ""; + + fetchUrl = `${templateGetPath}/${props.resource.templateName}?version=${selectedVersion}`; + + const newTemplate = await apiCall( + fetchUrl, + HttpMethod.Get, + props.resource.resourceType === ResourceType.UserResource ? workspaceCtx.workspaceApplicationIdURI : undefined, + undefined, + ResultType.JSON, + ); + + // Reuse cached current template if available to avoid redundant network calls + let currentTemplate; + if (currentTemplateRef.current) { + currentTemplate = currentTemplateRef.current; + } else { + currentTemplate = await apiCall( + `${templateGetPath}/${props.resource.templateName}?version=${props.resource.templateVersion}`, + HttpMethod.Get, + props.resource.resourceType === ResourceType.UserResource ? workspaceCtx.workspaceApplicationIdURI : undefined, + undefined, + ResultType.JSON, + ); + currentTemplateRef.current = currentTemplate; + } + + // Use full fetched schema from API + setNewTemplateSchema(newTemplate); + + const newSchemaProps = newTemplate?.properties || {}; + const currentProps = currentTemplate?.properties || {}; + + const newKeys = getAllPropertyKeys(newSchemaProps); + const currentKeys = getAllPropertyKeys(currentProps); + + const newPropKeys = newKeys.filter((k) => !currentKeys.includes(k)); + const removedPropsArray = currentKeys.filter((k) => !newKeys.includes(k)); + + setNewPropertiesToFill(newPropKeys); + setRemovedProperties(removedPropsArray); + + // prefill newPropertyValues with schema defaults or empty string + setNewPropertyValues( + newPropKeys.reduce((acc, key) => { + // Get top-level portion of the key + const topKey = key.split('.')[0]; + const defaultValue = newTemplate?.properties?.[topKey]?.default; + acc[key] = defaultValue !== undefined ? defaultValue : ''; + return acc; + }, {} as any), + ); + } catch (err: any) { + if (!err.userMessage) { + err.userMessage = "Failed to fetch new template schema"; + } + setApiError(err); + } finally { + setLoadingSchema(false); + } + }; + + fetchNewTemplateSchema(); + }, [selectedVersion]); + const upgradeCall = async () => { setRequestLoadingState(LoadingState.Loading); try { - let body = { templateVersion: selectedVersion }; + let body: any = { templateVersion: selectedVersion }; + + body.properties = newPropertyValues; + let op = await apiCall( props.resource.resourcePath, HttpMethod.Patch, @@ -77,12 +287,37 @@ export const ConfirmUpgradeResource: React.FunctionComponent< dispatch(addUpdateOperation(op.operation)); props.onDismiss(); } catch (err: any) { - err.userMessage = "Failed to upgrade resource"; + if (!err.userMessage) { + err.userMessage = "Failed to upgrade resource"; + } setApiError(err); setRequestLoadingState(LoadingState.Error); } }; + // Use buildReducedSchema to include only new properties + const reducedSchemaProperties = newTemplateSchema + ? buildReducedSchema(newTemplateSchema, newPropertiesToFill) + : null; + + // Extract any conditional blocks from full schema, filtered by new properties + const conditionalBlocks = newTemplateSchema ? extractConditionalBlocks(newTemplateSchema, newPropertiesToFill) : {}; + + // Compose final schema combining reduced properties with conditional blocks + const finalSchema = reducedSchemaProperties + ? { ...reducedSchemaProperties, ...conditionalBlocks } + : null; + + // UI schema override: hide the form's submit button because we use external Upgrade button + // start with existing UI order and classNames from full schema uiSchema + const baseUiSchema = newTemplateSchema?.uiSchema || {}; + + // Compose final uiSchema merging baseUiSchema with our overrides + const uiSchema = { + ...baseUiSchema, + "ui:submitButtonOptions": { norender: true }, + }; + const onRenderOption = (option: any): JSX.Element => { return (
@@ -129,6 +364,33 @@ export const ConfirmUpgradeResource: React.FunctionComponent< Upgrading the template version is irreversible. + + {loadingSchema && } + {!loadingSchema && removedProperties.length > 0 && ( + + Warning: The following properties are no longer present in the template and will be removed: {removedProperties.join(', ')} + + )} + {!loadingSchema && newPropertiesToFill.length > 0 && ( + + + You must specify values for new properties: + + + {finalSchema && ( +
setNewPropertyValues(e.formData)} + /> + )} + + )} + 0 && + Object.values(newPropertyValues).some( + (v) => v === "" || v === undefined, + )) + } text="Upgrade" onClick={() => upgradeCall()} /> @@ -156,7 +424,7 @@ export const ConfirmUpgradeResource: React.FunctionComponent< /> )} {requestLoadingState === LoadingState.Error && ( - + )}