Skip to content

docs(ci): Deploy Previews#1988

Merged
polarathene merged 12 commits intodocker-mailserver:masterfrom
polarathene:ci/deploy-preview
May 20, 2021
Merged

docs(ci): Deploy Previews#1988
polarathene merged 12 commits intodocker-mailserver:masterfrom
polarathene:ci/deploy-preview

Conversation

@polarathene
Copy link
Copy Markdown
Member

Description

Feature: When a PR contributes to our docs, a preview link will be added as a comment via Github Actions.


Each PR will have a unique prefix alias of pullrequest-N-- (where N is the PR number) to our docs preview site at dms-doc-previews.netlify.app (404 page is intentional, only the preview aliases for PRs will have actual content).

Example: https://pullrequest-42--dms-doc-previews.netlify.app


Details of interested are covered below. The workflows have some inlined documentation via comments, and each commit should be fairly clean with useful commit messages.

Sponsored plan + requirements to retain

Netlify has sponsored the docker-mailserver organization with an OSS team account.

To keep this sponsorship, we must comply with the plan policy; we qualified for sponsorship so currently meet the requirements. The Netlify branding mention is injected into build config for PRs only and can be found in the footer section on all pages.

We continue to deploy the production versions of documentation to Github Pages, but will utilize their service for deploying preview builds of PRs touching the documentation.

Billing / Plan limits

There are two billing concerns that still apply if exceeded
  • Bandwidth usage (400GB monthly)
    • This is regarding bandwidth transferring data to viewers. Our site is under 2MB in total, but this is considerably less over the wire as Netlify compresses responses/assets with gzip and brotli where appropriate. Caching is also leveraged on the client side.
    • As it's only being used for previewing PR contributions for docs, this is expected to be very low and a non-concern.
  • Build Minutes (1000 minutes per month)
    • This shouldn't be a concern. Our builds don't take long, but it could more easily be abused maliciously or through very high frequency of pushing commits.
    • Github Actions is handling builds instead, and then transferring the build to Netlify via their deploy API. This bandwidth is not part of the billing limit, and does not impact the build time.
    • To further guard against high frequency commits to a PR, Github Actions concurrency feature is in use to cancel any existing instance of the workflow for the same PR.

In the rare event one of those limits are exceeded to incur charges, see overdue accounts. If unpaid for 2 weeks, the account is restricted (no new deploys can be performed), within 4 weeks the account is suspended and the hosted deploys will no longer be available.

Github Actions - Workflow Security

This has been documented into the workflow to make the concern clear to any maintainers. Github authored a blog post recently about workflows with PRs.

Multi-staged workflow for security complicates the workflow a tad

pull_request event runs with secrets if the PR author is a trusted collaborator on the project, but for third-party community contributions untrusted code may be a concern for stealing secrets, thus Github restricts access.

Instead the pull_request workflow should handle running any untrusted code such as performing a build, and using a secure workflow (that's consistent with secrets access regardless of contributor) for handling anything requiring secrets that is not at the risk of running untrusted code from a contributor.

workflow_run is used as a 2nd stage of the PR workflow for this. It does complicate the process a bit more with additional steps, but is required for us to securely support deploy previews with all documentation PRs. This workflow will always run from the default branch only, and this affects some CI context such as the latest commit SHA.

Some actions have features that aren't compatible with this approach (eg nwtgck/actions-netlify). Either the maintainer will resolve them or I may contribute a fix. In the meantime that incompatibility has been minimized via additional actions in the workflow, while others like the deployment environment commit being logged incorrectly is a minor inconvenience.

Misc

These previews will remain reachable long after the PRs are merged
  • There is no automated cleanup, but it should not present any issue AFAIK.
  • As these are considered deploy previews by Netlify and not production marked builds, Netlify response headers advise search engines to not index these deployments.

Some small related improvements were made
  • Removed unnecessary build output and shared this logic in a shell script for running the build between preview and production deploy workflows. Version bumps to the docker image will be centralized there.
  • Cleaned up source logo files and optimized production versions.
  • Added a custom 404 page.
    • Not really needed, this was added to the Netlify project to initialize a site instead of connecting to the repo.
    • Figured it might as well be used as the docs 404 page at that point, it should not cause any maintenance burden AFAIK, but if source is required let me know.

Fixes #1871

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

- Removed trigger on workflow config modifications, provides no value for this workflow.
- Moved working-dir to the `run` command.
- Renamed job for new purpose.
- Modify `mkdocs.yml` so it builds correctly and works at netlify host.
Each PR that contributes to docs will generate a unique (to that PR) URL to preview the PR live for review.

We have chosen Netlify as the host and to build locally via Github Actions instead of offloading to Netlify (imposes limit in monthly minutes).

A separate action handles creating/updating the deployment comment. It provides a custom message and is portable should the deploy preview action change in future.
To support previews from non-collaborators PR contributions, we cannot rely on secrets access from workflows triggered by the `pull_request` event.

To do so securely, according to official advice from Github, we must run the third-party contribution in the restricted `pull_request` context, and then use a 2nd workflow to deploy the build (which requires secrets access).
Better naming convention for documentation workflows.

Split workflow only indicated status on PR of the 1st stage (building the preview to deploy), not the deployment progress/result. This needs to be managed more directly until the action better supports split-workflow scenario.
This would be more ideal on the 2nd phase workflow (`workflow_run`), however keeping it simple for now.

Limits the concurrency of the initial pull request workflow for documentation contributions that have PRs with multiple event triggers in a small time span (before the workflow triggered would complete). The main benefit is to avoid redundant deploys if the initial workflow has been triggered again to build the PR once more. It only will work against concurrent workflows for that PR in the 1st stage, if an existing `workflow_run` (2nd stage) is active for that PR it will not be cancelled.
Production and Deploy Preview builds are now maintained via the same shell command, so version updates of docker image is in one place.

Additionally deletes unnecessary build output which upstream provides no support to exclude.

---

update-versions-json script minor fix to meet style guidelines.

---

Minor comment correction in docs production deploy workflow.
This is used by the preview deploys on Netlify. Production deploys on Github Pages require a top-level 404 page manually deployed (since all are deployed to a version subpath).

This 404 page was custom built and optimized by me. This is the final minified output, separate source to build is available if needed. All content is inlined into the document except the large bg image. It compresses down to about 4KB with gzip/brotli when delivered over network.

---

Likewise the `favicon.ico` is a fallback for browsers that implicitly check the domain root for this file if the SVG isn't supported/preferred. Browsers check for this file without it being present in the HTML head meta elements.

On Github Pages the `favicon.ico` isn't likely to be picked up by even top-level as typical deployment has the project name as a subpath. The docs however reference a PNG favicon which should be widely supported.

The `favicon.ico` was generated by RealFaviconGenerator online tool with SVG source input. It contains 16px, 32px and 48px sizes. Quality is better than the `favicon.io` generator.

---

Minor quote style consistency fix in mkdocs.yml. Also added trailing slash to site_url, which was required in the past to avoid some 404 support errors but should be resolved now with latest mkdocs.
SVG source cleaned up and optimized with SVGO 2.3.

Minified versions (`.min.svg` extension) remove unnecessary data and white-space to reduce size further for production use. This extension better differentiates by filename that it's different from the `src` version.

PNGs updated to match minor differences from optimizations (reduced float precision).
`mkdocs build` will exclude directories that are prefixed with a `.`. There's no need for these files to be deployed each time.
@polarathene
Copy link
Copy Markdown
Member Author

Two secrets need to be added to this project for deployment to be successful, I have invited two maintainers and notified them of this requirement. Once that is done this PR is ready for merging.

@georglauterbach
Copy link
Copy Markdown
Member

LGTM 👍

@wernerfred could you have a look at the secrets?

@georglauterbach georglauterbach added area/ci area/documentation kind/new feature A new feature is requested in this issue or implemeted with this PR pr/needs review priority/medium labels May 20, 2021
@wernerfred
Copy link
Copy Markdown
Member

Insane, you gave me much insight and inspirations on how to tackle problems i had with triggers based on different worklfows. Thanks also for the linked blog post!
Is anything missing right now?

wernerfred
wernerfred previously approved these changes May 20, 2021
Copy link
Copy Markdown
Member

@wernerfred wernerfred left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution!
I could follow the changes/logic in general and it LvGTM 👍🏻

Could be that there are some minors that occur only during workflow run but should be easy fixable then.

Comment thread .github/workflows/scripts/docs/update-versions-json.sh Outdated
@polarathene
Copy link
Copy Markdown
Member Author

Insane, you gave me much insight and inspirations on how to tackle problems i had with triggers based on different worklfows.

You're welcome! It took me a while to figure it all out for non-personal usage 😄

Is anything missing right now?

Not that I know of. It went through multiple revisions over the past two months when I had time for it. I have not created a 2nd account to test a PR from a non-collaborator, but it should work or be addressable with a follow up PR if needed.

Deployment should otherwise work well, which has been tested and verified on my test repo.

Comment thread .github/workflows/scripts/docs/update-versions-json.sh Outdated
@wernerfred
Copy link
Copy Markdown
Member

Then take the honor (after fixing casperkleins suggestion) of merging this ;)

@polarathene polarathene dismissed stale reviews from georglauterbach and wernerfred via 4736ec3 May 20, 2021 09:53
@wernerfred wernerfred added this to the v10.0.0 milestone May 20, 2021
@polarathene polarathene merged commit cf22475 into docker-mailserver:master May 20, 2021
@wernerfred
Copy link
Copy Markdown
Member

wernerfred commented May 20, 2021

Shouldn't have this PR already build a preview? The Build Preview workflow has passed and you have changes in docs/**

I thought that changes in PRs are always directly valid/active during this PR. But could also be that only the file that was previously there (you only renamed it) was triggered and not the new one. We'll see with the LDAP docs PR ;)

@polarathene
Copy link
Copy Markdown
Member Author

But could also be that only the file that was previously there (you only renamed it) was triggered and not the new one.

If you rename a workflow, at least with the name field IIRC it will update the name to that. It did seem to run the new PR workflow steps however.

The 2nd stage which takes the build and deploys it via workflow_run is run from the default branch, presumably that was not available at this point.

We'll see with the LDAP docs PR ;)

😉 🙏

@wernerfred
Copy link
Copy Markdown
Member

😉 🙏

Insanity! 🎉 #1921 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci area/documentation kind/new feature A new feature is requested in this issue or implemeted with this PR priority/medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FR] Documentation PR preview deploys

4 participants