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

Implement proxy connection support #1677

Open
steve-todorov opened this issue Feb 17, 2020 · 10 comments · May be fixed by #1706
Open

Implement proxy connection support #1677

steve-todorov opened this issue Feb 17, 2020 · 10 comments · May be fixed by #1706
Assignees

Comments

@steve-todorov
Copy link
Member

Task Description

We need to complete the proxy connection implementation. Currently we have:

  1. Configuration endpoint which is capable of managing proxy connection configuration into strongbox.yaml
  2. Configuration endpoint which is capable of managing storage repositories and also save their proxy connection configuration.
  3. There is also UI for this.

However, the actual apache httpclient connection pool which is already in place is not honoring the saved configuration. The logic should be as follows:

  1. Use the Global Proxy Configuration from strongbox.yaml when it's in place
  2. If there is a Proxy Configuration for a Repository - it should take priority over the Global proxy
  3. Hearbeat monitoring must also use the appropriate proxy configuration when checking if the repository is live (i.e. if repo has no proxy configuration, but global proxy configuration exists - it should use it; if there is no proxy configuration at all - use direct connection)

Task Relationships

This task:

Help

@anki2189
Copy link
Member

@carlspring @sbespalov : Why there are two implementation for http Client in ProxyRepositoryConnectionPoolConfigurationServiceImpl

  • ProxyRepositoryConnectionPoolConfigurationServiceImpl#getRestClient()

  • ProxyRepositoryConnectionPoolConfigurationServiceImpl#getHttpClient()

@sbespalov
Copy link
Member

One is probably for REST and another for other HTTP interactions.

@carlspring
Copy link
Member

carlspring commented Feb 19, 2020

I believe we should be using Apache's httpclient for everything. We shouldn't use the classes from the JDK for this.

I believe that ProxyRepositoryConnectionPoolConfigurationServiceImpl#getRestClient() should be renamed to ProxyRepositoryConnectionPoolConfigurationServiceImpl#getHttpClient() and the ProxyRepositoryConnectionPoolConfigurationServiceImpl#getRestClient() should be removed.

@sbespalov
Copy link
Member

@carlspring they both using same poolingHttpClientConnectionManager. So we can say that Rest Client is just provide JAXRS interfaces above httpclient.

@carlspring
Copy link
Member

Okay, thanks for the clarification!

@anki2189
Copy link
Member

anki2189 commented Feb 20, 2020

Below are the usages of ProxyRepositoryConnectionPoolConfigurationServiceImpl#getHttpClient()

  • HttpGetRemoteRepositoryCheckStrategy#isAlive()

  • RepositoryProxyIndexCreator#fetchIndex()

ProxyRepositoryConnectionPoolConfigurationServiceImpl#getRestClient()

  • NugetRepositoryFeatures#downloadRemoteFeed()

  • NugetRepositoryFeatures.RepositorySearchEventListener#handle()

  • NpmRepositoryFeatures#fetchRemoteChangesFeed()

  • NpmRepositoryFeatures#fetchRemotePackageFeed()

  • NpmRepositoryFeatures#fetchRemoteSearchResult()

  • RestArtifactResolverFactory#newInstance()

  • FetchChangesFeedCronJobTestIT#prepareArtifactResolverContext()

  • ArtifactResolverIT#setUp()

  • ProxyRepositoryConnectionPoolConfigurationServiceImplIT#connectionsLeakedTest()

  • ProxyRepositoryConnectionPoolConfigurationServiceImplIT#connectionsReleasedTest()

@steve-todorov
Copy link
Member Author

@anki2189 ping - have you had a chance to look into this yet? :)

@anki2189
Copy link
Member

anki2189 commented Mar 4, 2020

@steve-todorov : Yes, I am working on it , didn't get the time to test the flow.

@steve-todorov
Copy link
Member Author

Okay, thanks for the update! :)

@carlspring
Copy link
Member

@anki2189 ,

What is the status of this task?

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

Successfully merging a pull request may close this issue.

4 participants