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

Added functionality for using the proxy without credentials. #115

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cheeseit
Copy link

Moved two methods.
Used the google style guide for this file.

Description

In our environment we use proxy without authentication. The authentication is done by another program. At this moment it is not possble to use a proxy without authentication. I add an if statement to check if the username is filled before creating the authentication. I have not created any tests because it is pretty hard to mock all the internals and couldnt find the other proxy tests.

Submitter checklist

  • Link to JIRA ticket in description, if appropriate.
  • Change is code complete and matches issue description.
  • Appropriate unit or acceptance tests or explanation to why this change has no tests.
  • Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

  • Run the changes and verified the change matches the issue description.
  • Reviewed the code.
  • Verified that the appropriate tests have been written or valid explanation given.
  • If applicable, tested by installing this plugin on the Jenkins instance.

Moved two methods.
Used the google style guide for this file.
@cheeseit
Copy link
Author

Do you need something more before the review proces can start or is it ok like this?

@cheeseit
Copy link
Author

Hello? anybody home?

@nrayapati
Copy link
Member

nrayapati commented Mar 12, 2021

Sorry it is been a while that I am not monitoring these repos actively, I will try to get to these couple of PRs on this repo, over this weekend. Thank you!

@cheeseit
Copy link
Author

Hi Nrayapati,

Is there something I can help with?

@nrayapati
Copy link
Member

Hi Nrayapati,

Is there something I can help with?

I tried testing this functionally, unfortunately I don't have a proxy setup to really verify these changes, would you be able to post some details i f you have verified and/or working example of this use case.

@cheeseit
Copy link
Author

Tomorrow I will get you an working example.
The use case we have is using your plugin with a proxy without authentication. So in the code I only added an if statement to check if the username is filled. If it is filled the authentication for the proxy is added otherwise no autentication is added. We are using another local proxy(cntlm) to do the authentication. This way you do not need to provide authetication for the proxy because cntlm already knows your credentials.

@cheeseit
Copy link
Author

cheeseit commented Mar 24, 2021

Hi I have added some pictures to prove it works. I have no proof that the proxy with authentication still works because that didnt work for us in the first place. The first picture shows the error we get when the old version is installed we get an timeout with -21 too many tunnels. In the last picture you can see we can get a response from our jira. Tell me if you need any else.

Configuration
proxysettingsandtest
With the old plugin
error_we_get_without_change
The new installed plugin
installedversion
Succesful connection with jira
afterinstallingplugin

@nrayapati nrayapati self-requested a review March 31, 2021 19:15
@nrayapati nrayapati self-assigned this Mar 31, 2021
@cheeseit
Copy link
Author

Hi nrayapati,
Any progress do you need something from me?

with kind regards,

Cheeseit

@cheeseit
Copy link
Author

cheeseit commented Jun 1, 2021

Any updates?

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 this pull request may close these issues.

2 participants