Skip to content

Commit

Permalink
docs: Drop sphinxcontrib-openapi fork, switch back to upstream
Browse files Browse the repository at this point in the history
Once upon a time, Cilium docs used the openapi Sphinx add-on to generate
its API reference based on the code. And things were good.

One day, Dependabot raised a security alert, stating that Mistune v2.0.2
was vulnerable to catastrophic backtracking [0] - this is a regex
parsing thing. Mistune was a dependency to m2r, an add-on to parse
Markdown in Sphinx, which in turn was a dependency to openapi.

The easy path would have been to update m2r to use the latest, fixed
Mistune version; but m2r was incompatible with Mistune >= 2.0.0, and
also it was no longer in development.

There was a fork, m2r2, which had little activity, and would avoid the
security issue by very simply pinning the Mistune version to 0.8.4
(which would either fail to build Cilium's reference correctly, or bring
some incompatibilities with other dependencies, at this point the
narrator does not remember for sure).

There was a fork of the fork, sphinx-mdinclude. We could use that
project to update openapi, except that it was not compatible with recent
versions of docutils, and that this would cause openapi's test suite to
fail to pass.

... So we ended up forking the openapi repository to update the
dependency to sphinx-mdinclude locally, and this is what we've been
using since last summer. And things were good again.

But things are even better when they go upstream [citation needed]. We
also filed the issue for docutils compatibility in sphinx-mdinclude [1].
It was fixed (thanks!). We submitted a PR to have openapi switch to
sphinx-mdinclude [2]. It was adjusted (thanks!), merged, and a new tag
was created.

Now at last, we can switch back to the upstream version of openapi!
[And the build system lived happily ever after.]

[0]: GHSA-fw3v-x4f2-v673
[1]: omnilib/sphinx-mdinclude#8
[2]: sphinx-contrib/openapi#127

I did _not_ run `make -C Documentation update-requirements`, because the
resulting changes seemed to break the Netlify preview [3]. I stuck to
openapi and bumped sphinx-mdinclude to >= 0.5.2, as required by openapi.

[3] https://app.netlify.com/sites/docs-cilium-io/deploys/63c55fcc5531c6000838b87c

Signed-off-by: Quentin Monnet <[email protected]>
  • Loading branch information
qmonnet authored and joestringer committed Feb 4, 2023
1 parent ecdae30 commit e4889d7
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 7 deletions.
4 changes: 1 addition & 3 deletions Documentation/requirements-min/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ semver==2.13.0

# Sphinx extensions
myst-parser==0.18.0
# Fork openapi until it uses something newer than unmaintained m2r
# (See git logs for details)
git+https://github.com/cilium/openapi.git@mdinclude#egg=sphinxcontrib-openapi
sphinx-tabs==3.4.0
sphinx-version-warning==1.1.2
sphinxcontrib-openapi==0.8.0
sphinxcontrib-spelling==7.7.0
sphinxcontrib-websupport==1.2.4

Expand Down
6 changes: 2 additions & 4 deletions Documentation/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@ sphinx-rtd-theme-cilium @ git+https://github.com/cilium/sphinx_rtd_theme.git@3bd
semver==2.13.0
# Sphinx extensions
myst-parser==0.18.0
# Fork openapi until it uses something newer than unmaintained m2r
# (See git logs for details)
sphinxcontrib-openapi @ git+https://github.com/cilium/openapi.git@a1d4fca2e7c3ae3cca69593baade1ebc297a12ff
sphinx-tabs==3.4.0
sphinx-version-warning==1.1.2
sphinxcontrib-openapi==0.8.0
sphinxcontrib-spelling==7.6.0
sphinxcontrib-websupport==1.2.4
# Linters
Expand Down Expand Up @@ -49,7 +47,7 @@ PyYAML==6.0
requests==2.28.1
six==1.16.0
snowballstemmer==2.2.0
sphinx_mdinclude==0.5.1
sphinx_mdinclude==0.5.3
sphinxcontrib-applehelp==1.0.2
sphinxcontrib-devhelp==1.0.2
sphinxcontrib-googleanalytics==0.3
Expand Down

0 comments on commit e4889d7

Please sign in to comment.