Openapi request conformance

.github/workflows/openapi_conformance.yml

@@ -0,0 +1,102 @@
+# Check the traffic our request specs already generate against the OpenAPI
+# description in docs/openapi, and report what it finds.
+#
+# Informational: the description already disagrees with the API in a handful
+# of places, so the run publishes a digest rather than failing. Dispatch it
+# with `strict` to see what gating would do.
+name: OpenAPI Conformance
+
+concurrency:
+  group: '${{ github.workflow }}-${{ github.head_ref || github.run_id }}'
+  cancel-in-progress: true
+
+on:
+  workflow_dispatch:
+    inputs:
+      strict:
+        description: 'Fail the job on a non-conforming response'
+        type: boolean
+        default: false
+  pull_request:
+    branches: [ main ]
+    paths:
+      - 'app/**'
+      - 'lib/**'
+      - 'docs/openapi/**'
+      - 'spec/request/**'
+      - 'spec/support/openapi_conformance.rb'
+      - 'spec/support/request_spec_helper.rb'
+      - '.github/workflows/openapi_conformance.yml'
+
+permissions:
+  contents: read
+
+jobs:
+  Conformance:
+    name: Request specs vs. OpenAPI description
+    runs-on: ubuntu-latest
+    timeout-minutes: 45
+    services:
+      postgres:
+        image: postgres:17
+        env:
+          POSTGRES_PASSWORD: rootpassword
+        # Test-only durability tuning: skip the work that backs durability.
+        command: >-
+          postgres
+          -c fsync=off
+          -c synchronous_commit=off
+          -c full_page_writes=off
+        options: >-
+          --tmpfs /var/lib/postgresql/data:rw,size=2g
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          - 5432:5432
+    env:
+      DB: postgres
+      POSTGRES_CONNECTION_PREFIX: postgres://postgres:rootpassword@localhost:5432
+      OPENAPI_CONFORMANCE: ${{ (github.event_name == 'workflow_dispatch' && inputs.strict) && 'strict' || '1' }}
+    steps:
+      - uses: actions/checkout@v7
+      - uses: ./.github/workflows/composite/setup
+      - uses: actions/setup-node@v7
+        with:
+          node-version: 24
+
+      # openapi_first takes a single file, so the multi-file redocly source
+      # has to be bundled before the specs can read it.
+      - name: Build the API description
+        working-directory: docs/openapi
+        run: |
+          yarn install --frozen-lockfile
+          yarn build
+
+      - name: Prepare the database
+        run: bundle exec rake db:recreate
+
+      - name: Run request specs against the description
+        run: bundle exec rspec spec/request --format progress
+
+      - name: Publish the report
+        if: always()
+        run: |
+          if [ -f out/openapi_conformance.md ]; then
+            cat out/openapi_conformance.md >> "$GITHUB_STEP_SUMMARY"
+          else
+            echo '## OpenAPI conformance' >> "$GITHUB_STEP_SUMMARY"
+            echo 'No report produced -- the run failed before the specs finished.' >> "$GITHUB_STEP_SUMMARY"
+          fi
+
+      - name: Upload the report
+        if: always()
+        uses: actions/upload-artifact@v7
+        with:
+          name: openapi-conformance
+          path: |
+            out/openapi_conformance.md
+            out/openapi_coverage.html
+          if-no-files-found: warn
+          retention-days: 7

Gemfile

@@ -51,6 +51,7 @@ end
 group :test do
   gem 'factory_bot', '~> 6.5'
   gem 'mock_redis'
+  gem 'openapi_first'
   gem 'parallel_tests'
   gem 'rack-test'
   gem 'rspec', '~> 3.13.2'

Gemfile.lock

@@ -109,6 +109,7 @@ GEM
     grpc (1.84.0-x86_64-linux-gnu)
       google-protobuf (>= 3.25, < 5.0)
       googleapis-common-protos-types (~> 1.0)
+    hana (1.3.7)
     hashdiff (1.2.1)
     highline (3.0.1)
     httpclient (2.9.0)
@@ -127,6 +128,11 @@ GEM
     json-schema (6.2.0)
       addressable (~> 2.8)
       bigdecimal (>= 3.1, < 5)
+    json_schemer (2.5.0)
+      bigdecimal
+      hana (~> 1.3)
+      regexp_parser (~> 2.0)
+      simpleidn (~> 0.2)
     kramdown (2.5.2)
       rexml (>= 3.4.4)
     kramdown-parser-gfm (1.1.0)
@@ -174,6 +180,11 @@ GEM
       bigdecimal (>= 3.0)
       ostruct (>= 0.2)
     open3 (0.2.1)
+    openapi_first (4.0.1)
+      drb (~> 2.0)
+      hana (~> 1.3)
+      json_schemer (>= 2.1, < 3.0)
+      rack (>= 2.2, < 4.0)
     openssl (4.0.2)
     ostruct (0.6.3)
     parallel (2.3.0)
@@ -353,6 +364,7 @@ GEM
       sequel (>= 4.38.0)
     sexp_processor (4.17.6)
     simplecov (1.3.2)
+    simpleidn (0.3.0)
     sinatra (4.2.1)
       logger (>= 1.6.0)
       mustermann (~> 3.0)
@@ -462,6 +474,7 @@ DEPENDENCIES
   newrelic_rpm
   nokogiri (>= 1.10.5)
   oj
+  openapi_first
   openssl (>= 3.2)
   parallel_tests
   pg

docs/openapi/README.md

@@ -112,6 +112,45 @@ This is useful for:
 - Validating a mock server's implementation against the OpenAPI spec.
 - Quickly checking a live API for compliance without running the full `capi-bara-tests` suite.
 
+## Checking the description against the request specs
+
+The Cloud Controller's rspec request specs drive the real rack app, so every
+example is already a request/response pair that this description either covers
+or doesn't. `spec/support/openapi_conformance.rb` wraps that app with
+[openapi_first](https://rubygems.org/gems/openapi_first) and records the
+verdict -- no deployed CF, no proxy, no traffic capture.
+
+It is off unless `OPENAPI_CONFORMANCE` is set, and it reads the bundled
+description, so build that first:
+
+```bash
+cd docs/openapi && yarn install && yarn build
+cd ../.. && DB=postgres OPENAPI_CONFORMANCE=1 bundle exec rspec spec/request
+```
+
+That writes `out/openapi_conformance.md` (a digest of coverage and violations)
+and `out/openapi_coverage.html` (every route, with the ones nothing exercised).
+`OPENAPI_CONFORMANCE=strict` turns a non-conforming response into a spec
+failure instead of a line in the report.
+
+Violations reading `does not match format: uuid` are counted but not listed.
+Around 250 places in `spec/request` hand a model a literal guid such as
+`'app1_guid'` and then assert on it, so that check only has signal against
+recorded traffic, not against fixtures.
+
+5xx responses are left out of coverage. They are described on nearly every
+operation, but request specs almost never produce them, so they only drag the
+number down; the HTML report lists them as skipped. A skipped response isn't
+tracked, so a malformed 5xx body won't show up as a violation either.
+
+The `OpenAPI Conformance` GitHub Action runs this on pull requests and
+publishes the digest to the job summary. The step log carries the same digest
+plus, in a collapsed "OpenAPI coverage details" group, every route that isn't
+fully covered -- the HTML report is also uploaded as an artifact, but there is
+no need to download it. It is informational -- the
+description already disagrees with the API in a handful of places, so there is
+nothing honest to gate on yet.
+
 ## Contributing
 
 Contributions are welcome! Please feel free to submit a pull request or open an issue to discuss any changes.

spec/support/openapi_conformance.rb

@@ -0,0 +1,189 @@
+# frozen_string_literal: true
+
+# Spike: check the traffic our request specs already generate against the
+# OpenAPI description in docs/openapi.
+#
+# The request specs drive the real rack app through rack-test, so every
+# example is a request/response pair the description either covers or
+# doesn't. openapi_first wraps that app and records the verdict, which gives
+# us contract checking without a deployed CF, a proxy, or a traffic capture.
+#
+# Off unless OPENAPI_CONFORMANCE is set, so `rake spec` is untouched -- the
+# gem isn't even required.
+#
+#   OPENAPI_CONFORMANCE=1       record violations, fail nothing
+#   OPENAPI_CONFORMANCE=strict  raise on the first request/response that
+#                               doesn't conform
+#
+# It reads the bundled description: openapi_first takes a single file and
+# docs/openapi is a multi-file redocly source, so `yarn build` has to run first.
+module OpenapiConformance
+  DESCRIPTION = File.expand_path('../../docs/openapi/dist/latest/openapi.yaml', __dir__)
+  OUT_DIR = File.expand_path('../../out', __dir__)
+  HTML_REPORT = File.join(OUT_DIR, 'openapi_coverage.html')
+  MARKDOWN_REPORT = File.join(OUT_DIR, 'openapi_conformance.md')
+
+  # Rows past this get cut from the markdown -- the HTML report has them all,
+  # and a step summary that long is unreadable anyway.
+  MAX_ROWS = 50
+
+  # `format: uuid` failures are ours, not the API's: 248 places in
+  # spec/request hand a model a literal guid like 'app1_guid', and specs
+  # assert on those literals, so they can't just be swapped for real UUIDs.
+  # Real Cloud Controller guids are SecureRandom.uuid, so this check only has
+  # signal against recorded traffic, not against fixtures. Counted, not listed.
+  UUID_FORMAT_ERROR = /does not match format: uuid/
+
+  # Only suppress when every clause is a uuid complaint -- a compound error
+  # like "... format: uuid. value at `/x` is not an object" still matters.
+  def self.uuid_noise?(message)
+    return false if message.nil?
+
+    clauses = message.sub(/\A(Request body invalid|Response body is invalid|Path segment is invalid): /, '').split('. ')
+    clauses.all? { |clause| clause.match?(UUID_FORMAT_ERROR) }
+  end
+
+  # openapi_first takes one reporter. This one writes the HTML report to read
+  # after the fact, plus a short markdown digest for the CI log and the job
+  # summary. The gem's own terminal reporter prints every route not fully
+  # covered -- around a thousand lines -- so it only runs in CI, folded away.
+  class Reporter
+    def initialize(**_options); end
+
+    def report(coverage_result)
+      FileUtils.mkdir_p(OUT_DIR)
+      OpenapiFirst::Test::Coverage::HtmlReporter.new(output: HTML_REPORT).report(coverage_result)
+
+      markdown = render(coverage_result)
+      File.write(MARKDOWN_REPORT, markdown)
+      puts markdown
+      print_details(coverage_result) if ENV['GITHUB_ACTIONS'] == 'true'
+    end
+
+    private
+
+    # The per-route detail otherwise only exists in the HTML artifact. In
+    # GitHub Actions it goes into a collapsed log group, so it can be read in
+    # the job log without downloading anything and doesn't bury the digest.
+    # Local runs skip it: the HTML report is right there in out/.
+    def print_details(coverage_result)
+      puts '::group::OpenAPI coverage details (every route not fully covered)'
+      OpenapiFirst::Test::Coverage::TerminalReporter.new.report(coverage_result)
+      puts '::endgroup::'
+    end
+
+    def render(coverage_result)
+      lines = ["## OpenAPI conformance\n"]
+      coverage_result.plans.each { |plan| lines.concat(render_plan(plan)) }
+      lines.join("\n")
+    end
+
+    def render_plan(plan)
+      exercised = plan.routes.count { |route| route.requests.any?(&:requested?) }
+      violations, suppressed = partition_violations(plan)
+
+      lines = [
+        "- Coverage: **#{plan.coverage.round(2)}%**",
+        "- Described routes exercised: **#{exercised}/#{plan.routes.size}**",
+        "- Responses skipped (5xx): #{plan.skipped_responses_count}",
+        "- Conformance violations: **#{violations.size + suppressed.size}**"
+      ]
+      lines << "  (#{suppressed.size} of them `format: uuid` vs. synthetic test guids, not listed)" if suppressed.any?
+      lines << ''
+      return lines if violations.empty?
+
+      lines << '| Route | What | Error |'
+      lines << '| --- | --- | --- |'
+      violations.first(MAX_ROWS).each do |route, what, error|
+        lines << "| #{route} | #{what} | #{error} |"
+      end
+      lines << "\n_#{violations.size - MAX_ROWS} more omitted; see the HTML report._" if violations.size > MAX_ROWS
+      lines
+    end
+
+    # A violation is something the specs actually exercised where every
+    # attempt was rejected. Routes nothing touched are a coverage gap, not a
+    # contract breach, and they are the overwhelming majority right now.
+    def violations(plan)
+      plan.routes.flat_map do |route|
+        label = "`#{route.request_method.upcase} #{route.path}`"
+        bad_requests = route.requests.select { |req| req.requested? && !req.any_valid_request? }
+        bad_responses = route.responses.select { |res| res.responded? && !res.any_valid_response? }
+
+        bad_requests.map { |req| [label, 'request', req.last_error_message] } +
+          bad_responses.map { |res| [label, "response #{res.status}", res.last_error_message] }
+      end
+    end
+
+    def partition_violations(plan)
+      violations(plan).partition { |(_route, _what, error)| !OpenapiConformance.uuid_noise?(error) }
+    end
+  end
+
+  class << self
+    def enabled?
+      !ENV['OPENAPI_CONFORMANCE'].to_s.empty?
+    end
+
+    def strict?
+      ENV['OPENAPI_CONFORMANCE'] == 'strict'
+    end
+
+    # Wraps the app the request specs drive. Returns it untouched when the
+    # check is off, so a normal run pays nothing.
+    def wrap(rack_app)
+      return rack_app unless enabled?
+
+      OpenapiFirst::Test.app(rack_app, api: :cloud_controller)
+    end
+
+    def setup!
+      return unless enabled?
+
+      unless File.exist?(DESCRIPTION)
+        raise "OPENAPI_CONFORMANCE is set but #{DESCRIPTION} is missing. " \
+              'Build it first: cd docs/openapi && yarn install && yarn build'
+      end
+
+      require 'openapi_first'
+
+      OpenapiFirst::Test.setup do |test|
+        test.register(DESCRIPTION, as: :cloud_controller)
+
+        # The description covers part of the API, so the request specs hit
+        # plenty of endpoints it says nothing about. That is the description
+        # being incomplete, not the app being wrong.
+        test.ignore_unknown_requests = true
+
+        # Default is to turn a non-conforming request or response into a spec
+        # failure. Off unless asked, so one violation doesn't hide the rest of
+        # the picture -- and so this can be added without breaking the suite.
+        if strict?
+          # Even here the uuid complaints stay out of the way -- they are test
+          # fixtures, so failing on them would make strict mode unusable.
+          test.ignore_request_error { |request| uuid_noise?(request.error&.message) }
+          test.ignore_response_error { |response, _rack_request| uuid_noise?(response.error&.message) }
+        else
+          test.ignore_request_error { true }
+          test.response_raise_error = false
+        end
+
+        # 5xx responses are described on nearly every operation, but request
+        # specs almost never produce them -- 245 of the 950 untested responses
+        # in the first run. Skipping takes them out of the coverage number and
+        # the report shows them as skipped rather than untested. The cost: a
+        # skipped response isn't tracked at all, so a malformed 5xx body would
+        # not show up as a violation.
+        test.skip_response_coverage { |response| response.status.to_s.start_with?('5') }
+
+        # :warn reports without the exit 2 that an incomplete description
+        # would otherwise trigger. Finding out what the number actually is is
+        # the point of the spike.
+        test.report_coverage = :warn
+        test.coverage_reporter = Reporter
+      end
+    end
+  end
+end
+
+OpenapiConformance.setup!