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

Consolidate certificate supplier module #410

Merged
merged 3 commits into from Dec 23, 2019
Merged

Conversation

@jianglai
Copy link
Member

jianglai commented Dec 11, 2019

Both the proxy and the proxy needs certificate suppliers. The PR
consolidates the module that providings those bindings to a shared
module and switched the proxy to use that module. The prober currently
uses P12 file to store its certificates. I am debating keeping that
supplier ro converting them to PEM files for simplicity.


This change is Reviewable

@googlebot googlebot added the cla: yes label Dec 11, 2019
Both the proxy and the proxy needs certificate suppliers. The PR
consolidates the module that providings those bindings to a shared
module and switched the proxy to use that module. The prober currently
uses P12 file to store its certificates. I am debating keeping that
supplier ro converting them to PEM files for simplicity.
@jianglai jianglai force-pushed the jianglai:cert-module branch from 0f0e876 to 5bfd205 Dec 11, 2019
@jianglai jianglai requested a review from CydeWeys Dec 11, 2019
Copy link
Member

CydeWeys left a comment

Reviewable status: 0 of 60 files reviewed, 1 unresolved discussion (waiting on @CydeWeys and @jianglai)


networking/src/main/java/google/registry/networking/module/CertificateSupplierModule.java, line 102 at r1 (raw file):

  @Qualifier
  @interface Pem {}

This is a little bit too short. How about Pemsomething? What makes the most sense? PemCertificate? PemKey? Something else?

Ditto for P12 and SelfSigned.

@jianglai jianglai requested a review from CydeWeys Dec 18, 2019
Copy link
Member Author

jianglai left a comment

Reviewable status: 0 of 60 files reviewed, 1 unresolved discussion (waiting on @CydeWeys)


networking/src/main/java/google/registry/networking/module/CertificateSupplierModule.java, line 102 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

This is a little bit too short. How about Pemsomething? What makes the most sense? PemCertificate? PemKey? Something else?

Ditto for P12 and SelfSigned.

Updated the names to include "file" to signify that they are read from files. I think "SelfSigned" is pretty self-explanatory though.

Copy link
Member

CydeWeys left a comment

:lgtm:

Reviewable status: 0 of 60 files reviewed, all discussions resolved (waiting on @CydeWeys)

@jianglai jianglai merged commit dd0e3b7 into google:master Dec 23, 2019
6 of 7 checks passed
6 of 7 checks passed
code-review/reviewable 60 files left (CydeWeys)
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
LGTM analysis: Java No new or fixed alerts
Details
cla/google All necessary CLAs are signed
kokoro-foss Kokoro build finished
Details
kokoro-internal Kokoro build finished
Details
@jianglai jianglai deleted the jianglai:cert-module branch Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.