Skip to content
This repository was archived by the owner on May 16, 2025. It is now read-only.

Using the FQDN of a GCE instance instead of the IP address. - #815

Merged
ivannaranjo merged 5 commits into
GoogleCloudPlatform:masterfrom
ivannaranjo:master_fixing_790
Oct 3, 2017
Merged

Using the FQDN of a GCE instance instead of the IP address.#815
ivannaranjo merged 5 commits into
GoogleCloudPlatform:masterfrom
ivannaranjo:master_fixing_790

Conversation

@ivannaranjo

@ivannaranjo ivannaranjo commented Oct 2, 2017

Copy link
Copy Markdown
Contributor

This PR changes the way we refer to the machine when opening a terminal server session, by using the FQDN of the GCE instance instead of the bare IP address.

Fixes #790

@codecov

codecov Bot commented Oct 2, 2017

Copy link
Copy Markdown

Codecov Report

Merging #815 into master will increase coverage by 0.13%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #815      +/-   ##
==========================================
+ Coverage   11.84%   11.97%   +0.13%     
==========================================
  Files         504      504              
  Lines       12326    12331       +5     
==========================================
+ Hits         1460     1477      +17     
+ Misses      10866    10854      -12
Impacted Files Coverage Δ
...dExtension/TerminalServer/TerminalServerManager.cs 0% <0%> (ø) ⬆️
...leCloudExtension.DataSources/InstanceExtensions.cs 27.27% <100%> (+27.27%) ⬆️
...eCloudExtension.DataSources/WindowsInstanceInfo.cs 62.5% <0%> (+62.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 564cd16...6865ba1. Read the comment docs.

@ivannaranjo
ivannaranjo requested a review from ILMTitan October 2, 2017 13:13
@ivannaranjo ivannaranjo added this to the M2.1-2017 milestone Oct 2, 2017
@ivannaranjo ivannaranjo changed the title Master fixing 790 Using the FQDN of a GCE instance instead of the IP address. Oct 2, 2017

@ILMTitan ILMTitan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Needs unit tests.

}

var reverseIp = string.Join(".", ipAddress.Split('.').Reverse());
return $"{reverseIp}.bc.googleusercontent.com";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Create unit tests for this function? You should really only need three cases (null instance, null ipAddress, proper ipAddress).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, will see what I can do.

@ivannaranjo

Copy link
Copy Markdown
Contributor Author

Didn't add a case for the null instance because I don't support that case.

PTAL.

@ILMTitan ILMTitan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM.

@ivannaranjo
ivannaranjo merged commit 182b8bd into GoogleCloudPlatform:master Oct 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants