Skip to content
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

Only add an explicit dependency on an existing resource when the deployments engine will use the GET response #15580

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

jeskew
Copy link
Contributor

@jeskew jeskew commented Nov 14, 2024

Resolves #15513
Resolves #15686

Microsoft Reviewers: Open in CodeFlow

Bicep will normally generate an explicit dependency when one resource refers to another. For example, if the body of b includes a symbolic reference to a, then in the compiled JSON template, the declaration for b will have a dependsOn property that includes a.

However, if a is an existing resource and the template is not being compiled to language version 2.0, then the compiler will "skip over" a and have b depend on whatever a depends on. For example, for the following template:

resource a 'type@version' existing = {
  name: 'a'
  dependsOn: [
    c
  ]
}

resource b 'type@version' = {
  name: 'b'
  properties: {
    foo: a.properties.bar
  }
}

resource c 'type@version' = {
  name: 'c'
}

the non-symbolic name output will have b depend on c.

#15447 added a couple of scenarios in which Bicep would skip over an existing resource even if compiling with symbolic name support. This was done because the ARM backend will perform a GET request on any existing resource in the template unless its body properties are never read and no deployed resource explicitly depends on it. The extra GET requests could sometimes cause template deployments to fail, for example if the deploying principal had permission to use secrets from a key vault as part of a deployment but did not have more generic /read permissions on the vault.

#15447 reused some existing logic for skipping over an intermediate existing dependency that unfortunately had an underlying bug that manifested when the skipped over resource was looped and used its loop iterator to index into the dependency once removed. For example, if we modify the earlier example slightly:

resource a 'type@version' existing = [for i in range(0, 10): {
  name: 'a${i}'
  dependsOn: [
    c[i]
  ]
}]

resource b 'type@version' = {
  name: 'b'
  properties: {
    foo: [for i in range(0, 10): a[i].properties.bar]
  }
}

resource c 'type@version' = [for i in range(0, 10): {
  name: 'c${i}'
}]

Then in the compiled output, b will have an explicit dependency on [resourceId('type', format('c[{0}]', copyIndex()))]. Because b is not looped, the deployment will fail. Related issues will occur if b indexes into a with a more complex expression or if there is an intervening variable.

This PR updates explicit dependency generation to take all steps between a depending resource and its dependency into account when generating index expressions. For example, in the following template:

resource vnets 'Microsoft.Network/virtualNetworks@2024-03-01' = [for i in range(0, 2): {
  name: 'vnet${i}'
}]

resource subnets 'Microsoft.Network/virtualNetworks/subnets@2024-03-01' existing = [for j in range(0, 10): {
  parent: vnets[j % 2]
  name: 'subnet'
}]

resource vault 'Microsoft.KeyVault/vaults@2023-07-01' = [for k in range(11, 10): {
  name: 'vault${k}'
  location: resourceGroup().location
  properties: {
    sku: {
      name: 'standard'
      family: 'A'
    }
    tenantId: subscription().tenantId
    networkAcls: {
      virtualNetworkRules: [{
        id: subnetIds[k - 11]
      }]
    }
  }
}]

var subnetIds = [for l in range(20, 10): subnets[l - 20].id]

vault will depend on vnets[(range(20, 10)[k - 11] - 20) % 2]. Prior to this PR, vault will instead depend on vnets[k % 2], which is the wrong vnet.

Copy link
Contributor

github-actions bot commented Nov 14, 2024

Test this change out locally with the following install scripts (Action run 12017232635)

VSCode
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-vsix.sh) --run-id 12017232635
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-vsix.ps1) } -RunId 12017232635"
Azure CLI
  • Mac/Linux
    bash <(curl -Ls https://aka.ms/bicep/nightly-cli.sh) --run-id 12017232635
  • Windows
    iex "& { $(irm https://aka.ms/bicep/nightly-cli.ps1) } -RunId 12017232635"

Copy link
Contributor

github-actions bot commented Nov 14, 2024

Dotnet Test Results

    78 files   -     39      78 suites   - 39   29m 33s ⏱️ - 15m 13s
11 463 tests +     4  11 463 ✅ +     5  0 💤 ±0  0 ❌  - 1 
26 657 runs   - 13 237  26 657 ✅  - 13 236  0 💤 ±0  0 ❌  - 1 

Results for commit 7f65b4d. ± Comparison against base commit c63bad7.

This pull request removes 1845 and adds 659 tests. Note that renamed tests count towards both.

		nestedProp1: 1
		nestedProp2: 2
		prop1: true
		prop2: false
	1
	2
	\$'")
	prop1: true
	prop2: false
…
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000
�ӽ\u000e� \u0010\u0007p�>�O��\u0001�\u001d�;�\u0015Hkҏ��b4i��šI\u0007M\u0017�M�o`�r��y�����Ǣnx&$���M\u0004����\u0000400/�	�52���O2�m���(K���0K�?�E\u000eF�&$��H�-�&�\u000e�5Hn֟��]]�9~i*74�S
k_4\u001a����z�O�\u0017\u0002\u0019(�0,�C�A���E2����?֞ ��(Z�\u0013�=��\u0000\u000c\u0000\u0000,"The path: index.json was not found in artifact contents")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000
�Խ
�0\u0010\u0000��>E�\u0003Ĥ�k��.��\u000f\u0010�\u0013+��6BA|w�A\Z\�#�o\u0008!w�K����݂ɠnHD�Z\u001346�(!z�;Lh��\u001bETK�\u0011�����7�Ԯ�9��Aa���\u000bH��T�b��\u0019�4^�X:\u0014c1��='y�AK.ͭ�ҋ�����
���tW���)\u0017��\u0001!�\u0006%\u0011UR�l<KO�y�?pp(�� ����8B\u001dl��\\u001bx.]��y�7�\u0017J�\u000f%\u0000\u000c\u0000\u0000,"Value cannot be null. (Parameter 'source')")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000
��\u000fH��HMLI-*ֳ006��c�60\u0000\u00023\u0013\u0013�� `hb�`h\u000e\u0003\u0006\u000c\u0006榦�\u000c\u000c
\u0015Tw	\u0016PZ\�X\u0004t
=�\u001a���B!�$37�����������X������Ԕ\u000b(��K��R� �$�63/%�B/�8?\u000f�<\u0017����(�\u0001\u0010�G� "��
�L\u0018�e��!�03e00355\u0006�����\u0011����<��\u001c}<]⽂����\u0014�%�@�i\u0014��Q0
F\u0001�\u0001\u0000�z��\u0000\u000c\u0000\u0000,"'7' is an invalid end of a number. Expected a delimiter. Path: $.INVALID_JSON | LineNumber: 0 | BytePositionInLine: 20.")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000\u0003�ӽ
�0\u0010\u0007��>E� ��\\u001a\u001d�;�
A\u000b~�T��\u0005��M\u0007��ť���
\u0019r\u001cw!�U�s�6w����\u0006\u000b�\u0012S� e\u001e��A��\u0005\u0004X�Z\u0008�M�ɀ�n\\u0015V�c�\u000f��tͩ�3��LJ�6*5��f\u0015j��\u001aʫk�Y�/��yu�K�7\u0014c
K?4\u001a���ߚ�)�\u0000$�-a8�1�\u001fuȣ�%�����c�
�(��%<\u0001	e��\u0000\u000c\u0000\u0000,"The path: index.json was not found in artifact contents")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000\u0003�Խ
�0\u0010\u0000��>E�\u0001һ&iTpw\u0011\|�hOTl-M�������\u001f0ߐ!wp\u0017�HV�Y�ͩvB��D��A�)�y�Be\u0018�\u0007``�\u0006�x3x'\u001d.��:��Z?(�p�\u000f\u0005���Tg��(2��`F!���W���2�F\u001cݹlӋ��o?3������x5� \u0015\u000b; �p��\u000c2\u001d6\u0002�\u001f��?��+\u001f�Kj*�zʗTl�\u001e��Ξ\u001cݾ�[\u0014EQ�>w�Q��\u0000\u000c\u0000\u0000,"Value cannot be null. (Parameter 'source')")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Bicep_compiler_handles_corrupted_extension_package_gracefully (\u001f�\u0008\u0000\u0000\u0000\u0000\u0000\u0000\u0003�Խ
�0\u0010\u0000��>E�\u0003���\u0015:\u0008\u000eV�
��\u0004\u001b�B��\u0015
��� .-.m\u0015�7d�\u001d܅p����WF'�(1\u0003\u0005\u0001F}\u0003Kr�z� \!�^\u0000�\u0012\u0002\u0010���;iq/+]�Vƨ�����U���(F���\u0011,\u0019�\u001c���N]12�n�:�i��\u001a_�kޤg]��~��������4�@9�;�\u0012{\u0010�\u0010H����Qf�����G�a�����~\u001b�s�n�o��8��\u000c�	�X\u0011/\u0000\u000c\u0000\u0000,"'7' is an invalid end of a number. Expected a delimiter. Path: $.INVALID_JSON | LineNumber: 0 | BytePositionInLine: 20.")
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Repository_not_found_in_registry (ArtifactRegistryAddress { RegistryAddress = mcr.microsoft.com, RepositoryPath = unknown/path/az, ExtensionVersion = 0.0.0-placeholder },Azure.RequestFailedException: The artifact does not exist in the registry.
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 138,[(BCP192, Error, Unable to restore the artifact with reference "br:mcr.microsoft.com/unknown/path/az:0.0.0-placeholder": The artifact does not exist in the registry.)])
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Repository_not_found_in_registry (ArtifactRegistryAddress { RegistryAddress = mcr.microsoft.com, RepositoryPath = unknown/path/az, ExtensionVersion = 0.0.0-placeholder },Azure.RequestFailedException: The artifact does not exist in the registry.
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 138,[(BCP192, Error, Unable to restore the artifact with reference "br:mcr.microsoft.com/unknown/path/az:0.0.0-placeholder": The artifact does not exist in the registry.)])
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Repository_not_found_in_registry (ArtifactRegistryAddress { RegistryAddress = unknown.registry.azurecr.io, RepositoryPath = bicep/extensions/az, ExtensionVersion = 0.0.0-placeholder },System.AggregateException: Retry failed after 4 tries. Retry settings can be adjusted in ClientOptions.Retry or by configuring a custom retry policy in ClientOptions.RetryPolicy. (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443))
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.<>c__DisplayClass4_0.<<PullArtifactAsync>g__DownloadManifestInternalAsync|0>d.MoveNext() in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 44
--- End of stack trace from previous location ---
   at Bicep.Core.Registry.AzureContainerRegistryManager.PullArtifactAsync(RootConfiguration configuration, IOciArtifactReference artifactReference) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 51
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.<>c__DisplayClass4_0.<<PullArtifactAsync>g__DownloadManifestInternalAsync|0>d.MoveNext() in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 44
--- End of stack trace from previous location ---
   at Bicep.Core.Registry.AzureContainerRegistryManager.PullArtifactAsync(RootConfiguration configuration, IOciArtifactReference artifactReference) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/AzureContainerRegistryManager.cs:line 63
   at Bicep.Core.Registry.OciArtifactRegistry.TryRestoreArtifactAsync(RootConfiguration configuration, OciArtifactReference reference) in /home/runner/work/bicep/bicep/src/Bicep.Core/Registry/OciArtifactRegistry.cs:line 499,[(BCP192, Error, Unable to restore the artifact with reference "br:unknown.registry.azurecr.io/bicep/extensions/az:0.0.0-placeholder": Retry failed after 4 tries. Retry settings can be adjusted in ClientOptions.Retry or by configuring a custom retry policy in ClientOptions.RetryPolicy. (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)))])
Bicep.Core.IntegrationTests.AzTypesViaRegistryTests ‑ Repository_not_found_in_registry (ArtifactRegistryAddress { RegistryAddress = unknown.registry.azurecr.io, RepositoryPath = bicep/extensions/az, ExtensionVersion = 0.0.0-placeholder },System.AggregateException: Retry failed after 4 tries. Retry settings can be adjusted in ClientOptions.Retry or by configuring a custom retry policy in ClientOptions.RetryPolicy. (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443))
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.<>c__DisplayClass4_0.<<PullArtifactAsync>g__DownloadManifestInternalAsync|0>d.MoveNext() in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 44
--- End of stack trace from previous location ---
   at Bicep.Core.Registry.AzureContainerRegistryManager.PullArtifactAsync(RootConfiguration configuration, IOciArtifactReference artifactReference) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 51
   at Bicep.Core.Registry.AzureContainerRegistryManager.DownloadManifestAndLayersAsync(IOciArtifactReference artifactReference, ContainerRegistryContentClient client) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 138
   at Bicep.Core.Registry.AzureContainerRegistryManager.<>c__DisplayClass4_0.<<PullArtifactAsync>g__DownloadManifestInternalAsync|0>d.MoveNext() in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 44
--- End of stack trace from previous location ---
   at Bicep.Core.Registry.AzureContainerRegistryManager.PullArtifactAsync(RootConfiguration configuration, IOciArtifactReference artifactReference) in D:\a\bicep\bicep\src\Bicep.Core\Registry\AzureContainerRegistryManager.cs:line 63
   at Bicep.Core.Registry.OciArtifactRegistry.TryRestoreArtifactAsync(RootConfiguration configuration, OciArtifactReference reference) in D:\a\bicep\bicep\src\Bicep.Core\Registry\OciArtifactRegistry.cs:line 499,[(BCP192, Error, Unable to restore the artifact with reference "br:unknown.registry.azurecr.io/bicep/extensions/az:0.0.0-placeholder": Retry failed after 4 tries. Retry settings can be adjusted in ClientOptions.Retry or by configuring a custom retry policy in ClientOptions.RetryPolicy. (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)) (No such host is known. (unknown.registry.azurecr.io:443)))])
…

♻️ This comment has been updated with latest results.

@jeskew jeskew requested review from majastrz and a team November 14, 2024 23:33
@jeskew jeskew marked this pull request as ready for review November 14, 2024 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant