Skip to content

Conversation

@detienne20
Copy link
Contributor

Description

Customer reported template specs issue (Azure/template-specs#37) where the contents of multiline strings were replaced with #Azure CLI#. This was the result of the _remove_comments_from_json function used to process the template input. This function is typically used in deployments for processing template inputs; multiline strings are processed to prompt for parameters. The full functionality is not needed for creating a template spec from a template input therefore the function has been replaced by process_template_spec in _packing_engine.py.

Example:

Template Input:
{
"variables": {
"myVar": "[if(equals(parameters('kind'), ''),
variables('resources')[variables('provider')][variables('resourceType')],
variables('resources')[variables('provider')][variables('resourceType')][parameters('kind')]))]"
}
}

Template Specs Created:
{
"variables": {
"myVar": "#Azure Cli#"
}
}

Testing Guide
az ts create -g resource_group --name Issue37 -v 1.0 -l westus --template-file original.json

--> Multiline strings starting on lines 2644, 2657, and 2661 should not be replaced by "#Azure CLI#"

-->Can compare template spec created with postTsCreation.json (showcases issue).
Issue37Templates.zip

@yonzhan yonzhan added this to the S178 milestone Nov 2, 2020
@yonzhan yonzhan requested a review from jsntcy November 2, 2020 22:26
@yonzhan
Copy link
Collaborator

yonzhan commented Nov 2, 2020

TS Multiline String Support

@zhoxing-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@zhoxing-ms
Copy link
Contributor

May I ask if there is a requirement in the design for template spec that it cannot be modified on the client side, but should be sent directly to the service side?
For ARM template, it is designed like this, so I would like to confirm with you whether template spec has considered this aspect.

@detienne20
Copy link
Contributor Author

detienne20 commented Nov 4, 2020

May I ask if there is a requirement in the design for template spec that it cannot be modified on the client side, but should be sent directly to the service side?
For ARM template, it is designed like this, so I would like to confirm with you whether template spec has considered this aspect.

@zhoxing-ms There is no requirement for this in Template Spec; template specs takes in a valid json object.

template_content = read_file_content(abs_local_path)
sanitized_template = _remove_comments_from_json(template_content)
template_json = json.loads(json.dumps(sanitized_template))
template_json = json.loads(json.dumps(process_template(template_content)))
Copy link
Contributor

Choose a reason for hiding this comment

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

json.loads(json.dumps(

Why serialize and then deserialize the object from process_template() ? The data returned by process_template has been deserialized~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

data returned by process_template is a ordered dictionary due to shell_safe_json_parse, the template_json needs to be a json object.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

Copy link
Contributor

@Juliehzl Juliehzl left a comment

Choose a reason for hiding this comment

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

Could you add test for the scenario?

displayName: 'Use Python 3'
inputs:
versionSpec: 3.x
versionSpec: 3.8
Copy link
Contributor

Choose a reason for hiding this comment

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

You could revert this file as this PR #15804 has fixed the build issue

Copy link
Member

@jiasli jiasli Nov 6, 2020

Choose a reason for hiding this comment

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

Or merge from release to get a clear branch history.

@yonzhan yonzhan merged commit 2a84a5b into Azure:release Nov 6, 2020
@detienne20 detienne20 deleted the daetienn/TSIssue37Expressions branch December 2, 2020 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants