Allow Template upgrade properties to be set and remove properties that no longer exist.

[JC-wk]

Resolves #4732 #4730

What is being addressed

Currently if you add a new property to a template there is no way to specify this property before an upgrade is ran.
The upgrade may fail due to the missing property (although the template version is still incremented)
The user then has to click update and supply the property.
Similarly when removing a property from a template and running an upgrade, the property still exists on the resource.

Todo

• Enhancement: Current template should only be retrieved once.
• To check Pipeline Template JSON Schema versions #4795 and fix issue Error sending create / update request Unevaluated properties are not allowed ('your_property_name' was unexpected) occurs when updatable is false #4728
• To allow us to merge in the Unit Test Initial UI tests #4794 first so the UI tests can be added
• Possibly handle cases such as where an enum value is removed (not the field entirely) and that value is currently chosen

How is this addressed

• Adds a form to allow the user to specify new properties prior to the upgrade
• Removes any template properties that no longer exist in the new template
• Added tests
• Updated CHANGELOG.md
• Increment API version

[github-actions]

Unit Test Results

1 011 tests   1 011 ✅  29s ⏱️
   28 suites      0 💤
    2 files        0 ❌

Results for commit 554d1b4.

♻️ This comment has been updated with latest results.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

Suppressed comments (1)

api_app/db/repositories/resources.py:387

• During template upgrades, existing properties whose enum values were removed in the new template can make upgrades impossible when the property is marked updateable: false. The UI explicitly detects enum-invalid existing values and prompts the user to pick a new value, but the API currently rejects updates to existing non-updateable properties during upgrade (see is_leaf_allowed). This results in a deadlock: leaving the invalid value fails schema validation, but sending a new value is rejected as “not updateable”.

Consider allowing a one-time update during upgrade when (and only when) the current persisted value is no longer in the target template’s enum.

            if current_properties is not None and is_upgrade:
                has_existing, existing_val = get_nested_val(current_properties, prop_path)
                if has_existing and existing_val == prop_val:
                    return True

            return False

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

Suppressed comments (3)

api_app/tests_ma/test_db/test_repositories/test_resource_repository.py:431

• The test is using @patch('...ResourceTemplateRepository.enrich_template'), but then treats the injected mock argument (template_repo) as a repository instance by setting template_repo.enrich_template = MagicMock(...) and passing it into validate_patch. This works only by accident (because the injected argument is a MagicMock), but it’s confusing and makes the test setup easy to break when refactoring. Consider removing the @patch decorator and explicitly creating a template_repo = MagicMock() with an enrich_template method.

@pytest.mark.asyncio
@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template')
async def test_validate_patch_with_good_fields_passes(template_repo, resource_repo):
    """
    Make sure that patch is valid when updateable fields are included

api_app/tests_ma/test_db/test_repositories/test_resource_repository.py:450

• Same issue as the previous test: the @patch('...enrich_template') decorator injects a mock function, but the test treats that injected argument as a repository instance by assigning .enrich_template and passing it into validate_patch. This is confusing and brittle. Prefer explicitly constructing a template_repo = MagicMock() and removing the decorator/extra parameter.

@pytest.mark.asyncio
@patch('db.repositories.resources.ResourceTemplateRepository.enrich_template')
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
    """

    template_repo.enrich_template = MagicMock(return_value=sample_resource_template())
    template = sample_resource_template()

api_app/db/repositories/resources.py:279

• _get_pipeline_properties returns a list and unconditionally indexes prop["name"]. This has two concrete downsides: (1) membership checks later (prop_path in pipeline_properties) are O(n) instead of O(1), and (2) a malformed pipeline entry without a name key would raise KeyError during validation. Returning a set and using prop.get("name") avoids both issues.

    def _get_pipeline_properties(self, enriched_template) -> List[str]:
        properties = []
        pipeline = enriched_template.get("pipeline")
        if pipeline:
            for phase in ["install", "upgrade"]:
                if phase in pipeline and pipeline[phase]:
                    for step in pipeline[phase]:
                        if "properties" in step and step["properties"]:
                            for prop in step["properties"]:
                                properties.append(prop["name"])
        return properties

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

Suppressed comments (4)

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:538

• The Upgrade button remains enabled while the selected template schema is being fetched. Immediately after selecting a version (or while switching versions), newPropertiesToFill still reflects the empty or previous schema, so a user can submit the upgrade before newly required fields and removed properties are known. Set loading synchronously when the selection changes and include it in the disabled condition.

              <PrimaryButton
                primaryDisabled={
                  !selectedVersion ||
                  (newPropertiesToFill.length > 0 &&

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:518

• Conditional schemas are evaluated by RJSF against only newPropertyValues, so selectors that already exist on the resource are missing. For example, templates/workspaces/base/template_schema.json:253-281 requires auth_type in its if; an existing resource with auth_type: "Manual" is therefore evaluated as the else branch here, and a newly added required client_id is not rendered even though the button's separate combinedState validation keeps Upgrade disabled. Include existing selector values in the form's condition-evaluation state while still extracting only new fields for the PATCH payload.

                    schema={finalSchema}
                    formData={newPropertyValues}
                    uiSchema={uiSchema}
                    validator={validator}
                    onChange={(e) => setNewPropertyValues(e.formData)}

api_app/db/repositories/resources.py:463

• This helper removes every required constraint from the target schema when it is invoked below, including properties introduced by the upgrade and conditional then.required/else.required rules. Since merged_properties represents the resource's full target state during an upgrade, a client can omit a newly required value and validation still succeeds, leaving deployment to fail after the template version is advanced. Preserve required validation for upgrades, while explicitly handling values supplied later by pipeline substitution.

        def _strip_required(schema_node: Any):
            if isinstance(schema_node, dict):
                schema_node.pop("required", None)
                for v in schema_node.values():
                    _strip_required(v)

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:320

• newPropKeysToSend is the union of properties from both then and else branches, and this loop applies defaults without checking which branch matches the existing resource. If both branches introduce fields, defaults from the inactive branch are included in newPropertyValues and sent; the API's target schema has unevaluatedProperties: false, so that otherwise valid upgrade is rejected as containing an unexpected property. Evaluate conditionals against the merged resource state and initialize/send only active-branch keys.

This issue also appears on line 535 of the same file.

        // prefill newPropertyValues with schema defaults (excluding pipeline properties)
        const initialValues: any = {};
        newPropKeysToSend.forEach((key) => {
          const propSchema = getSchemaProperty(newTemplate, key);
          const currentValue = getNestedValue(props.resource.properties, key);

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

Suppressed comments (4)

api_app/db/repositories/resources.py:205

• The removal set is derived only from the two schemas, so it cannot clean up properties that are already stale on the resource. For example, if an earlier upgrade left foo in Cosmos after foo disappeared from the current template, foo is absent from both schema sets and survives every later upgrade. Derive removals from the actual resource.properties paths against the enriched target schema (while retaining system properties).

            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

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:457

• For a nested enum-invalid field, the visibility filter can require input even when an ancestor has tre-hidden, but this code removes the class only from the leaf. The ancestor remains hidden, so the user cannot provide the required replacement and the Upgrade button can remain disabled. Remove tre-hidden from every traversed path segment for fields made visible.

        if (i === parts.length - 1) {
          if (typeof current[part].classNames === "string") {
            current[part].classNames = current[part].classNames.replace(/\btre-hidden\b/g, "").trim();
          }
          if (typeof current[part]["ui:classNames"] === "string") {
            current[part]["ui:classNames"] = current[part]["ui:classNames"].replace(/\btre-hidden\b/g, "").trim();
          }
        } else {
          current = current[part];
        }

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:583

• An enum-invalid existing value with no target default is not blocked when the field is optional: newPropertyValues has no value, this condition skips undefined, and the required check also passes. The upgrade then sends the old invalid value from the merged resource state and backend schema validation rejects it. Require a valid replacement whenever the resource's current enum value is no longer allowed, even for optional fields.

                        const val = getNestedValue(newPropertyValues, key);

                        // Check if value is invalid enum (for both required and optional fields)
                        const propSchema = getSchemaProperty(newTemplateSchema, key);
                        if (
                          propSchema &&
                          propSchema.enum &&
                          val !== undefined &&
                          val !== "" &&
                          !propSchema.enum.includes(val)
                        ) {
                          return true;
                        }

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:564

• This external button never submits the RJSF form, and its disabled logic only reimplements required and enum checks. Other target-schema constraints such as pattern, minLength, numeric bounds, and array constraints can therefore be PATCHed even when invalid, causing the upgrade to fail at the API. Submit through the form or track validator errors and call upgradeCall only after the complete schema validates.

              <PrimaryButton
                primaryDisabled={
                  !selectedVersion ||
                  loadingSchema ||
                  (newPropertiesToFill.length > 0 &&

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

Suppressed comments (4)

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:536

• Because this reduced form uses omitExtraData with liveOmit, existing resource fields that are not in the reduced schema are removed from e.formData. If an allOf condition for a new field depends on one of those existing fields, extractNewPropertyValues evaluates the condition as inactive and drops the user's new value. Merge the persisted properties back before evaluating active branches.

                    onChange={(e) => {
                      const updatedNewVals = extractNewPropertyValues(e.formData, newTemplateSchema, allNewProperties);
                      setNewPropertyValues(updatedNewVals);
                      setFormHasErrors(Boolean(e.errors && e.errors.length > 0));

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:102

• Missing optional values are always converted to "". That persists absent optional strings and makes optional number/boolean/array properties fail target-schema validation during upgrade. Only add a key when the form actually contains a value; required fields are already blocked separately.

        setNestedValue(updatedNewVals, key, val !== undefined ? val : "");

api_app/db/repositories/resources.py:205

• This collects only leaf paths. If the target template removes a non-empty object property, its children are deleted but the object key itself remains as {}, so the upgrade does not fully remove the deleted property. Collect intermediate object paths as well for this removal pass (without changing _get_leaf_properties, which is also used by authorization validation).

            existing_paths = [path for path, _ in self._get_leaf_properties(resource.properties)]

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:552

• formHasErrors is never reset when the selected target version changes. After entering an invalid value for one version, switching to another version can leave Upgrade permanently disabled even when the new schema is valid and has no editable fields.

                    setSelectedVersion(option.text);
                    setLoadingSchema(true);

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

Suppressed comments (4)

api_app/db/repositories/resources.py:278

• This combines install- and upgrade-pipeline properties, but pipeline substitution executes only the selected primary action (service_bus/helpers.py:77-90). During an upgrade, an install-only property is therefore treated as pipeline-supplied and removed from required validation even though no upgrade step will populate it, allowing an invalid resource into deployment. Collect properties for the active phase only.

        if pipeline:
            for phase in ["install", "upgrade"]:
                if phase in pipeline and pipeline[phase]:
                    for step in pipeline[phase]:
                        if "properties" in step and step["properties"]:

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:557

• Reset formHasErrors when changing versions. Otherwise, after a user makes the first version's form invalid, selecting another version retains the stale true value and keeps Upgrade disabled even when the newly loaded schema has no errors or no fields.

                  if (option) {
                    setSelectedVersion(option.text);
                    setLoadingSchema(true);
                  }

api_app/db/repositories/resources.py:137

• The recursive key walk only follows object properties; it treats arrays as indivisible leaves and never visits items.properties. Existing templates contain arrays of objects (for example templates/workspaces/base/template_schema.json:292-323), so removing a field from an item schema will leave that field in every persisted array element, contrary to this upgrade's removal behavior. The schema/data traversal needs to recurse through array items as well.

            for k, v in properties.items():
                full_key = f"{prefix}{k}"
                keys.add(full_key)
                if isinstance(v, dict) and "properties" in v:
                    keys.update(self._get_all_property_keys_from_template(v, prefix=f"{full_key}."))

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:268

• Properties that already existed but become required in the target version are excluded here. If such a property is absent from the resource, no form is shown for it, while backend target-schema validation rejects the upgrade as missing a required property. Include missing properties that are required in the target state, not only newly named or enum-invalid properties.

        const newPropKeys = newKeys.filter((key) => {
          if (!currentKeys.includes(key)) {
            return true;
          }
          const propSchema = getSchemaProperty(newTemplate, key);

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

Suppressed comments (4)

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:537

• liveOmit strips existing resource fields because they are absent from the reduced schema. Those fields are still needed to evaluate retained allOf conditions: if a new conditional field depends on an existing property, the first edit removes the controller from e.formData, isKeyActiveInTemplate treats the branch as inactive, and the entered value is discarded. Keep extra fields during edits; extractNewPropertyValues already limits the PATCH to new keys.

                    liveOmit={true}

api_app/db/repositories/resources.py:237

• Removing only runtime leaf paths leaves deleted container properties behind. For example, if the target drops obsolete and the resource contains { obsolete: { a: 1 } }, this loop deletes obsolete.a but persists obsolete: {}; an array of objects is similarly left as an array of empty objects. Remove the highest schema path that disappeared (or otherwise prune containers recursively) so a property removed from the target template is actually removed in full.

            existing_paths = [path for path, _ in self._get_leaf_properties(resource.properties)]
            for path in existing_paths:
                if path not in target_properties:
                    self._remove_property_by_path(resource.properties, path)

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:473

• The full template's ui:order is copied into a schema that contains only upgrade fields. RJSF rejects order entries that are not properties in the rendered schema, so upgrades for existing templates with explicit orders (for example templates/shared_services/sonatype-nexus-vm/template_schema.json:67) can fail while rendering the dialog. Recursively prune each ui:order to the corresponding reduced schema, retaining or adding * for remaining upgrade fields.

  // Compose final uiSchema merging sanitizedUiSchema with our overrides
  const uiSchema = {
    ...sanitizedUiSchema,
    "ui:submitButtonOptions": { norender: true },
  };

ui/app/src/utils/schemaUpgradeUtils.ts:21

• This recursion handles nested properties but not object schemas under items. Consequently, adding a required field inside an existing array item is not detected as a new property, even though such arrays are supported by repository templates (for example templates/shared_services/firewall/template_schema.json:9-22) and the backend upgrade diff traverses items. The form/path utilities also need array-aware traversal so these upgrades can collect and submit the new item values.

    if (value && typeof value === "object" && "properties" in value) {
      // recur for nested properties
      keys = keys.concat(getAllPropertyKeys((value as any)["properties"], prefix + key + "."));
    } else {
      keys.push(prefix + key);

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

Suppressed comments (3)

ui/app/src/utils/schemaUpgradeUtils.ts:26

• Array item properties are flattened into dotted paths here (for example, rule_collections.name), but downstream getNestedValue, setNestedValue, and required-state traversal treat every segment as an object key. When an array item gains a required field, form edits cannot be extracted into newPropertyValues, so the PATCH omits them and the upgrade fails validation. Either retain the array property as the unit of comparison/patching or add array-aware traversal that maps values across items.

        "items" in value &&
        typeof (value as any).items === "object" &&
        (value as any).items !== null &&
        "properties" in (value as any).items
      ) {
        keys = keys.concat(getAllPropertyKeys((value as any).items["properties"], prefix + key + "."));

api_app/db/repositories/resources.py:282

• This recursively deletes every empty dict/list in the resource, not just containers left empty by removed schema fields. Empty arrays are valid values in existing templates (for example, templates/shared_services/firewall/template_schema.json:9-15 defines rule_collections with default []), so any template upgrade will silently remove such properties even though they still exist in the target schema. Limit pruning to ancestors of paths actually removed, or preserve containers whose path remains defined by the target schema.

            # Prune any empty dict/list containers left behind after removal
            self._prune_empty_containers(resource.properties)

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:290

• Newly-required detection evaluates conditionals only against the old resource state. If the target adds a selector with a default that activates an allOf branch, and that branch makes a previously optional property required, the selector is added to the form but the dependent property is omitted because its condition is false before defaults are applied. Once the default activates the branch, the user has no field to satisfy it and the upgrade is blocked or rejected. Apply target defaults before this comparison and re-evaluate conditional requirements from the resulting state.

          if (currentValue === undefined && isPropertyRequiredInState(newTemplate, key, props.resource.properties)) {
            return true;

[Copilot]

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

• ui/app/package-lock.json: Generated file

Suppressed comments (3)

ui/app/src/components/shared/ConfirmUpgradeResource.tsx:631

• This treats an omitted optional enum as invalid because undefined enters this branch. A newly added optional enum without a default therefore disables Upgrade indefinitely even though the target JSON Schema and API allow it to be absent. Only validate enum membership when a value is present; the following required-field check should handle missing required enums.

                        // Check if value is invalid enum (for both required and optional fields)
                        if (
                          propSchema &&
                          propSchema.enum &&
                          (valInState === undefined ||

api_app/db/repositories/resources.py:263

• target_properties is the union of both allOf branches, so pruning retains values from a branch that becomes inactive after the upgrade. If an enum migration changes a selector from Automatic to Manual, for example, old else-only fields remain in resource.properties; validation then rejects them because resource templates default unevaluatedProperties to false. Determine active target branches from the post-patch state and remove or exclude inactive-branch fields before validation.

            enriched_target_template = resource_template_repo.enrich_template(new_template, is_update=True)
            target_properties = self._get_all_property_keys_from_template(enriched_target_template)

            # Remove at the highest path that is completely absent from the target template,
            # so that containers (e.g. obsolete: {}) and array remnants are also cleaned up.
            existing_paths = [path for path, _ in self._get_leaf_properties(resource.properties)]
            removed_top_paths: set[str] = set()
            for path in existing_paths:
                if path not in target_properties:

api_app/tests_ma/test_api/test_routes/test_workspaces.py:696

• Remove this unconditional debug print; it adds response bodies to normal test output and CI logs on every successful run.

        print("RESPONSE STATUS:", response.status_code, response.text)