feat: Support Logic App Standard

[pim-simons]

Add support for Logic App Standard in our Logic Apps scripts.

Closes #413

[netlify]

✅ Deploy Preview for arcus-scripting canceled.

Name	Link
🔨 Latest commit	f5f1ec2
🔍 Latest deploy log	https://app.netlify.com/sites/arcus-scripting/deploys/655c620c1bdb4b00081ef3e1

[pim-simons]

I couldn't find a way to implement the prefix stuff we discussed here, since this works really a lot different in Logic App Standard. Might be worth a look later on as an enhancement though.

I split up the documentation pages to Logic App - Consumption and Logic App - Standard as I feel this makes the docs much clearer.

[stijnmoreels]

I couldn't find a way to implement the prefix stuff we discussed here, since this works really a lot different in Logic App Standard. Might be worth a look later on as an enhancement though.

I split up the documentation pages to Logic App - Consumption and Logic App - Standard as I feel this makes the docs much clearer.

Yes, okay, a big thx for investigating in this and good thinking in splitting this up.
Should we add some redirects to the https://github.com/arcus-azure/arcus.scripting/blob/main/netlify.toml for that best?

[pim-simons]

I couldn't find a way to implement the prefix stuff we discussed here, since this works really a lot different in Logic App Standard. Might be worth a look later on as an enhancement though.
I split up the documentation pages to Logic App - Consumption and Logic App - Standard as I feel this makes the docs much clearer.

Yes, okay, a big thx for investigating in this and good thinking in splitting this up. Should we add some redirects to the main/netlify.toml for that best?

I honestly don't really have any knowledge on that toml file, not really sure what it does 😇
I notice that Logic App isn't in the toml file currently?

[stijnmoreels]

I honestly don't really have any knowledge on that toml file, not really sure what it does 😇 I notice that Logic App isn't in the toml file currently?

That's right. I would believe that we need to add some [redirects] at the end that redirects the previous logic apps version to the standard or consumption now (whatever you find more appropriate and/or recommended to use).

I think these two should be added:

[[redirects]]
  from = "/:version/features/powershell/azure-logic-apps"
  to = "/:version/features/powershell/azure-logic-apps-standard"

[[redirects]]
  from = "/features/powershell/azure-logic-apps"
  to = "/features/powershell/azure-logic-apps-standard"

But I also think that we should only add them when we have a new version of the documentation released, not when they are still in /preview. So I would suggest to create a task for the next version so that we take this up when we release the next version.

[pim-simons]

Ah, now I see it. This parameter is also responsible if the implementation is using consumption/standard, right?
Just a thought: this is a bit implicit, it might be a good idea to include Standard/Consumption in the function names? If yes: then we should make sure that we write an deprecated error when the 'old' ones, without the type, are still used.

Yes the WorkflowName parameter determines if the script executes the Consumption or Standard functionality since a workflow only exists in the Logic App Standard. It is a bit implicit, but I did not want to change the names of the scripts as that would break current implementations.

[pim-simons]

Yes, my concern is that this was only used for the tests, and that it could potentially be vulnerable. I'm wondering why we need it, though, as the Get-AzCachedAccessToken function already returns the same information? And once it is set, it is not used for assertion, too.

Just tried again and without the $Global:subscriptionId = '123456' the test fails with:

Cannot bind argument to parameter 'SubscriptionId' because it is an empty string.

So I honestly don't really know how else to solve this?

[stijnmoreels]

Yes the WorkflowName parameter determines if the script executes the Consumption or Standard functionality since a workflow only exists in the Logic App Standard. It is a bit implicit, but I did not want to change the names of the scripts as that would break current implementations.

Yes, that's why I suggested to maybe log a warning for the current module functions and create additional ones with Consumption/Standard? Can be in a follow-up PR.

[stijnmoreels]

Just tried again and without the $Global:subscriptionId = '123456' the test fails with:

Cannot bind argument to parameter 'SubscriptionId' because it is an empty string.

So I honestly don't really know how else to solve this?

If I read this correctly, we retrieve the $token from the Get-AzCachedAccessToken and pass that value to the next function. Why there would be a 'global' variable accessible outside the script, I don't understand directly?
https://github.com/arcus-azure/arcus.scripting/pull/415/files#diff-da181b2ee5931f86b51b900f462579712ed7a306125dfccf1a2f9ebe14e730baR25

[pim-simons]

Integration tests fail due to #416, will look into that asap.

[pim-simons]

Yes the WorkflowName parameter determines if the script executes the Consumption or Standard functionality since a workflow only exists in the Logic App Standard. It is a bit implicit, but I did not want to change the names of the scripts as that would break current implementations.

Yes, that's why I suggested to maybe log a warning for the current module functions and create additional ones with Consumption/Standard? Can be in a follow-up PR.

Lets do that in a follow up indeed, than we have a bit more time to look at this 👍🏻

[pim-simons]

If I read this correctly, we retrieve the $token from the Get-AzCachedAccessToken and pass that value to the next function. Why there would be a 'global' variable accessible outside the script, I don't understand directly?

I don't really know the background of this script.

What I think happens is that because we Mock the Get-AzCachedAccessToken the global variables are not set.
That is why the $Global:subscriptionId = '123456' is needed.
Does that make sense?

[pim-simons]

If I read this correctly, we retrieve the $token from the Get-AzCachedAccessToken and pass that value to the next function. Why there would be a 'global' variable accessible outside the script, I don't understand directly?

I don't really know the background of this script.

What I think happens is that because we Mock the Get-AzCachedAccessToken the global variables are not set. That is why the $Global:subscriptionId = '123456' is needed. Does that make sense?

As discussed, I removed the global variables from the scripts where they were not already present.

[stijnmoreels]

If we have refactored the script to something smaller, it will be easier to see if we have handled all cases. Thx for the work!

Created these follow-up issues:
#418
#419

Feel free to correct my mistakes in describing them 😉 .