Skip to content

Conversation

wgabor0427
Copy link
Contributor

@wgabor0427 wgabor0427 commented Sep 3, 2025

@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 3, 2025
Copy link

@bharath-b-rh bharath-b-rh left a comment

Choose a reason for hiding this comment

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

@wgabor0427 Thank you for the doc udpates.

For ESO we will need to document on configuring the proxy parameters http_proxy, https_proxy and noproxy, but ESO doesn't have trust bundle related use case. Let me share a draft on the required content.

@wgabor0427
Copy link
Contributor Author

@bharath-b-rh I was wondering if you had a draft of the required content yet. Thanks.

@wgabor0427 wgabor0427 force-pushed the OSDOCS-16038 branch 2 times, most recently from f46e2ef to 5d8dc83 Compare September 30, 2025 14:15
@wgabor0427 wgabor0427 force-pushed the OSDOCS-16038 branch 7 times, most recently from 3eae74a to dd64ea6 Compare October 2, 2025 21:04


// Proxy security considerations
include::modules/external-secrets-proxy-security-considerations.adoc[leveloffset=+2]

Choose a reason for hiding this comment

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

We need to add it as a sub-section under 11.1. External Secrets Operator for Red Hat OpenShift overview

@wgabor0427 wgabor0427 force-pushed the OSDOCS-16038 branch 2 times, most recently from 9c8af29 to fd11cc1 Compare October 7, 2025 18:46
Copy link

@bharath-b-rh bharath-b-rh left a comment

Choose a reason for hiding this comment

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

/lgtm

except for couple of nits. Thank you!

* xref:../../installing/overview/installing-fips.adoc#installing-fips-mode_installing-fips[Installing a cluster in FIPS mode]
* xref:../../installing/overview/installing-preparing.adoc#installing-preparing-security[Do you need extra security for your cluster?]

// Proxy security considerations

Choose a reason for hiding this comment

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

Security considerations are applicable to the product in general, maybe we need to update it to below or something similar.

Suggested change
// Proxy security considerations
// Product security considerations


:_mod-docs-content-type: REFERENCE
[id="external-secrets-proxy-security-considerations_{context}"]
= Security considerations when using the egress proxy

Choose a reason for hiding this comment

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

Suggested change
= Security considerations when using the egress proxy
= Security Considerations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe say "Security considerations"

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2025
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2025
@bharath-b-rh
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2025
@wgabor0427
Copy link
Contributor Author

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Oct 10, 2025
@bscott-rh bscott-rh added merge-review-in-progress Signifies that the merge review team is reviewing this PR and removed merge-review-needed Signifies that the merge review team needs to review this PR labels Oct 13, 2025
Copy link
Contributor

@bscott-rh bscott-rh left a comment

Choose a reason for hiding this comment

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

Generally looks good, but there is a procedure that needs to be slightly reworked for DITA compliance. Also one suggestion - if those features are going GA, you may as well delete the tech preview lines rather than just commenting them out. Up to you.

<2> Proxy URL for the https requests.
<3> Comma-separated list of hostnames and/or CIDRs and/or IPs for which the proxy should not be used.

To set the proxy in the `ExternalSecretsManager` resource, perform the following steps.
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there are two separate procedures contained in a single procedure module, which is not allowed from a DITA migration perspective. You can either break this into two separate procedure modules, or turn it into a "compound procedure" by doing something like this:

* To set the proxy in the `ExternalSecretsConfig` resource, perform the following steps:
.. step 1
.. step 2
* To set the proxy in the `ExternalSecretsManager` resource, perform the following steps:
.. step 1
.. step 2

or you can generalize the procedure so that one procedure covers both cases, like this:

. Edit the custom resource you want to use by running the following command:
+
[source,terminal]
----
$ oc edit <cr_name> cluster
----
+
etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Put it into a "compound procedure"

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2025
Copy link

openshift-ci bot commented Oct 13, 2025

New changes are detected. LGTM label has been removed.

Copy link

openshift-ci bot commented Oct 13, 2025

@wgabor0427: all tests passed!

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

Copy link
Contributor

@bscott-rh bscott-rh left a comment

Choose a reason for hiding this comment

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

Merge review LGTM.

@bscott-rh bscott-rh added branch/enterprise-4.20 and removed merge-review-in-progress Signifies that the merge review team is reviewing this PR labels Oct 13, 2025
@bscott-rh bscott-rh added this to the Planned for 4.20 GA milestone Oct 13, 2025
@bscott-rh bscott-rh merged commit b65ceae into openshift:main Oct 13, 2025
2 checks passed
@bscott-rh
Copy link
Contributor

/cherrypick enterprise-4.20

@openshift-cherrypick-robot

@bscott-rh: new pull request created: #100405

In response to this:

/cherrypick enterprise-4.20

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-sigs/prow repository.

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

Labels

branch/enterprise-4.20 size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants