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

Fix proxy.md #116

Merged
merged 1 commit into from
Jun 30, 2023
Merged

Fix proxy.md #116

merged 1 commit into from
Jun 30, 2023

Conversation

xingxingxia
Copy link
Contributor

@xingxingxia xingxingxia commented Apr 24, 2023

/cc @swghosh @thejasn
The display in https://github.com/openshift/cert-manager-operator/blob/master/docs/proxy.md does not hide ```bash, it displays as:

    ```bash
    oc -n cert-manager-operator patch subscription cert-manager-operator ...
    ```

Also, the subscription name (if installed via web) is openshift-cert-manager-operator instead. https://docs.openshift.com/container-platform/4.12/security/cert_manager_operator/cert-manager-operator-proxy.html needs update to be subscription openshift-cert-manager-operator as well (didn't catch this when GA sorry), CC @bergerhoffer .

@openshift-ci openshift-ci bot requested review from swghosh and thejasn April 24, 2023 03:17
@thejasn
Copy link
Contributor

thejasn commented Apr 24, 2023

/lgtm

@thejasn
Copy link
Contributor

thejasn commented Apr 24, 2023

cc: @xenolinux

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 24, 2023
@swghosh
Copy link
Member

swghosh commented Apr 25, 2023

/assign @xenolinux
if you could review, it'd be great as we need to update the docs accordingly

(JFYI, installing the operator from the GitHub source uses operator name as: "cert-manager-operator" while when installed from the productized build on an OpenShift cluster it is "openshift-cert-manager-operator" which the reason by the .md docs in this repo may contain different values to what's expected in openshift-docs)

TIA

@swghosh
Copy link
Member

swghosh commented Apr 28, 2023

/label px-approved
/label qe-approved
No functional changes are being introduced here.

@openshift-ci openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR labels Apr 28, 2023
docs/proxy.md Outdated
oc -n cert-manager-operator patch subscription cert-manager-operator --type='merge' -p '{"spec":{"config":{"env":[{"name":"TRUSTED_CA_CONFIGMAP_NAME","value":"trusted-ca"}]}}}'
```
```bash
oc -n cert-manager-operator patch subscription openshift-cert-manager-operator --type='merge' -p '{"spec":{"config":{"env":[{"name":"TRUSTED_CA_CONFIGMAP_NAME","value":"trusted-ca"}]}}}'
Copy link
Member

@swghosh swghosh May 18, 2023

Choose a reason for hiding this comment

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

@xingxingxia after giving this one a re-thought, IMO can we please keep this one as is in case of this GitHub repo and just update this to use "openshift-cert-manager-operator" only in https://github.com/openshift/openshift-docs would be totally in favour of getting the docs updated.
The rationale behind it being: users installing the operator from GitHub source will continue to use subscription name as "cert-manager-operator" while those who installed the productized RH version of the operator from OperatorHub (rh operator-index) would have subscription object named "openshift-cert-manager-operator" (which is becuase we patch the operator package id in midstream before building the productized version).

cc: @xenolinux

Copy link
Contributor Author

@xingxingxia xingxingxia May 18, 2023

Choose a reason for hiding this comment

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

Sure, but I updated with placeholder "<subscription_name>", is it fine?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's a good idea :)

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 18, 2023
@swghosh
Copy link
Member

swghosh commented May 18, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 18, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 18, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: swghosh, thejasn, xingxingxia

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@swghosh
Copy link
Member

swghosh commented Jun 30, 2023

/label docs-approved
PR for doc change is up: openshift/openshift-docs#61748

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Jun 30, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 30, 2023

@xingxingxia: 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/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot merged commit 65714ee into openshift:master Jun 30, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants