-
Notifications
You must be signed in to change notification settings - Fork 771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Module resource outputs can't be reference outside of the module #6065
Comments
We discussed an outlining solution to this problem as a way to make resource-typed outputs more deterministic for the deployment engine to reason about. Here's an example of what I mean by outlining: // my-module.bicep
resource sql 'Microsoft.Sql/servers@2021-08-01-preview' = existing {
name: 'my-db'
properties: {
}
}
output resource sql = sql // main.bicep
module mod './my-module.bicep' = {
name: 'my-module'
}
output string hostname = mod.outputs.sql.properties.hostname The output expression in
This is the double-reference that's causing heck in the deployment engine. Since the thing inside the reference An outlining solution would simplify this expression inside
This is a much simpler expression, and the hope is that the deployment engine can reason about it. The limitations brought about by outlining are twofold:
I think we fail this second test in some really common cases. For example consider what happens when The really vexing cases involve some commonly-used patterns for resource naming. Here's an example of a change we could make to // my-module.bicep
resource sql 'Microsoft.Sql/servers@2021-08-01-preview' = existing {
name: 'my-db-${uniqueString(resourceGroup().id)}'
properties: {
}
}
output resource sql = sql If we outline the name expression, we end up with:
The problem of course is that a function like |
If we are dealing with a module that is also written in Bicep, the compiler could update the outputs of the nested deployment to account for resource properties accessed in the parent template. An "output inlining" solution might look like this: Module template: {
...
"outputs": {
"sql": {
...
},
"sql_properties_hostname": {
"type": "string",
"value": "[reference('sql').hostname]"
}
}
} Parent template: {
...
"outputs": {
"hostname": {
"type": "string",
"value": "[reference('mod').outputs.sql_properties_hostname]"
}
}
} This is essentially what a user would do in a template directly authored in ARM JSON, and Bicep could make this pattern easier. However, I see two major drawbacks to the "output inlining" solution:
I'm not sure this issue could be fully solved without making |
I think you could make this work if you could do the same transformation for each case. We already include enough metadata that the semantic model for an ARM-JSON template knows about resource-typed-outputs. The bigger problem is secrets since outputs are not supposed to contain them, or array access inside a loop. |
This idea is half-baked at the moment, but I wonder if there should be a type-level distinction here? Perhaps // my-module.bicep
resource sql 'Microsoft.Sql/servers@2021-08-01-preview' = existing {
name: 'my-db'
properties: {
}
}
output resourceId sql = sql.id // or maybe just `= sql` Both // my-other-module.bicep
param sql resource
var hostname = sql.properties.hostname // main.bicep
module mod './my-module.bicep' = {
name: 'my-module'
}
module otherMod './my-other-module.bicep' = {
name: 'my-other-module'
params: {
sql: mod.outputs.sql // totally legit! no warnings or errors
}
}
output string hostname = mod.outputs.sql.properties.hostname // error! resourceId type has no `properties` property That sounds like a lot of complexity to encode in the type system, but it would surface the distinction between a |
I'm not sure from your description why the distinction is valuable to users 🤔 . The goal of the work I started was to just use resources everywhere, whether your goal is to access the id or access the resource body. If we can nail that promise then it seems overall simpler. |
The distinction is valuable only in the sense that it turns the runtime error reported by the OP into a compile-time error. The ARM engine doesn't allow lazy evaluation of the resource ID, so a template can't directly use a resource output by a module. Taking a resource output from one module and providing it as a parameter for another module is allowed, though, because it doesn't result in a |
@jeskew - I think I see what you're suggesting now, but it feels like a workaround to me without solving the problem of dynamic lookups in the deployment engine. I think what you're proposing is reasonable, coherent, and could be implemented. All of the scenarios where I've wanted to use resource-typed-outputs require accessing the resource body. If there are scenarios that just require the identifiers and we want to unblock them then it makes sense - to me it just feels underwhelming, but I just might be missing of the use cases for ( If we're able to implement dynamic lookups in the future would we regret implementing "typed resource ids" now? Alternative if we're able to implement "output secrets" then the output outlining approach you described might work really well. |
It's very important to consider the impact to what-if and preflight here. I hypothesize that the 99% case is users wanting to output resources from a module whose identifying characteristic are static w.r.t the outer template, and would certainly want the deployment engine to be able to deterministically calculate the deployment graph at the start of the deployment - in order to support what-if & preflight. If we come up with a solution that allows 'unsafe' behavior w.r.t predictability, I think we need to surface this to users so that they can consider the tradeoffs. Think SQL cursors - powerful but most of the time worth avoiding as they impact the query plan. |
We discussed this yesterday as a team, and the consensus was that we should block resource-typed module outputs while we explore updating the ARM deployment engine to support nested reference expressions. The behavior currently supported under the // my-module.bicep
resource sql 'Microsoft.Sql/servers@2021-08-01-preview' = existing {
name: 'my-db'
}
output dbName string = sql.name // my-other-module.bicep
param dbName string
resource sql 'Microsoft.Sql/servers@2021-08-01-preview' = existing {
name: dbName
}
output hostname = sql.properties.hostname // main.bicep
module mod './my-module.bicep' = {
name: 'my-module'
}
module otherMod './my-other-module.bicep' = {
name: 'my-other-module'
params: {
sql: mod.outputs.sql
}
}
output string hostname = otherMod.outputs.hostname
|
While working on a proof-of-concept of the outlining approach proposed above, I encountered a few cases where the ARM code generated would be less than ideal.
{
...
"parameters": {
"foo": {
"type": "string,
},
"bar": {
"type": "string",
"defaultValue": "[dataUri('snap')]"
},
"baz": {
"type": "int",
"defaultValue": "[add(mul(2, 2), 2)]"
}
},
"resources": [
{
"type": "Microsoft.Sql/servers",
"apiVersion": "2021-08-01-preview"
"name": "[parameters(parameters('foo'))]"
....
}
],
"outputs": {
"sql": {
"type": "string",
"value": "[reference(parameters(parameters('foo')).id]",
"metadata": {
"resourceType": "Microsoft.Sql/servers@2021-08-01-preview"
}
}
}
} and the following parent template: param fizz string
module mod './nested.json' = {
name: 'mod'
params: {
foo: fizz
}
}
output hostname = mod.outputs.sql.properties.hostname Codegen in the parent template is obliged to generate an ARM expression like
with the generated code getting more and more unwieldy if the nested template declares additional parameters or uses complex default value expressions. I can see this easily running into ARM's expression length limit or pushing a template over the 4MB limit if a multi-KB expression needs to be repeated multiple times.
param num int
var arr = [for i range(0, 100): {
foo: 'foo${i}'
}]
resource sql 'Microsoft.Sql/servers@2021-08-01-preview' = existing {
name: 'server-${arr[num]}'
}
outputs resource sql = sql and the following parent template: param parentNum int
module mod './nested' = {
name: 'mod'
params: {
num: parentNum
}
}
output hostname = mod.outputs.sql.properties.hostname the generated code for
As with case 1, my main concern here is running into template limits for more complex examples or repeat inclusions. Some cases along these lines could be simplified via constant folding once that lands in the compiler, but only the ARM runtime would be able to simplify expressions that contain a |
Given we already have some cases where customers are hitting the 4MB limit, I am not particularly concerned with making the problem worse. I would optimize for enabling the functionality and then later we can figure out how to generate more efficient JSON and/or raise the 4MB limit. |
Bicep version
run
bicep --version
via the Bicep CLI,az bicep version
via the AZ CLI or via VS code by navigating to the extensions tab and searching for BicepOn main with #4971 enabled.
Describe the bug
Currently output that are resources can't be used to access properties on the resource.
This is a contrived example which doesn't necessarily work, but gets the point across.
Additional context
The main problem here is that there seems to be a double reference call when you access
mymodule.outputs.sql.properties.administratorLogin
The text was updated successfully, but these errors were encountered: