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.

Sign up for GitHub

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

Jump to bottom

New HTTP framework to incorporate automatic insecure connection handling #2100

Merged
merged 35 commits into from
Nov 1, 2019
Merged

New HTTP framework to incorporate automatic insecure connection handling #2100

merged 35 commits into from
Nov 1, 2019

Conversation

chanseokoh
Copy link
Member

@chanseokoh chanseokoh commented Oct 24, 2019 •
edited
Loading

Introducing thread-safe TlsFailoverHttpClient (still named Connection for now) designed to be instantiated as a singleton stateful instance throughout the Jib build life cycle.

The TLS failover feature is decoupled from RegistryEndpointCaller and integrated into TlsFailoverHttpClient. The new client provides a complete abstraction for taking care of all possible TLS failover scenarios and controlling whether to send credentials over plain-HTTP.

RegistryAuthenticator will make use of the new client, so this will fix #2074.

In the future, this setup will allow

@chanseokoh
wip
9f1618c
@chanseokoh
wip
dd1d872
@chanseokoh
Merge branch 'master' into i2027-allow-insecure-auth-server
9d60cd1
@chanseokoh
wip
20bd9a0
@chanseokoh
Rename Connection to TlsFailoverHttpClient
578846f
@chanseokoh
Fix sendCredentialsOverHttp
dcdda78
@chanseokoh
Fix Javadoc
a002d6e
@chanseokoh
Rename class
4246b0d
@chanseokoh
Enhance TestWebServer and some cleanups
31505a9
@chanseokoh
Refactor
9f2b5c3
@chanseokoh
Merge branch 'test-web-server' into i2027-allow-insecure-auth-server
e686797
@chanseokoh
Move code
38795ed
@chanseokoh
Merge branch 'master' into i2027-allow-insecure-auth-server
728f594
@chanseokoh
rename
e7cd11c
@chanseokoh
Add comment
d30b0dd
@chanseokoh
Various minor fixes
c603e3c
@chanseokoh
Rename variable
cb56782
@chanseokoh
Renaming
41aefce
@chanseokoh
Don't log error for SSLException
05788bf
@chanseokoh
Merge branch 'minor-refactoring' into i2027-allow-insecure-auth-server
5ffac00
@chanseokoh
rename
02c150a
@chanseokoh
Log broken pipe for SSLException too
aa6f980
@chanseokoh
Merge branch 'minor-refactoring' into i2027-TlsFailoverHttpClient
f7a3b21
@chanseokoh
Merge remote-tracking branch 'origin/master' into i2027-TlsFailoverHt…
593bf8c 
…tpClient
@chanseokoh chanseokoh mentioned this pull request Oct 25, 2019
Closed
TadCordle
TadCordle reviewed Oct 28, 2019
View reviewed changes
Copy link
Contributor

@TadCordle TadCordle left a comment

Choose a reason for hiding this comment

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

This mostly LGTM, I think others should take a look though since it seems like an important change.

jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java Show resolved Hide resolved
jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionTest.java Outdated Show resolved Hide resolved
...src/test/java/com/google/cloud/tools/jib/http/TlsFailoverHttpClientProxyCredentialsTest.java Outdated Show resolved Hide resolved
jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerTlsFailoverHttpClientTest.java Outdated Show resolved Hide resolved
briandealwis
briandealwis reviewed Oct 29, 2019
View reviewed changes
Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

Still looking, but some early comments

jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Show resolved Hide resolved
briandealwis
briandealwis reviewed Oct 31, 2019
View reviewed changes
Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

Sorry haven't finished. But some more things.

jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Show resolved Hide resolved
briandealwis
briandealwis reviewed Nov 1, 2019
View reviewed changes
Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

Some more.

jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java Show resolved Hide resolved
briandealwis
briandealwis approved these changes Nov 1, 2019
View reviewed changes
jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java Show resolved Hide resolved
jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerConnectionTest.java Outdated Show resolved Hide resolved
jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java Outdated Show resolved Hide resolved
jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java Outdated Show resolved Hide resolved
@chanseokoh chanseokoh merged commit 3030cde into master Nov 1, 2019
@chanseokoh chanseokoh deleted the i2027-TlsFailoverHttpClient branch November 1, 2019 19:56
This was referenced Nov 1, 2019
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@loosebazooka loosebazooka loosebazooka left review comments

@TadCordle TadCordle TadCordle left review comments

@briandealwis briandealwis briandealwis approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

-Djib.allowInsecureRegistries=true does not seems to work
5 participants
@chanseokoh @briandealwis @TadCordle @loosebazooka @googlebot