Skip to content

BZ:1942662 - Adding firewall URL sections to point 5 instead of 4#37792

Merged
lpettyjo merged 1 commit intoopenshift:mainfrom
kelbrown20:additional-url-needed-in-firewall-1942662
Nov 30, 2021
Merged

BZ:1942662 - Adding firewall URL sections to point 5 instead of 4#37792
lpettyjo merged 1 commit intoopenshift:mainfrom
kelbrown20:additional-url-needed-in-firewall-1942662

Conversation

@kelbrown20
Copy link
Contributor

@kelbrown20 kelbrown20 commented Oct 20, 2021

For Versions 4.6+
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1942662

Description: It was recommended to move oso-rhc4tp-docker-registry.s3-us-west-2.amazonaws.com to point 5 instead of point 4

Preview: https://deploy-preview-37792--osdocs.netlify.app/openshift-enterprise/latest/installing/install_config/configuring-firewall

Ready for QA: @xiuwang

For Peer Reviewers: Can you please add 'enterprise-4.6', 'enterprise-4.7', 'enterprise-4.8' enterprise-4.9' 'enterprise-4.10' and 'Peer Review needed'. Thank you!!

Note: Still in progress

@openshift-ci openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 20, 2021
@netlify
Copy link

netlify bot commented Oct 20, 2021

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 4bdf39d

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/61a658c5d852ca0008b0d6fb

😎 Browse the preview: https://deploy-preview-37792--osdocs.netlify.app/openshift-enterprise/latest/installing/install_config/configuring-firewall

Copy link
Contributor

@codyhoag codyhoag left a comment

Choose a reason for hiding this comment

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

The table edits look good! Just a nit and question.

Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, we should add a period at the end to match the other descriptions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I gather that the reasoning for adding this to point 5 is because this URL is also needed for other clusters not installed on AWS. If so, perhaps we should add specifics on that to the description? As it stands, it still reads as though this is AWS-only. A good comparison is the Google URL storage.googleapis.com/openshift-release listed in step 5.

@codyhoag codyhoag added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Oct 20, 2021
@kelbrown20 kelbrown20 force-pushed the additional-url-needed-in-firewall-1942662 branch 2 times, most recently from 475dbc7 to 2191bed Compare October 25, 2021 15:29
@openshift-ci openshift-ci bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 25, 2021
@kelbrown20
Copy link
Contributor Author

@xiuwang We were uncertain if the URL oso-rhc4tp-docker-registry.s3-us-west-2.amazonaws.com is required for other AWS specifications, including the ones described in point 4, as well as " Required to access images on registry.connect.redhat.com." in the new description? We were thinking about leaving the link in both sections, but we weren't sure if the URL had both of these functions

@lautou
Copy link

lautou commented Oct 29, 2021

@xiuwang @codyhoag
According to the BZ:
https://registry.connect.redhat.com/v2/f5networks/k8s-bigip-ctlr-operator/blobs/sha256:2490557f8586f8f37df6402c083720f6feb0945ffde3a3cee84c6715543aa9c6
https://registry.connect.redhat.com/v2/sonatype/nexus-repository-manager/blobs/sha256:e598ae6c78d234638ffbcd2529f46525fdd7eb517d7919a61bb55fafa1879835
The 2 operators which are referencing this URL are third party vendors operators (Sonatype Nexus and F5 Big IP).
We should rather mention their whitelisting as optional in a separated section.
We can create a new section:

Allowlist the following URLs for optional operators:
URL | Port | Function
oso-rhc4tp-docker-registry.s3-us-west-2.amazonaws.com | 443 | Required for Sonatype Nexus, F5 Big IP operators

@kelbrown20
Copy link
Contributor Author

@lautou So do you think the registry.connect.redhat.com section that was added should be removed from point 5?

@codyhoag
Copy link
Contributor

@lautou a lot of those URLs specified in step 5 could be seen as optional in certain scenarios. Perhaps we could rephrase step 5 to state whitelisting "as necessary", based on the needs of the custom installation.

I'll defer to @xiuwang for any technical considerations 🙂

@lautou
Copy link

lautou commented Oct 29, 2021

@kelbrown20 registry.connect.redhat.com is the registry for installing 3rd party certified operators by Red Hat partners.
This url is need only if you plan to install theses operators from the partners. Yes it should be considered as optional.
According to the BZ this URL redirect to oso-rhc4tp-docker-registry.s3-us-west-2.amazonaws.com for Sonatype Nexus and F5 Big IP operators.
I cannot confirm if there are any other redirections from registry.connect.redhat.com depending the 3rd party operators.
At least we can build the allowlist for optional operators:

URL | Port | Function
registry.connect.redhat.com | 443 | Required for all third party certified operators
oso-rhc4tp-docker-registry.s3-us-west-2.amazonaws.com | 443 | Required for Sonatype Nexus, F5 Big IP operators

@xiuwang
Copy link

xiuwang commented Nov 1, 2021

LGTM

@kelbrown20 kelbrown20 force-pushed the additional-url-needed-in-firewall-1942662 branch from 2191bed to 4c4c28e Compare November 1, 2021 14:47
@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 1, 2021
@kelbrown20
Copy link
Contributor Author

@xiuwang @lautou How does this updated doc look?

@lautou
Copy link

lautou commented Nov 3, 2021

update for optional operators URL is correct but i have one remark for the text below:

Operators require route access to perform health checks. Specifically, the authentication and web console Operators connect to two routes to verify that the routes work. If you are the cluster administrator and do not want to allow *.apps.<cluster_name>.<base_domain>, then allow these routes:
* oauth-openshift.apps.<cluster_name>.<base_domain>
* console-openshift-console.apps.<cluster_name>.<base_domain>, or the hostname that is specified in the spec.route.hostname field of the consoles.operator/cluster object if the field is not empty.

This text should be rather placed below the table containing *.apps.<cluster_name>.<base_domain> URL.

@jianzhangbjz
Copy link
Member

@kelbrown20 Sorry for the late reply. I missed it, yes looks good to me, thanks!
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2021
@kelbrown20 kelbrown20 force-pushed the additional-url-needed-in-firewall-1942662 branch from d3a6786 to 172c1df Compare November 30, 2021 15:27
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2021
@openshift-ci
Copy link

openshift-ci bot commented Nov 30, 2021

New changes are detected. LGTM label has been removed.

@lpettyjo lpettyjo self-assigned this Nov 30, 2021
@lpettyjo lpettyjo added peer-review-needed Signifies that the peer review team needs to review this PR and removed peer-review-done Signifies that the peer review team has reviewed this PR labels Nov 30, 2021
@lpettyjo
Copy link
Contributor

The term third-party is hyphenated. I see two instances to correct, but check for more. Otherwise, LGTM.

@kelbrown20 kelbrown20 force-pushed the additional-url-needed-in-firewall-1942662 branch from 172c1df to 4bdf39d Compare November 30, 2021 17:00
@lpettyjo lpettyjo added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Nov 30, 2021
@lpettyjo lpettyjo merged commit f8c6fed into openshift:main Nov 30, 2021
@lpettyjo
Copy link
Contributor

/cherrypick enterprise-4.10

@openshift-cherrypick-robot

@lpettyjo: new pull request created: #39374

Details

In response to this:

/cherrypick enterprise-4.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@lpettyjo
Copy link
Contributor

/cherrypick enterprise-4.9

@lpettyjo
Copy link
Contributor

/cherrypick enterprise-4.8

@openshift-cherrypick-robot

@lpettyjo: new pull request created: #39375

Details

In response to this:

/cherrypick enterprise-4.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@lpettyjo
Copy link
Contributor

/cherrypick enterprise-4.7

@openshift-cherrypick-robot

@lpettyjo: new pull request created: #39376

Details

In response to this:

/cherrypick enterprise-4.8

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@lpettyjo
Copy link
Contributor

/cherrypick enterprise-4.6

@openshift-cherrypick-robot

@lpettyjo: new pull request created: #39377

Details

In response to this:

/cherrypick enterprise-4.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot

@lpettyjo: new pull request created: #39378

Details

In response to this:

/cherrypick enterprise-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.6 branch/enterprise-4.7 branch/enterprise-4.8 branch/enterprise-4.9 branch/enterprise-4.10 peer-review-done Signifies that the peer review team has reviewed this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[enterprise-4.9] Issue in file installing/install_config/configuring-firewall.adoc

7 participants