Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add k8s container #295

Merged
merged 6 commits into from
Jan 11, 2022
Merged

Add k8s container #295

merged 6 commits into from
Jan 11, 2022

Conversation

maeve-fpf
Copy link
Contributor

@maeve-fpf maeve-fpf commented Dec 22, 2021

Status

Ready for review

Description of Changes

This adds a container which runs Sphinx during the build, and serves the output with nginx. We will deploy this container in GKE to self-host the documentation and move off of RTD.

HTML output is relatively straightforward. There is an RTD-like theme in Sphinx. In order to get the functionality RTD gives us of a version switching widget, I added a small bit of template based on what RTD's JavaScript adds to the document. It's not as full-featured as RTD but it does show you if you're on latest or stable and lets you download the PDF.

PDF output required some wrangling: non-latin characters must be defined as LaTeX, and SVG inclusion does not work (it did not work on RTD either, however). In going through characters used in the source, I only fixed incorrectly encoded quotation marks, and did not try to make all the triangles used to denote submenu items consistent (there are U+25B6 ▶, U+25B8 ▸, and U+003E >).

Testing

The output is deployed at https://staging-docs.securedrop.org/ (publicly accessible). To run locally, docker build -f deploy/Dockerfile .; and run with docker run <imageid> -p 127.0.0.1:5080:5080, then navigate to http://localhost:5080/. Because the container will be deployed behind another Nginx setup, it is set to omit port numbers from redirects, so you may have to add them back in.

The main redirect is from / to /en/stable/, to match the current behavior of RTD (as there is no localization the output is currently just copied to this directory to get the correct path). There are also redirects from the current RTD PDF URLs, /_/downloads/en/stable/pdf/ (and latest), to the actual PDF files.

To verify that the content is different between stable and latest, you can check the "Set Up the Network Firewall" page - in latest, it has been split out into pfSense and OPNSense.

Release

Currently, k8s CI is set up to deploy a test branch to staging. Once this is merged, I can adjust that to run on pushes to main here.

Checklist (Optional)

  • Doc linting (make docs-lint) passed locally - the container build does not run this, but we could add failing on that if desired.
  • Doc link linting (make docs-linkcheck) passed - No, I'm currently getting a 500 from Gitter. Maybe an actual outage on their end?
  • You have previewed (make docs) docs at http://localhost:8000 - still works, although the version switcher/PDF link will be useless. There is no containerized live preview.

deploy/build Show resolved Hide resolved
@eloquence
Copy link
Member

eloquence commented Jan 6, 2022

Thanks much for all the hard work on this @maeve-fpf. I was able to confirm that:

  • the documentation is built correctly in two versions, stable and latest
  • these versions correspond to the HEAD of main and to the most recently tagged version
    • respecting post-release modifiers: 2.2.0-1 > 2.2.0
    • honoring SemVer: 2.2.21 > 2.2.3
  • the URL paths correspond to the current documentation, i.e. no existing external links should break
  • the PDF is generated (with errors as noted in the code comment) and corresponds mostly to the current PDF (see below)
  • the version selector widget for stable and latest is present iff the documentation is built from a commit that includes the changes in this branch (I tested this by locally tagging this branch with a more recent version)
  • the version selector widget works as expected

Differences of note:

  1. The documentation no longer links directly to the GitHub URL corresponding to a specific section of the docs; the link points instead to the raw page source. IIRC we had previously discussed linking to a GitHub URL (could be the HEAD of main rather than the specific version the user is looking at). If that's easy enough to do, it seems somewhat more useful than the RST page source.
  2. The PDF no longer includes a table of contents. That seems like a valuable addition given the length of the documentation.

IMO neither of these are blocking issues for this PR though, and we can investigate this further in follow-up.

I'd appreciate a second pair of eyes from a maintainer before we merge (no need to repeat every single of the above checks though).

One small thing: I'd suggest squashing the commits down a bit before merging.

@maeve-fpf
Copy link
Contributor Author

I've added a commit here, branched off of 2.1.0-2, as a candidate to be tagged 2.1.0-3. It contains a subset of these changes, squashed into one commit, so that a build with that tag checked out will generate a version switcher and PDF: 2.1.0-2...stable-plus-k8s-container

Copy link
Member

@cfm cfm left a comment

Choose a reason for hiding this comment

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

Following @eloquence's review, I've reviewed the diff and taken staging-docs.securedrop.org for a spin, testing especially:

  • the documentation is built correctly in two versions, stable and latest
  • these versions correspond to the most recently tagged version and to the HEAD of main (respectively)
  • the URL paths correspond to the current documentation, i.e. no existing external links should break
  • the PDF is generated (with errors as noted in the code comment) and corresponds mostly to the current PDF (see below)
  • the version selector widget works as expected

Inline I've requested (slash offered to make :-) one change in the underlying ReST source in this repository specifically (not a functional change to this implementation).

docs/development/updating_ossec.rst Outdated Show resolved Hide resolved
@maeve-fpf
Copy link
Contributor Author

Table of contents should now be working.

cfm
cfm previously approved these changes Jan 11, 2022
@cfm
Copy link
Member

cfm commented Jan 11, 2022

Approved!—but I'll defer to you, @maeve-fpf and @eloquence, for merge, in light of any operational considerations I'm not in the loop on.

@maeve-fpf
Copy link
Contributor Author

maeve-fpf commented Jan 11, 2022

Thanks @cfm, I've picked up that commit!

Ready to merge now - noting here that for staging CI purposes, we are still building the "stable" docs off off this backported branch: 2.1.0-2...stable-plus-k8s-container - tagging that commit as something like 2.1.0-3 will remove the need for that. Let's observe if anything now in main causes problems for RTD, since we will still leave prod pointed at RTD for a bit.

Copy link
Member

@eloquence eloquence left a comment

Choose a reason for hiding this comment

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

Approving and merging based on @cfm's and my prior review.

@eloquence eloquence merged commit 97f7c98 into main Jan 11, 2022
@legoktm legoktm deleted the k8s-container branch May 28, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants