Reviewing YAML Config Changes in a Pull Request Safely

A pull request that touches a YAML file is often the most dangerous kind to approve, because the diff is tiny and the effect is large. A pair of changed lines can move a setting to another section, turn a string into a number, or break the file for the next deploy. Reviewers see whitespace and punctuation where the program sees a tree of values. This post shows three diffs that look harmless, what the program reads instead, and a short routine that puts the loaded values in front of the reviewer. It extends the habits in structuring YAML configuration files that last, and it assumes you already lint your YAML.

The starting file

The base commit holds an ordinary service config:

service:
  name: orders
  python: "3.10"
  debug: false
db:
  host: db.internal
  port: 5432
  timeout: 30
  pool:
    size: 10
features:
  - search
  - export

Three changes follow, each a plausible edit by a person in a hurry. None adds more than two lines to the diff.

Change one: a quote removed

The author wants the file to look tidier and drops the quotes around the Python version:

-  python: "3.10"
+  python: 3.10

In review this reads as formatting. In the program it is a different type. YAML has no way to know that 3.10 is a version, so the parser reads it as a number, and the number 3.10 is the number 3.1. The loaded value changed from the string 3.10 to the float 3.1. A deploy script that picks an interpreter with python{version} now asks for python3.1. The full story, with other values that change type, is in YAML version numbers turning into floats, and the broader rule is that any value that looks like a number but is an identifier needs quotes.

Change two: a key moved one level

The author decides the timeout belongs with the pool settings and moves the line:

   port: 5432
-  timeout: 30
   pool:
+    timeout: 30
     size: 10

That diff is easy to read correctly if you count the leading spaces, and easy to skim past if you do not. The line left db and entered db.pool. The code that reads config["db"]["timeout"] now finds nothing, and depending on the language it falls back to a default, raises a KeyError or passes None to the driver. The file is valid and no linter objects. Only the meaning changed, and the diff expresses meaning in indentation.

Change three: a one-space slip

The third edit is a mistake, not a decision. The author re-indents a line and lands one space short:

   port: 5432
-  timeout: 30
+ timeout: 30
   pool:

Git shows a line that changed by one character at the start, which is easy to miss in a proportional font. The parser does not miss it. PyYAML raises a ParserError that starts while parsing a block mapping and ends expected <block end>, but found '<block mapping start>' at line 4, column 2, and yamllint reports the same failure with the position:

bad.yaml
  4:2       error    syntax error: expected <block end>, but found '<block mapping start>' (syntax)

This is the easy case, because a tool says no. A required CI job that runs the linter turns it into a red check before a human reads the diff. The mechanics of tab and space errors are in finding tab characters hiding in YAML.

Review the data, not the text

The first two changes pass a linter and a syntax check. To see them, compare what the two files load to. This script flattens each file into path: value pairs and prints every path that differs:

import sys
import yaml


def flat(node, path=""):
    out = {}
    if isinstance(node, dict):
        for key, value in node.items():
            out.update(flat(value, f"{path}.{key}"))
    elif isinstance(node, list):
        for i, value in enumerate(node):
            out.update(flat(value, f"{path}[{i}]"))
    else:
        out[path] = node
    return out


old = flat(yaml.safe_load(open(sys.argv[1])))
new = flat(yaml.safe_load(open(sys.argv[2])))
for path in sorted(old.keys() | new.keys()):
    a, b = old.get(path, "<absent>"), new.get(path, "<absent>")
    if a != b:
        print(f"{path}: {a!r} -> {b!r}")

Running it on the base file and the file with changes one and two applied:

git show HEAD:app.yaml > old.yaml
python yamldiff.py old.yaml app.yaml

.db.pool.timeout: '<absent>' -> 30
.db.timeout: 30 -> '<absent>'
.service.python: '3.10' -> 3.1

Three lines, and every one is a statement a reviewer can accept or reject. The quoted string became the float 3.1, and the timeout moved from .db.timeout to .db.pool.timeout. The reviewer's question changes from "do these spaces look right" to "did you mean to move the timeout, and is the version meant to be a number". The output uses repr, so the quotes around '3.10' and the missing quotes around 3.1 are visible.

yq can do a coarser version without Python. Sorting keys and printing both files as JSON makes diff show only content changes, which also hides reorderings that carry no meaning:

diff <(yq -o=json -I=2 'sort_keys(..)' old.yaml) <(yq -o=json -I=2 'sort_keys(..)' app.yaml)

On the same pair this printed the three differences as JSON hunks: size gaining a comma and a timeout entry below it, timeout leaving db, and "python": "3.10" becoming "python": 3.10. The JSON form is noisier than the flattened paths but needs only the tools you already run. See diffing YAML files without whitespace noise for more ways to normalize before comparing.

Put the check where reviewers cannot skip it

A routine only helps if it runs on every pull request. A workable setup has three steps in CI:

1. Lint every changed YAML file with yamllint. It catches syntax, tabs, duplicate keys and trailing spaces.

2. Validate against a JSON Schema, so a moved key or a wrong type fails. Validating config YAML with JSON Schema shows the setup, and a schema with a string type for python rejects the float.

3. Post the data diff as a comment or a job summary, with the changed paths from the script above.

The third step is the one people skip, and it is the one that helps the human. A schema can say a value has the wrong type; it cannot say you meant to move the key. A short list of changed paths gives the reviewer the question to ask. When the script prints nothing, the change is formatting only, and the pull request can be approved quickly with confidence that nothing the program reads has changed.

For files with anchors and merge keys, load before comparing, as the script does. A one-line change to an anchored mapping alters every place that aliases it, and only the loaded data shows how many. The aliasing rules are in YAML anchors, aliases and merge keys.

What a reviewer still checks by hand

Tools report differences; they do not judge them. Keep a short list in the review template:

• Is every changed value intended, and is it safe in every environment the file reaches?

• Did a secret or a hostname land in the file? Config files drift toward holding both.

• Does the code that reads the file change in the same pull request, when a key is renamed or moved?

• For lists, did the order change when the order matters, such as a middleware chain or a list of migrations?

A reviewer who has these four questions and a list of changed paths can review a YAML change in a couple of minutes. A reviewer with only the text diff is guessing about indentation.

When a text diff is enough for a YAML review

The file is data nobody executes. A changelog, a list of contributors or a documentation index can be reviewed as text. The risk of a moved key is low when nothing reads the structure.

The change is an addition of whole entries in a flat list. Adding a line to a list of allowed hosts is visible in a text diff, and the list has no nesting to get wrong. A schema still helps with the entry's format.

You already have a schema that pins the structure. If the schema allows each key in exactly one place and fixes its type, a moved key or a float for a string fails the check. The data diff then adds less, though it still shows value changes the schema allows.

The file is generated. Review the generator and its input. Reading the output of a generator line by line is the wrong place to look for intent.

The tools to run before approving are small. A YAML formatter normalizes layout so the text diff shows only real changes, the yamllint documentation lists the rules to turn on, and the YAML 1.2.2 specification defines how the unquoted 3.10 resolves to a number.