Pull Request Review for Configuration Changes¶

Every change to the configuration repository goes through a pull request (PR) and a second pair of eyes (Making a change safely). This page gives the author and the reviewer a shared checklist. It applies to the platform team in both operating models, and to workload teams if your platform lets them prepare changes (for example firewall rules).

The step-by-step runbooks for each type of change (what to open, what to check, how to deploy and how to close the ticket) are in Reviewer and operator runbooks. This page holds the checks that apply to every pull request.

Who does what¶

Step	Who	Notes
Prepare the change and open the PR	The requester, or the platform team on behalf of the requester	If the platform team prepares it for a workload team, add the owner of the workload as a required reviewer and don't merge before they approve.
Approve the content	The owner of the workload, where your process requires it	They confirm that the change matches what they asked for.
Review technically	A platform team engineer who did not write the change	Four-eyes principle.
Merge and deploy	The platform team	The requester doesn't complete their own PR. Don't enable auto-complete on a change that needs a separate approval.

In the managed-service model the contract decides who approves and how long deployment may take. In the self-managed model your own team decides. A pull request that changes a workload's configuration without the involvement and approval of its owner may be rejected.

Branch, title and description¶

Branch name: change/<request-id>-<short-description>, as in Making a change safely.
Title: a fixed pattern helps to scan the list, for example [<WORKLOAD>] <Action> - <short description> (for example [APP1] Add firewall rules - database connectivity).
Description: what the change does, the workload, the business reason, the link to the request, and which environments it touches. For firewall rules, describe the traffic: from where, to where, which port.
Link the request (ticket or work item) so the change record and the code stay together.

Before you review the content¶

Check the state of the pull request first. A pull request that is not ready costs a reviewer's time.

Check	If it is not in order
The pull request is not a draft	Send it back: the author finishes it first.
The required checks (the validation pipeline) passed	If the check expired, queue it again. If it failed, send it back with the error.
There are no merge conflicts	Send it back: the author resolves them (Resolve merge conflicts).
All comments are resolved	Send it back until they are.
A work item or ticket is linked	Link it. When the platform requires a link for every pull request but yours has no ticket, ask for one.

Send it back in the same way every time, so the author knows what to do and nothing gets lost:

Go to the ticket that is linked to the pull request.
Mention the author and say what is wrong and what must change.
Put the ticket back in the state New (or the equivalent in your tooling) and assign it to the author.

The author fixes the pull request and moves the ticket forward again (The standard change process).

Complete the pull request¶

Set auto-complete on the pull request only when no separate approval is needed, and then:

switch off complete the linked work items after merging, because the ticket is resolved after the deployment is verified, not after the merge;
switch on delete the source branch after merging.

The author does not complete their own pull request.

What the reviewer checks¶

Every PR¶

The change matches the request, and nothing else changed.
The same change is made in each platform environment folder that needs it (for example canary and prod), and applied to canary first.
The JSON is valid and the config tests pass (pwsh ./dev/dev.ps1 -Action Test, see Run the tests locally).
Names and priorities are unique and follow your naming scheme.
The PR is small: one logical change, and for the firewall one rule collection group.
The plan (not only the diff) shows only the expected adds and changes. Stop at any destroy or must be replaced that nobody expects. A whole folder of resources being destroyed usually means a wrong path.

Firewall rules and IP groups¶

The right team owns the file (see Firewall rule and IP group guidelines).
A canary file and a prod file exist, with the same change (unless the rule is meant for one environment only).
Global or regional: the rule is in the folder that fits its content (Global or regional rule).
The rule type fits: application rule for internet FQDNs, network rule for internal traffic.
The destination is as specific as possible and the ports are exact. No * or Any without a written reason.
Only one source type and one destination type per rule. Referenced IP groups exist: the file is already in the repository, or it is part of the same pull request, in an ip_groups folder of the same platform environment (and region).
When a priority field changed: the NAT collections have a lower number than the network collections, and these a lower number than the application collections.
Priorities sit inside the owner's range and are unique.
When a rule replaces another: the new rule is added first, and the old rule is removed in a later PR.
When an IP group is renamed or removed: references are changed first, in an earlier PR.

IP group files¶

The file is in an ip_groups folder, in both canary and prod, and the name follows your scheme.
cidrs contains valid IPv4 addresses or CIDR ranges that are agreed for the purpose. No unrelated subnets in one group.
When a group is renamed or removed: see the two-step rule above.

See IP group change.

Spokes¶

workloadShortName and environment of an existing spoke are unchanged.
The file name follows your convention and is unique across all sub-folders of spokes/ (for example canvas_<workload>_<environment>.json).
azureSubscriptionId is filled in with the existing subscription for a new spoke. Who creates the subscription depends on your process (New spoke).
Storage account and Key Vault: public access is off (EnablePublicAccess / enablepublicaccess is false) and the private endpoint is on (CreatePrivateEndpoint / createprivateendpoint is true). If not, ask the requester whether that is intended. For the storage account also check infrastructure_encryption_enabled (true) and allow_nested_items_to_be_public (false).
There is exactly one resourceconfiguration.network entry for each region in targetRegions.primary and targetRegions.additional.
Every subnet range lies inside the address space of its virtual network, and the subnets do not overlap. Use a subnet calculator if needed.
Each network has a subnet with isprivatelinksubnet set to true. One is the convention. The code takes the first one.
Every network security group named in linkednetworksecuritygroupname exists in networksecuritygroups of the same network.
Address ranges don't overlap with any other range in the IP plan, and the range is recorded in the plan. The code does not check overlaps.
Delegations, service endpoints and NSG rules match the request. An existing NSG rule keeps its priority.
Team group names exist in Entra ID.

Application Gateway¶

Every reference (pool, setting, listener, port, certificate) points to an existing key.
Each HTTPS listener has a certificate, and the certificate secret exists in the gateway Key Vault.
The network path to the backend is open (firewall rule and NSG).
A listener and its port match: Https with the HTTPS port, Http with the HTTP port. Routing rule priorities are unique.
For a WAF policy file: see WAF policy review.

Managed VMs¶

The file is in the folder of the right spoke, follows the naming for the environment, and the folder is listed in the module pipeline.
A new file looks like a working one: compare them, and check that all variables are present.
See Managed VM pull request review.

Policies¶

Assignment names are unique and at most 24 characters at management group scope.
Exemptions have an approval reference, a rationale and an end date (Policy exemptions).

Deleting configuration¶

Deleting is the most expensive mistake, because it destroys real resources.

Before you delete a configuration, compare its name with the actual resources in Azure. Names that contain an environment (for example a configuration set called prd that builds a VM with prd in its name) are easy to mix up. Avoid environment names in the names of configuration items, and ask a second person to confirm the target before the PR is merged.
Ask the requester to confirm the delete in writing, with the name of the resource.
Read the plan and check that only the intended resources are destroyed.

Resolve merge conflicts¶

A conflict means that someone changed the same file after you created your branch. Bring your branch up to date and resolve the conflict in your own branch:

git fetch origin
git switch <your branch>
git merge origin/main

Git lists the conflicting files. Open each file and keep the lines you need from both sides (an editor with a merge view shows the incoming change, your change and the result). Check that the JSON is still valid, run the config tests, then commit and push:

git add <resolved files>
git commit
git push

Resolving a conflict depends on the change. Ask a colleague when you are not sure what the other change did.

Optional: automated review comments¶

Some teams add a pipeline step to the PR that sends the diff to an AI model and posts the findings as comments. This can find mistakes early, for example a missing key or an inconsistent name. Rules for using it:

Treat the comments as advice. They don't replace the human review and the plan check.
Give the model the same guidelines you give a human, for example the Firewall rule and IP group guidelines, as part of the prompt.
Limit the size of the diff you send, and don't send secrets or personal data.
Give the pipeline identity only the permission it needs to post comments.