Skip to content

Commit 5657740

Browse files
sethboylesclaude
andauthored
Openapi request conformance (#5463)
Check request specs against the OpenAPI description ai-assisted=yes Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 54fc6fa commit 5657740

6 files changed

Lines changed: 348 additions & 1 deletion

File tree

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
# Check the traffic our request specs already generate against the OpenAPI
2+
# description in docs/openapi, and report what it finds.
3+
#
4+
# Informational: the description already disagrees with the API in a handful
5+
# of places, so the run publishes a digest rather than failing. Dispatch it
6+
# with `strict` to see what gating would do.
7+
name: OpenAPI Conformance
8+
9+
concurrency:
10+
group: '${{ github.workflow }}-${{ github.head_ref || github.run_id }}'
11+
cancel-in-progress: true
12+
13+
on:
14+
workflow_dispatch:
15+
inputs:
16+
strict:
17+
description: 'Fail the job on a non-conforming response'
18+
type: boolean
19+
default: false
20+
pull_request:
21+
branches: [ main ]
22+
paths:
23+
- 'app/**'
24+
- 'lib/**'
25+
- 'docs/openapi/**'
26+
- 'spec/request/**'
27+
- 'spec/support/openapi_conformance.rb'
28+
- 'spec/support/request_spec_helper.rb'
29+
- '.github/workflows/openapi_conformance.yml'
30+
31+
permissions:
32+
contents: read
33+
34+
jobs:
35+
Conformance:
36+
name: Request specs vs. OpenAPI description
37+
runs-on: ubuntu-latest
38+
timeout-minutes: 45
39+
services:
40+
postgres:
41+
image: postgres:17
42+
env:
43+
POSTGRES_PASSWORD: rootpassword
44+
# Test-only durability tuning: skip the work that backs durability.
45+
command: >-
46+
postgres
47+
-c fsync=off
48+
-c synchronous_commit=off
49+
-c full_page_writes=off
50+
options: >-
51+
--tmpfs /var/lib/postgresql/data:rw,size=2g
52+
--health-cmd pg_isready
53+
--health-interval 10s
54+
--health-timeout 5s
55+
--health-retries 5
56+
ports:
57+
- 5432:5432
58+
env:
59+
DB: postgres
60+
POSTGRES_CONNECTION_PREFIX: postgres://postgres:rootpassword@localhost:5432
61+
OPENAPI_CONFORMANCE: ${{ (github.event_name == 'workflow_dispatch' && inputs.strict) && 'strict' || '1' }}
62+
steps:
63+
- uses: actions/checkout@v7
64+
- uses: ./.github/workflows/composite/setup
65+
- uses: actions/setup-node@v7
66+
with:
67+
node-version: 24
68+
69+
# openapi_first takes a single file, so the multi-file redocly source
70+
# has to be bundled before the specs can read it.
71+
- name: Build the API description
72+
working-directory: docs/openapi
73+
run: |
74+
yarn install --frozen-lockfile
75+
yarn build
76+
77+
- name: Prepare the database
78+
run: bundle exec rake db:recreate
79+
80+
- name: Run request specs against the description
81+
run: bundle exec rspec spec/request --format progress
82+
83+
- name: Publish the report
84+
if: always()
85+
run: |
86+
if [ -f out/openapi_conformance.md ]; then
87+
cat out/openapi_conformance.md >> "$GITHUB_STEP_SUMMARY"
88+
else
89+
echo '## OpenAPI conformance' >> "$GITHUB_STEP_SUMMARY"
90+
echo 'No report produced -- the run failed before the specs finished.' >> "$GITHUB_STEP_SUMMARY"
91+
fi
92+
93+
- name: Upload the report
94+
if: always()
95+
uses: actions/upload-artifact@v7
96+
with:
97+
name: openapi-conformance
98+
path: |
99+
out/openapi_conformance.md
100+
out/openapi_coverage.html
101+
if-no-files-found: warn
102+
retention-days: 7

‎Gemfile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ end
5151
group :test do
5252
gem 'factory_bot', '~> 6.5'
5353
gem 'mock_redis'
54+
gem 'openapi_first'
5455
gem 'parallel_tests'
5556
gem 'rack-test'
5657
gem 'rspec', '~> 3.13.2'

‎Gemfile.lock‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ GEM
109109
grpc (1.85.0-x86_64-linux-gnu)
110110
google-protobuf (>= 3.25, < 5.0)
111111
googleapis-common-protos-types (~> 1.0)
112+
hana (1.3.7)
112113
hashdiff (1.2.1)
113114
highline (3.0.1)
114115
httpclient (2.10.0)
@@ -127,6 +128,11 @@ GEM
127128
json-schema (6.2.1)
128129
addressable (~> 2.8)
129130
bigdecimal (>= 3.1, < 5)
131+
json_schemer (2.5.0)
132+
bigdecimal
133+
hana (~> 1.3)
134+
regexp_parser (~> 2.0)
135+
simpleidn (~> 0.2)
130136
kramdown (2.5.2)
131137
rexml (>= 3.4.4)
132138
kramdown-parser-gfm (1.1.0)
@@ -174,6 +180,11 @@ GEM
174180
bigdecimal (>= 3.0)
175181
ostruct (>= 0.2)
176182
open3 (0.2.1)
183+
openapi_first (4.0.1)
184+
drb (~> 2.0)
185+
hana (~> 1.3)
186+
json_schemer (>= 2.1, < 3.0)
187+
rack (>= 2.2, < 4.0)
177188
openssl (4.0.2)
178189
ostruct (0.6.3)
179190
parallel (2.3.0)
@@ -353,6 +364,7 @@ GEM
353364
sequel (>= 4.38.0)
354365
sexp_processor (4.17.6)
355366
simplecov (1.3.2)
367+
simpleidn (0.3.0)
356368
sinatra (4.2.1)
357369
logger (>= 1.6.0)
358370
mustermann (~> 3.0)
@@ -462,6 +474,7 @@ DEPENDENCIES
462474
newrelic_rpm
463475
nokogiri (>= 1.10.5)
464476
oj
477+
openapi_first
465478
openssl (>= 3.2)
466479
parallel_tests
467480
pg

‎docs/openapi/README.md‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,45 @@ This is useful for:
112112
- Validating a mock server's implementation against the OpenAPI spec.
113113
- Quickly checking a live API for compliance without running the full `capi-bara-tests` suite.
114114

115+
## Checking the description against the request specs
116+
117+
The Cloud Controller's rspec request specs drive the real rack app, so every
118+
example is already a request/response pair that this description either covers
119+
or doesn't. `spec/support/openapi_conformance.rb` wraps that app with
120+
[openapi_first](https://rubygems.org/gems/openapi_first) and records the
121+
verdict -- no deployed CF, no proxy, no traffic capture.
122+
123+
It is off unless `OPENAPI_CONFORMANCE` is set, and it reads the bundled
124+
description, so build that first:
125+
126+
```bash
127+
cd docs/openapi && yarn install && yarn build
128+
cd ../.. && DB=postgres OPENAPI_CONFORMANCE=1 bundle exec rspec spec/request
129+
```
130+
131+
That writes `out/openapi_conformance.md` (a digest of coverage and violations)
132+
and `out/openapi_coverage.html` (every route, with the ones nothing exercised).
133+
`OPENAPI_CONFORMANCE=strict` turns a non-conforming response into a spec
134+
failure instead of a line in the report.
135+
136+
Violations reading `does not match format: uuid` are counted but not listed.
137+
Around 250 places in `spec/request` hand a model a literal guid such as
138+
`'app1_guid'` and then assert on it, so that check only has signal against
139+
recorded traffic, not against fixtures.
140+
141+
5xx responses are left out of coverage. They are described on nearly every
142+
operation, but request specs almost never produce them, so they only drag the
143+
number down; the HTML report lists them as skipped. A skipped response isn't
144+
tracked, so a malformed 5xx body won't show up as a violation either.
145+
146+
The `OpenAPI Conformance` GitHub Action runs this on pull requests and
147+
publishes the digest to the job summary. The step log carries the same digest
148+
plus, in a collapsed "OpenAPI coverage details" group, every route that isn't
149+
fully covered -- the HTML report is also uploaded as an artifact, but there is
150+
no need to download it. It is informational -- the
151+
description already disagrees with the API in a handful of places, so there is
152+
nothing honest to gate on yet.
153+
115154
## Contributing
116155

117156
Contributions are welcome! Please feel free to submit a pull request or open an issue to discuss any changes.
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
# frozen_string_literal: true
2+
3+
# Spike: check the traffic our request specs already generate against the
4+
# OpenAPI description in docs/openapi.
5+
#
6+
# The request specs drive the real rack app through rack-test, so every
7+
# example is a request/response pair the description either covers or
8+
# doesn't. openapi_first wraps that app and records the verdict, which gives
9+
# us contract checking without a deployed CF, a proxy, or a traffic capture.
10+
#
11+
# Off unless OPENAPI_CONFORMANCE is set, so `rake spec` is untouched -- the
12+
# gem isn't even required.
13+
#
14+
# OPENAPI_CONFORMANCE=1 record violations, fail nothing
15+
# OPENAPI_CONFORMANCE=strict raise on the first request/response that
16+
# doesn't conform
17+
#
18+
# It reads the bundled description: openapi_first takes a single file and
19+
# docs/openapi is a multi-file redocly source, so `yarn build` has to run first.
20+
module OpenapiConformance
21+
DESCRIPTION = File.expand_path('../../docs/openapi/dist/latest/openapi.yaml', __dir__)
22+
OUT_DIR = File.expand_path('../../out', __dir__)
23+
HTML_REPORT = File.join(OUT_DIR, 'openapi_coverage.html')
24+
MARKDOWN_REPORT = File.join(OUT_DIR, 'openapi_conformance.md')
25+
26+
# Rows past this get cut from the markdown -- the HTML report has them all,
27+
# and a step summary that long is unreadable anyway.
28+
MAX_ROWS = 50
29+
30+
# `format: uuid` failures are ours, not the API's: 248 places in
31+
# spec/request hand a model a literal guid like 'app1_guid', and specs
32+
# assert on those literals, so they can't just be swapped for real UUIDs.
33+
# Real Cloud Controller guids are SecureRandom.uuid, so this check only has
34+
# signal against recorded traffic, not against fixtures. Counted, not listed.
35+
UUID_FORMAT_ERROR = /does not match format: uuid/
36+
37+
# Only suppress when every clause is a uuid complaint -- a compound error
38+
# like "... format: uuid. value at `/x` is not an object" still matters.
39+
def self.uuid_noise?(message)
40+
return false if message.nil?
41+
42+
clauses = message.sub(/\A(Request body invalid|Response body is invalid|Path segment is invalid): /, '').split('. ')
43+
clauses.all? { |clause| clause.match?(UUID_FORMAT_ERROR) }
44+
end
45+
46+
# openapi_first takes one reporter. This one writes the HTML report to read
47+
# after the fact, plus a short markdown digest for the CI log and the job
48+
# summary. The gem's own terminal reporter prints every route not fully
49+
# covered -- around a thousand lines -- so it only runs in CI, folded away.
50+
class Reporter
51+
def initialize(**_options); end
52+
53+
def report(coverage_result)
54+
FileUtils.mkdir_p(OUT_DIR)
55+
OpenapiFirst::Test::Coverage::HtmlReporter.new(output: HTML_REPORT).report(coverage_result)
56+
57+
markdown = render(coverage_result)
58+
File.write(MARKDOWN_REPORT, markdown)
59+
puts markdown
60+
print_details(coverage_result) if ENV['GITHUB_ACTIONS'] == 'true'
61+
end
62+
63+
private
64+
65+
# The per-route detail otherwise only exists in the HTML artifact. In
66+
# GitHub Actions it goes into a collapsed log group, so it can be read in
67+
# the job log without downloading anything and doesn't bury the digest.
68+
# Local runs skip it: the HTML report is right there in out/.
69+
def print_details(coverage_result)
70+
puts '::group::OpenAPI coverage details (every route not fully covered)'
71+
OpenapiFirst::Test::Coverage::TerminalReporter.new.report(coverage_result)
72+
puts '::endgroup::'
73+
end
74+
75+
def render(coverage_result)
76+
lines = ["## OpenAPI conformance\n"]
77+
coverage_result.plans.each { |plan| lines.concat(render_plan(plan)) }
78+
lines.join("\n")
79+
end
80+
81+
def render_plan(plan)
82+
exercised = plan.routes.count { |route| route.requests.any?(&:requested?) }
83+
violations, suppressed = partition_violations(plan)
84+
85+
lines = [
86+
"- Coverage: **#{plan.coverage.round(2)}%**",
87+
"- Described routes exercised: **#{exercised}/#{plan.routes.size}**",
88+
"- Responses skipped (5xx): #{plan.skipped_responses_count}",
89+
"- Conformance violations: **#{violations.size + suppressed.size}**"
90+
]
91+
lines << " (#{suppressed.size} of them `format: uuid` vs. synthetic test guids, not listed)" if suppressed.any?
92+
lines << ''
93+
return lines if violations.empty?
94+
95+
lines << '| Route | What | Error |'
96+
lines << '| --- | --- | --- |'
97+
violations.first(MAX_ROWS).each do |route, what, error|
98+
lines << "| #{route} | #{what} | #{error} |"
99+
end
100+
lines << "\n_#{violations.size - MAX_ROWS} more omitted; see the HTML report._" if violations.size > MAX_ROWS
101+
lines
102+
end
103+
104+
# A violation is something the specs actually exercised where every
105+
# attempt was rejected. Routes nothing touched are a coverage gap, not a
106+
# contract breach, and they are the overwhelming majority right now.
107+
def violations(plan)
108+
plan.routes.flat_map do |route|
109+
label = "`#{route.request_method.upcase} #{route.path}`"
110+
bad_requests = route.requests.select { |req| req.requested? && !req.any_valid_request? }
111+
bad_responses = route.responses.select { |res| res.responded? && !res.any_valid_response? }
112+
113+
bad_requests.map { |req| [label, 'request', req.last_error_message] } +
114+
bad_responses.map { |res| [label, "response #{res.status}", res.last_error_message] }
115+
end
116+
end
117+
118+
def partition_violations(plan)
119+
violations(plan).partition { |(_route, _what, error)| !OpenapiConformance.uuid_noise?(error) }
120+
end
121+
end
122+
123+
class << self
124+
def enabled?
125+
!ENV['OPENAPI_CONFORMANCE'].to_s.empty?
126+
end
127+
128+
def strict?
129+
ENV['OPENAPI_CONFORMANCE'] == 'strict'
130+
end
131+
132+
# Wraps the app the request specs drive. Returns it untouched when the
133+
# check is off, so a normal run pays nothing.
134+
def wrap(rack_app)
135+
return rack_app unless enabled?
136+
137+
OpenapiFirst::Test.app(rack_app, api: :cloud_controller)
138+
end
139+
140+
def setup!
141+
return unless enabled?
142+
143+
unless File.exist?(DESCRIPTION)
144+
raise "OPENAPI_CONFORMANCE is set but #{DESCRIPTION} is missing. " \
145+
'Build it first: cd docs/openapi && yarn install && yarn build'
146+
end
147+
148+
require 'openapi_first'
149+
150+
OpenapiFirst::Test.setup do |test|
151+
test.register(DESCRIPTION, as: :cloud_controller)
152+
153+
# The description covers part of the API, so the request specs hit
154+
# plenty of endpoints it says nothing about. That is the description
155+
# being incomplete, not the app being wrong.
156+
test.ignore_unknown_requests = true
157+
158+
# Default is to turn a non-conforming request or response into a spec
159+
# failure. Off unless asked, so one violation doesn't hide the rest of
160+
# the picture -- and so this can be added without breaking the suite.
161+
if strict?
162+
# Even here the uuid complaints stay out of the way -- they are test
163+
# fixtures, so failing on them would make strict mode unusable.
164+
test.ignore_request_error { |request| uuid_noise?(request.error&.message) }
165+
test.ignore_response_error { |response, _rack_request| uuid_noise?(response.error&.message) }
166+
else
167+
test.ignore_request_error { true }
168+
test.response_raise_error = false
169+
end
170+
171+
# 5xx responses are described on nearly every operation, but request
172+
# specs almost never produce them -- 245 of the 950 untested responses
173+
# in the first run. Skipping takes them out of the coverage number and
174+
# the report shows them as skipped rather than untested. The cost: a
175+
# skipped response isn't tracked at all, so a malformed 5xx body would
176+
# not show up as a violation.
177+
test.skip_response_coverage { |response| response.status.to_s.start_with?('5') }
178+
179+
# :warn reports without the exit 2 that an incomplete description
180+
# would otherwise trigger. Finding out what the number actually is is
181+
# the point of the spike.
182+
test.report_coverage = :warn
183+
test.coverage_reporter = Reporter
184+
end
185+
end
186+
end
187+
end
188+
189+
OpenapiConformance.setup!

0 commit comments

Comments
 (0)