Skip to content

fix: use native azurerm attribute for ACR task network bypass - #142

Open
sry9681 (sry9681) wants to merge 1 commit into
Azure:mainfrom
sry9681:fix/acr-task-bypass-native-attribute
Open

fix: use native azurerm attribute for ACR task network bypass#142
sry9681 (sry9681) wants to merge 1 commit into
Azure:mainfrom
sry9681:fix/acr-task-bypass-native-attribute

Conversation

@sry9681

Copy link
Copy Markdown

Overview/Summary

azapi_update_resource.network_rule_bypass_allowed_for_tasks (added in #102) sets networkRuleBypassAllowedForTasks on a registry that azurerm_container_registry.alz also manages.

azurerm 4.81.0 added network_rule_bypass_for_tasks_enabled to azurerm_container_registry. In the provider schema it is optional = true, computed = false, so leaving it unset means "use the schema default", not "leave the existing value alone". The provider therefore reverts what azapi wrote, and every subsequent plan against a private networking deployment shows:

~ azurerm_container_registry.alz[0]
    ~ network_rule_bypass_for_tasks_enabled = true -> false

Applying that reverts the bypass. With public_network_access_enabled = false this is the flag the build task relies on to reach the registry, so image builds are at risk.

This sets the attribute natively and removes the azapi workaround.

This PR fixes/adds/changes/removes

  1. Sets network_rule_bypass_for_tasks_enabled = var.use_private_networking on azurerm_container_registry.alz.
  2. Removes azapi_update_resource.network_rule_bypass_allowed_for_tasks and its reference in the azurerm_container_registry_task_schedule_run_now.alz depends_on. Ordering is preserved implicitly, since the task run already depends on the task, which depends on the registry.
  3. Raises the azurerm constraint in modules/azure/terraform.tf from ~> 4.20 to ~> 4.81.

Breaking Changes

The azurerm minimum moves to 4.81.0. Verified by schema inspection that 4.80.0 and earlier do not expose network_rule_bypass_for_tasks_enabled, so the constraint bump is required rather than optional.

Existing deployments are not modified. azapi_update_resource destroy is a state only operation, and the azurerm resource asserts the same value, so the registry is left as is.

Testing Evidence

Tested against a deployed bootstrap using self hosted runners with private networking, where the registry already carried the flag set by the current code.

Registry state before, showing the azapi value in place:

{
  "publicAccess": "Disabled",
  "bypassOption": "AzureServices",
  "bypassForTasks": true,
  "sku": "Premium"
}

Plan on the current main, showing the reverting diff:

~ azurerm_container_registry.alz[0]
    ~ network_rule_bypass_for_tasks_enabled = true -> false

Plan: 4 to add, 1 to change, 0 to destroy.

Plan on this branch, against the same state and the same registry:

# module.azure.azapi_update_resource.network_rule_bypass_allowed_for_tasks[0] will be destroyed
# (because azapi_update_resource.network_rule_bypass_allowed_for_tasks is not in configuration)

Plan: 4 to add, 0 to change, 1 to destroy.

0 to change confirms the diff is resolved. The single destroy is the azapi resource leaving state; the registry itself is untouched.

terraform validate passes and make fmt reports no further changes.

As part of this Pull Request I have

  • Checked for duplicate Pull Requests
  • Associated it with relevant issues, for tracking and closure.
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Performed testing and provided evidence.
  • Updated relevant and associated documentation.

The azapi_update_resource added in Azure#102 sets networkRuleBypassAllowedForTasks
on a registry that azurerm_container_registry also manages. azurerm 4.81.0
added network_rule_bypass_for_tasks_enabled as optional and non-computed, so
leaving it unset means the provider reverts the azapi value on every plan.

Set the attribute natively and drop the azapi workaround. Requires azurerm
>= 4.81.
@sry9681

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant