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

Automate best practices for production #114

Open
twifkak opened this issue Sep 19, 2018 · 4 comments
Open

Automate best practices for production #114

twifkak opened this issue Sep 19, 2018 · 4 comments
Assignees

Comments

@twifkak
Copy link
Member

twifkak commented Sep 19, 2018

This could be a collection of scripts, or packages of various formats (Docker, Flatpak, .deb, etc.). Broad coverage of most of the production environments probably necessitates multiple formats, though we should prefer a solution that covers as many as possible in as few variants as possible. This will reduce the cost of maintenance and the chance of error in one or more of them. Googlers, see go/amp-packager-deployment-requirements for more info.

@twifkak twifkak added the good first issue Good for newcomers label Sep 19, 2018
@twifkak twifkak added this to the v4+ milestone Sep 19, 2018
@twifkak
Copy link
Member Author

twifkak commented May 8, 2019

I've come across a couple examples on the webs:

Haven't looked at them in detail yet, so I can't vouch for them, but we should link to them on an Implementations wiki page (and link to that from the readme).

@twifkak
Copy link
Member Author

twifkak commented Jun 6, 2019

One thing that came up in practice was difficulty meeting the "inner = outer" requirement for URLs with pct-encoded chars (such as https://example.com/index%2ehtml or https://example.com/foo%2fbar):

The signed fallback URL must equal the URL at which the SXG was delivered.

This was fixed in amppkg in #313, but the bug may still occur in the various reverse proxies used in front. For instance, I believe https://github.com/Warashi/try-amppackager/blob/895df747aa0948641e7ce979fed88dbf1906cbd6/reverse/proxy.conf#L39 will result in inner URLs that are doubly encoded. OTOH, using rewrite ... ?sign=... results in the opposite problem -- URLs that aren't doubly encoded when we want them to be.

The only mechanism I found that worked with nginx is:

proxy_pass http://amppkg/priv/doc/$scheme://$server_name$request_uri;

proxy_pass doesn't seem to have the same double-encoding bug.

@twifkak
Copy link
Member Author

twifkak commented Jun 7, 2019

Filed Warashi/try-amppackager#3 in the meantime.

@twifkak twifkak modified the milestones: v6+, v4: Ease of deployment Jun 29, 2019
@twifkak twifkak removed the good first issue Good for newcomers label Sep 24, 2019
@twifkak
Copy link
Member Author

twifkak commented Nov 8, 2019

This should also include an auto-updating facility (optional, but enabled by default), to allow the deployed amppackager to remain current with AMP Caches' AMP-Cache-Transform request header.

Additionally, when grabbing the latest release from the GitHub releases branch, it would be great if the fetcher could verify the snapshot by checking against either a) a checksum published on cdn.ampproject.org, or b) a signature included in the codebase, verified against a public key published on cdn.ampproject.org (see minisign or signify).

@banaag Feel free to split both of these things off as separate bugs if you prefer.

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

No branches or pull requests

3 participants