Regenerate Bicep files with new translator and update baseline#158
Regenerate Bicep files with new translator and update baseline#158jiangmingzhe wants to merge 4 commits into
Conversation
| var keyspaceName = 'resourcenameks' | ||
| var tableName = 'resourcenametbl' | ||
| var accountName = 'resourcename' |
There was a problem hiding this comment.
This looks like a regression in the generated output
There was a problem hiding this comment.
This is because the translation logic automatically evaluates the local structure in the Terraforma and places the string result in the bicep. I've made the change to keep the original function invocation and interpolation and translate to corresponding var structure in Bicep
var accountName = toLower(replace(resourceName, '-', ''))
var keyspaceName = '${toLower(resourceName)}ks'
var tableName = '${toLower(resourceName)}tbl'
| clientRegistrationEndpoint: 'https://azacceptance.hashicorptest.com/client/register' | ||
| clientSecret: null | ||
| defaultScope: '' | ||
| clientSecret: '${oauthClientSecret}' |
There was a problem hiding this comment.
The interpolation here is unnecessary - instead of:
clientSecret: '${oauthClientSecret}'
It's idiomatic to output:
clientSecret: oauthClientSecret
There was a problem hiding this comment.
Yup, I've fixed this in the new release
| publicNetworkAccess: 'Enabled' | ||
| publisherEmail: 'pub1@email.com' | ||
| publisherName: 'pub1' | ||
| virtualNetworkType: 'None' |
There was a problem hiding this comment.
What's the reason for the change in ordering of these properties? Is this to match the original source file?
There was a problem hiding this comment.
The original ordering is not preserved during translation since I was using unordered map to the intermediate nodes. I have added a list to keep the order of the properties, now the generated bicep resource properties should have the same order
| tier: 'GeneralPurpose' | ||
| identity: { | ||
| type: 'None' | ||
| userAssignedIdentities: null |
There was a problem hiding this comment.
Should we omit this property entirely instead of setting to null?
There was a problem hiding this comment.
Removed null value property emission
| resource policy 'Microsoft.DevTestLab/labs/policySets/policies@2018-09-15' = { | ||
| parent: policySet | ||
| name: 'LabVmCount' | ||
| name: 'policySets/default/LabVmCount' |
There was a problem hiding this comment.
The original code looked more idiomatic here - better to use the parent property with an existing resource, than to use a fully-qualified name
There was a problem hiding this comment.
Added the existing resource emission
jiangmingzhe
force-pushed
the
release/bicep-samples-02092025-2
branch
from
f597155 to
6cc70b0
Compare
3 participants
Translate all the Terraform files with new translator.
The new translator went through an architecture change that make it easier to support more scenarios in future.
Previous translator still has 14 translation failures, while the new translator has zero translation failures, besides the Terraform files that are skipped for translation.