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

Updates to the Proxy Handling #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kms254
Copy link

@kms254 kms254 commented Apr 2, 2018

detect-task.ps1:
When hub is running on a machine that does not have direct outside internet connection it won't be able to reach the github repository to download the newest version of detect.ps1. This change uses the Hub Proxy settings with the Invoke-RestMethod to use the proxy to get outside access. If you are not using the proxy it does nothing different.

detect.ps1

Previously the Proxy Url builder would end up returning an empty string with the way this was parsing. Now we are using the Proxy's string to create the new URIBuilder object and then assigning the port. Now when we call ProxyUrlBuilder.Uri we actually get a correctly configured Uri verses an empty value which ends up throwing an exception.

Test Code:

Write-Host "ProxyUrlBuild Host: " $ProxyUrlBuilder.Host
Write-Host "ProxyUrlBuild Port: " $ProxyUrlBuilder.Port
Write-Host "ProxyUrlBuild Uri: "  $ProxyUrlBuilder.Uri
$ProxyInfoProperties.Uri = $ProxyUrlBuilder.Uri

Output Before:

Checking for proxy.
Found proxy host.
Found proxy port.
ProxyUrlBuild Host: [http://xyx.domainName.com]
ProxyUrlBuild Port: 1234
ProxyUrlBuild Uri:
Found proxy credentials.
Successfully setup proxy.
Getting detect.
Finding latest detect version.
System.Management.Automation.ValidationMetadataException
The cmdlet cannot run because the following parameter is missing: Proxy. Provide a valid proxy URI for the Proxy parameter when using the ProxyCredential or UseDefaultProxyCredentials parameters, then retry.

Output After:

Checking for proxy.
Found proxy host.
Found proxy port.
ProxyUrlBuild Host: http://xyx.domainName.com
ProxyUrlBuild Port: 1234
ProxyUrlBuild Uri: http://xyx.domainName.com:1234/
Found proxy credentials.
Successfully setup proxy.
Getting detect.
Finding latest detect version.
Resolved version 3.1.0
Using detect version 3.1.0

Kevin Still added 2 commits April 2, 2018 14:24
retrieve hub-detect.ps1.

Updating detect.ps to not truncate the proxy address when you have a
port associated with it.
@taikuukaits
Copy link
Contributor

Hello Kevin Still,

These are great ideas and thank you very much for the PR. If I understand what you are trying to accomplish:

  1. Use proxy when pulling detect.ps1 which I agree this is something we should support. There is code already to handle the more complex proxy cases in detect.ps1 which I'd rather pull up to detect-task.ps1 such as handling a proxy with no credentials.
  2. In detect.ps1: The uri is blank even after setting the host. I did some debugging and it is because the host contains a scheme. If I use "http://xyx.domainName.com" the Uri is blank but if I use "xyx.domainName.com" it works. This will also need to be fixed in the detect powershell script in the detect repo. The file modified in this pull request is automatically updated with that file (this is a cached version).

To that end, I think the implementation should be approached a little differently. I will open a new PR and incorporate your concepts and add you to the PR so you can contribute. How does that sound?

@kms254
Copy link
Author

kms254 commented Apr 6, 2018

@taikuukaits

  1. Correct, and moving the more complex handling up to the detect-task.ps1 file sounds like a good idea. What I have done will work with a proxy with no credentials, at lest in my testing.

  2. Taking what you said here, I used it to help solve some of my issues in the actual Detect executable as well. It also does not handle the case when the proxy host has its protocol prepended to it when trying to phone home to https://collect.blackduck.com. Which looking at your codebase I think there is an entire repo for the phone home package.

Feel free to make another PR with your changes.

@taikuukaits
Copy link
Contributor

@kms254

I do wonder if the protocol works all the way through detect, I'll create a ticket for detect to confirm. In theory, the TFS/Powershell can use the uri builder to only pass the host and not the scheme.

I've opened the new PR: #2
And I've opened one for detect: blackducksoftware/hub-detect#226

I believe it addresses both issues.

Thanks!

stavvy-akamen pushed a commit that referenced this pull request Nov 14, 2018
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