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

Snapshot release v3 #386

Merged
merged 43 commits into from
Jan 31, 2020
Merged

Snapshot release v3 #386

merged 43 commits into from
Jan 31, 2020

Conversation

twifkak
Copy link
Member

@twifkak twifkak commented Jan 29, 2020

No description provided.

twifkak and others added 30 commits July 1, 2019 15:17
- Document percent-escaping issue with nginx.
- Document amp-install-serviceworker limitation.
- Clarify instructions for following amppkg releases.
- Document error behavior if the SXG is not AMP-cache-valid; this is
  here as an example for others implementing AMP SXG caches, and also as
  an explanation of the likely behavior if publishers don't keep their
  packager up-to-date.
During renewing of SXG cert in cluster servers, the new cert cannot
be obtained in requesting cert-url from the cluster. `-cert_url_base`
option enables users to overriding scheme, hostname and parent path of
cert-url so that they can obtain the newer cert by specifying a server.

In the server, one can get a new cert and confirm its cert renewal by
using localhost such as

`amppkg_dl_sxg -cert_url_base http://localhost:8080/amppkg/cert/ http://localhost:8080/priv/doc?...`
…nds with a comma character. It turns out that the srcset field format does not correctly parse URLs ending with a comma. This is true in the AMP Validator (so these transformations break validity of AMP pages) as well as in browsers (so these transformations break browser images).

PiperOrigin-RevId: 259062826
All SXG certificates that have a Validity Period longer than 90 days
are rejected after 2019-08-01. b1 test data is no longer needed.
* Add link to amp.dev docs for Signed Exchanges in AMP Packager Readme.

Fixes #305.

* Fix link.

* swap link order
…ansformer (run before amp packager, such as in the optimizer) determined that boilerplate was unnecessary.

PiperOrigin-RevId: 262203609
PiperOrigin-RevId: 262448044
Changes to accomodate ACME configs in amppackager toml.
Added utility func to compute cert expiry.
* Added certloaded and refactored signer to be able to pull certs
from the cache.

* Added unit test for certloader

* Updated gateway_server
* Transformer to strip HTML style comments from script tags.

Examples:
<script><!-- Hello --></script> == <script></script>

<script><!--[if IE]>var ie = true;<![endif]--></script> == <script></script>

PiperOrigin-RevId: 263786190

* Fix test failure for insufficient args to Errorf.

PiperOrigin-RevId: 267655188
Fix formatting error, and change future tense to present.
Generate go mod files with replace directive to pin to desired versions.
…rary. (#345)

CertFetcher abstraction on top of LEGO/ACME library. The intent is to modify the current flow to inject an instance of CertFetcher into CertCache to take care of the scheduled cert renewals.
Update go modules deps, go mod tidy and go mod vendor.
This uses the new metadata field returned from the transformer library,
which is based on the presence of any inline amp-scripts.
* Added /healthz support. Fixed IsHealthy logic. Added tests for both. Added healthz http handler.
Generated protobuf messages contain internal data structures
that general purpose comparison functions (e.g., reflect.DeepEqual,
pretty.Compare, etc) do not properly compare. It is already the case
today that these functions may report a difference when two messages
are actually semantically equivalent.

Fix all usages by either calling proto.Equal directly if
the top-level types are themselves proto.Message, or by calling
cmp.Equal with the cmp.Comparer(proto.Equal) option specified.
This option teaches cmp to use proto.Equal anytime it encounters
proto.Message types.

PiperOrigin-RevId: 278904435
…sive.

The AMP Runtime interprets an img with a height and width plus an empty heights
or sizes as layout=fixed. Currently the transformers and validator treat it as
layout=responsive.
PiperOrigin-RevId: 279189628
PiperOrigin-RevId: 280045190
* Update ACME config to include email adddress and acme challenge port. Update certcache to use certfetcher if cert autorenew is turned on. Update certloader.PopulateCertCache to instantiate certfetcher and pass it on to certcache instance.

* Add config file error checking for PopulateCertCache.

* Add more logic to handle initial conditions with invalid cert and to address comments from gregable@.

* Code refactor/cleanup involving certs.

* Add DNS and TLS challenges, added them to load from config, cleaned up autorenewcert config parsing.

* go mod tidy, go mod vendor updates

* Fixed CSR Loading, added it to config

* Fixed bugs with checking for cert expiry.

* Added support for saving the fetched certs to disk and for certs to be loaded by non-auto-renewing amppackager instances.

* Fixed gateway server call to certcache. Removed go module files inside gateway server dir. Re-ran go mod tidy.

* Fixed certcache_test.go after merge.

* Fixed bugs in certcache, also fixed unit test.

* Added locking for reading/writing certs.

* Ran go fmt on files that have incorrect formatting

* Fix twifkak first-pass comments except the logic change comment which I will address in a later commit.

* Fix twifkak comments for ocsp refresh logic and ocsp cache purge.

* Fix additional twifkak comments.

* Fix 2nd round of twifkak comments.

* Fix gregable@ comments.
twifkak and others added 10 commits December 9, 2019 14:05
errors.Wrap() includes a colon, so the period is redundant.
…ansformer

was #148, and text/plain scripts should not be relevant to that aim. Note that
the text/plain scripts used by the amp-script component are further guarded by
computeMaxAgeSeconds in transformer.go.

PiperOrigin-RevId: 286250297
If the cert doesn't have an AIA extension with an OCSP responder URL,
then the cache will just include a fake (invalid) OCSP response in its
cert-chain+cbor. This is sufficient for displaying the SXG in Chrome
with --ignore-certificate-errors-spki-list.
…direct dependency on a now-unknown code commit (#380)
To be cautious re: privacy/security, specify only host. Note that this
changes behavior for anybody who was previously including "Forwarded" or
"X-Forwarded-Host" in their ForwardedRequestHeaders in the config.
…L, and

thus breaks idempotence.

PiperOrigin-RevId: 288735975
PiperOrigin-RevId: 291960396
@twifkak twifkak requested review from Gregable and banaag January 29, 2020 02:50
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@twifkak
Copy link
Member Author

twifkak commented Jan 29, 2020

Authors already consented in the original PRs to master.

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@twifkak
Copy link
Member Author

twifkak commented Jan 31, 2020

Authors already consented in the original PRs to master.

@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants