jeffhollan / LogicAppTemplateCreator

Script to convert Logic Apps into templates for deployment
MIT License
143 stars 74 forks source link

resourceGroup().location is not valid in parameters file #11

Closed wsilveiranz closed 7 years ago

wsilveiranz commented 7 years ago

[resourcegroup().location] is an ARM template expression which gets translated to appropriate value on deployment. While parsing the logic app template to generate the parameter file paramters with ARM Template exceptions should be skipped, as they are already automated.

wsilveiranz commented 7 years ago

Add this here just for reference, but I will take a look on how to fix this one.

wsilveiranz commented 7 years ago

Do you guys think it would be as simple as filtering parameters that starts with [ like this (in ParamGenerator.ProcessRecord() ?

if (!(obj["value"].ToString().StartsWith("["))) { paramTemplate.parameters.Add(param.Name, obj); }

MLogdberg commented 7 years ago

What about if we set this parameter was set to the "current" location for the Logic App. That will solve the issue and still leave flexibility to change with deployment. What do you think?

wsilveiranz commented 7 years ago

The logic app itself have default value for location setup to resourceGroup().locationv- which guarantees that the logic app will follow the resource group when the template is used for deployment. If we try to set the parameter to the current location of the logic app (at the time of extraction), we will kind of hardcode it to the old location, and will end up overriding the resourceGroup().location.

So in one way, we already have the flexibility of deployment, and the end user can always add the parameter if the intention is to hard code. But if we add it beforehand we might end up causing more work for the developer on a regular basis (as hardcode the location for the logic app would be the exception than the rule).

BTW, I didn't added this change to the current pull request exactly because we still need to agree on what to do (I've tried locally and is working fine - once I've removed the parameters the deployment is validating).

MLogdberg commented 7 years ago

I've checked the code and now I understand the issue and yes we should add a check in the ParamGenerator.cs file under the function "CreateParameterFileFromTemplate" to skip these parameters.

So I would say that you have the solution allready =)

MLogdberg commented 7 years ago

Please go ahead and add a PR with the fix you have.

MLogdberg commented 7 years ago

Merged your fix and updates it, it threw errors if we had objects and the tests where not updated. But now this issue is solved and I'm closing this thread.