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

ui: Remove any trailing fullstop/period DNS characters from Gateways UI API #9752

Merged
merged 3 commits into from
Feb 25, 2021

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Feb 10, 2021

Previous to this commit, the API response for getting information about the upstreams for a gateway (/v1/internal/ui/gateway-services-nodes/:gateway) would include Gateway Addresses in the form domain.name.:8080, which due to the addition of the port is probably not the expected format (the trailing . is DNS not Address:Port URLs).

This commit rightTrims any . characters from the end of the domain before formatting the address to include the port resulting in domain.name:8080

One thing I'm unsure of, whilst this 'fixes' things for displaying this in the UI, does this effect anything else backend related? I had a quick search to see is the .Addresses() method was being used elsewhere, and I couldn't see anything, but would be good to double check.

Previous t othis commit, the API response would include Gateway
Addresses in the form `domain.name.:8080`, which due to the addition of
the port is probably not the expected response.

This commit rightTrims any `.` characters from the end of the domain
before formatting the address to include the port resulting in
`domain.name:8080`
@hashicorp-ci
Copy link
Contributor

🤔 Double check that this PR does not require a changelog entry in the .changelog directory. Reference

@vercel vercel bot temporarily deployed to Preview – consul February 11, 2021 09:26 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging February 11, 2021 09:26 Inactive
@johncowen johncowen changed the title Right trim any trailing fullstop/period DNS characters from Gateways Right trim any trailing fullstop/period DNS characters from Gateways UI API Feb 11, 2021
@johncowen johncowen changed the title Right trim any trailing fullstop/period DNS characters from Gateways UI API ui: Right trim any trailing fullstop/period DNS characters from Gateways UI API Feb 11, 2021
@johncowen johncowen requested a review from crhino February 11, 2021 09:45
@johncowen johncowen added the theme/ui Anything related to the UI label Feb 11, 2021
@johncowen johncowen marked this pull request as ready for review February 11, 2021 09:45
@johncowen johncowen changed the title ui: Right trim any trailing fullstop/period DNS characters from Gateways UI API ui: Remove any trailing fullstop/period DNS characters from Gateways UI API Feb 11, 2021
Copy link
Contributor

@crhino crhino left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for doing this!

@crhino
Copy link
Contributor

crhino commented Feb 23, 2021

We should probably add a changelog entry for this as well.

@vercel vercel bot temporarily deployed to Preview – consul February 24, 2021 09:06 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging February 24, 2021 09:06 Inactive
@johncowen
Copy link
Contributor Author

Thanks @crhino! I've added a changelog so I'll merge this a little later on 👍

@johncowen johncowen merged commit 5892e75 into master Feb 25, 2021
@johncowen johncowen deleted the gateway-address-trailing-dots branch February 25, 2021 09:34
@hashicorp-ci
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/332203.

@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit 5892e75 onto release/1.9.x succeeded!

hashicorp-ci pushed a commit that referenced this pull request Feb 25, 2021
…UI API (#9752)

Previous to this commit, the API response would include Gateway
Addresses in the form `domain.name.:8080`, which due to the addition of
the port is probably not the expected response.

This commit rightTrims any `.` characters from the end of the domain
before formatting the address to include the port resulting in
`domain.name:8080`
@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit 5892e75 onto release/1.8.x succeeded!

hashicorp-ci pushed a commit that referenced this pull request Feb 25, 2021
…UI API (#9752)

Previous to this commit, the API response would include Gateway
Addresses in the form `domain.name.:8080`, which due to the addition of
the port is probably not the expected response.

This commit rightTrims any `.` characters from the end of the domain
before formatting the address to include the port resulting in
`domain.name:8080`
dizzyup pushed a commit that referenced this pull request Apr 21, 2021
…UI API (#9752)

Previous to this commit, the API response would include Gateway
Addresses in the form `domain.name.:8080`, which due to the addition of
the port is probably not the expected response.

This commit rightTrims any `.` characters from the end of the domain
before formatting the address to include the port resulting in
`domain.name:8080`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants