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

Doc updates for egress IP addresses #1566

Merged
merged 1 commit into from
Aug 21, 2024
Merged

Conversation

skord
Copy link
Member

@skord skord commented Aug 19, 2024

Description:

This just updates docs to reflect the three current possible egress IP's for Flow.

Workflow steps:

(How does one use this feature, and how has it changed)

Documentation links affected:

It's all documentation, so every doc in the commit.


This change is Reviewable

@skord skord requested a review from a team August 19, 2024 17:58
@skord skord self-assigned this Aug 19, 2024
Copy link

github-actions bot commented Aug 19, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-08-21 15:01 UTC

@@ -286,7 +288,7 @@ In some cases, your source or destination endpoint may be within a secure networ
to allow direct access to its port due to your organization's security policy.

:::tip
If permitted by your organization, a quicker solution is to whitelist the Estuary IP address, `34.121.207.128`.
If permitted by your organization, a quicker solution is to whitelist the Estuary IP addresses, `34.121.207.128, 35.226.75.135, 34.68.62.148`.
Copy link
Member

Choose a reason for hiding this comment

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

I think from a UI/UX aspect breaking these up into a small list that has one IP per line could make sense. However, that would make them a lot harder to copy/paste the list.

From an ops perspective would keeping these as a comma separated list make it easier to configure on the client's system?

image

Copy link
Member Author

Choose a reason for hiding this comment

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

image

This is actually kind of tricky. I moved these into a code block for where it's in a tool tip and it looks like the attached.

In other cases, I just added a : so it was clear a list was coming. Trying to make a UL in markdown for other cases was very visually jarring. The other option was to put something like

34.121.207.128, 35.226.75.135 and 34.68.62.148

for proper English, but the flow on that is gross too.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me and follows the pattern up above... though that is kinda different given you really should copy the value.

image

@skord skord force-pushed the mdanko/ip-addr-doc-updates branch from be56166 to c7fc51a Compare August 20, 2024 20:31
committing for preview

formatting
@skord skord force-pushed the mdanko/ip-addr-doc-updates branch from c7fc51a to 3d0c6aa Compare August 21, 2024 13:39
Copy link
Member

@travjenkins travjenkins left a comment

Choose a reason for hiding this comment

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

LGTM

Tested the main changes on pages with the new IPs (just searched for 35.226.75.135 and hit those pages).

Hit a few other pages that changed due to linting and they all seem to match prod.

@skord skord merged commit 27cf5c7 into master Aug 21, 2024
1 check passed
@skord skord deleted the mdanko/ip-addr-doc-updates branch August 21, 2024 14:59
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