Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .github/workflows/api-diff.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: API diff
#
# Breaking-change gate on the OpenAPI contract (#196).
#
# Cryptify's routes are unversioned, so there is no way to keep an old shape
# running next to a new one: any breaking change to api-description.yaml
# breaks the clients pinned to it (pg-js, pg-dotnet, the add-ins). This job
# diffs the PR's spec against the branch it targets and fails on anything
# oasdiff rates ERR. Additive changes pass.
#
# To land a genuinely breaking change: add a new versioned route, leave the
# old one in place, and deprecate it once privacybydesign/postguard-ops#64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit — dead cross-reference.

privacybydesign/postguard-ops#64 does not resolve: the repo 404s on the API and is absent from a full repo list of both orgs. The string appears nowhere else in cryptify.

This is the one pointer a future maintainer follows to answer "when may I drop the old route", so a 404 there costs real time. #196 and encryption4all/postguard#247 both resolve if you want an existing tracker, or create the ops issue and update the line.

# telemetry shows nobody is calling it.
#

on:
pull_request:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit — a bare pull_request: will not re-run this gate when a PR is retargeted.

The default activity types are opened, synchronize, reopened. Changing a PR's base branch fires edited, which is not in that set. Since the verdict is derived from github.event.pull_request.base.sha (line 33), a retargeted PR keeps the check result computed against its old base.

This PR is the live example. automatic_base_change_succeeded fired at 14:29:00 when #200 merged, and it triggered no API-diff run — the gate only re-evaluated against main because of the unrelated force-push at 14:30:14 twenty seconds later. Without that push, the green you would be merging on was computed against the deleted audit branch.

The substance happens to be fine here (the spec blob is identical at both bases), and the stacked-PR flow the description recommends is exactly the flow that hits this. It matters more once this becomes a required check, because a stale green is still a passing required check. Fix is one line:

on:
  pull_request:
    types: [opened, synchronize, reopened, edited]

edited also fires on title/body edits, so this costs a few extra 13-second runs. Worth it for a gate whose answer depends on the base.


permissions:
contents: read

jobs:

breaking-changes:
name: oasdiff breaking changes
runs-on: ubuntu-latest
steps:
- name: Check out the pull request
uses: actions/checkout@v6
- name: Check out the base spec
uses: actions/checkout@v6
with:
ref: ${{ github.event.pull_request.base.sha }}
path: base
- name: Diff the spec against the base branch
uses: oasdiff/oasdiff-action/breaking@v0.1.10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style — third-party Docker action pinned to a mutable tag.

oasdiff/oasdiff-action/breaking@v0.1.10 is a runs: using: docker action (confirmed at that tag), so CI builds and runs code from that repo (62 stars) with the workspace mounted. I resolved the tag: v0.1.100ab8ad204b00d25acc5ae87106281433e288d0c1, a commit on the default branch — a tag that can be repointed.

Every other action here is first-party or well-known (actions/*, docker/*, dtolnay, Swatinem, release-plz), so this is the first genuinely small third-party action in the repo. Suggestion:

uses: oasdiff/oasdiff-action/breaking@0ab8ad204b00d25acc5ae87106281433e288d0c1 # v0.1.10

Blast radius is already small (permissions: contents: read, review: false, no secrets on a pull_request trigger), so this is hardening rather than a hole — hence style, not blocking. Note the SHA pin also makes the engine pin exact, since the tag is what currently guarantees tufin/oasdiff:v1.26.1.

with:
base: base/api-description.yaml
revision: api-description.yaml
fail-on: ERR
# Do not upload the spec to oasdiff.com for a side-by-side review
# page. The default is `true`; detection and annotations work
# without it, so nothing leaves CI.
review: false