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

RHIDP-4780: Document per-ConfigMap/Secret configuration of mountPath #842

Open
wants to merge 5 commits into
base: release-1.4
Choose a base branch
from

Conversation

pabel-rh
Copy link
Contributor

@pabel-rh pabel-rh commented Jan 13, 2025

IMPORTANT: Do Not Merge - To be merged by Docs Team Only

Version(s): 1.4.x, main
Issue: RHIDP-4780
**Preview link:**https://redhat-developer.github.io/red-hat-developers-documentation-rhdh/pr-842/configuring/#mounting-additional-files-in-your-custom-configuration-using-rhdh-operator

(This is showing the new procedure as Chapter 2, but it's supposed to be under 1.2. So, technically, as Chapter 1.2.1.. I've made the correction in the second commit but for some reason it isn't reflecting in the preview link)

@rhdh-bot
Copy link
Collaborator

rhdh-bot commented Jan 13, 2025

Copy link
Member

@linfraze linfraze left a comment

Choose a reason for hiding this comment

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

This content is a great addition to the docs. A few comments.


.Procedure

. In {ocp-short}, author your ConfigMap or Secret custom resource in a YAML file. For more information, see link:{installing-on-ocp-book-url}#provisioning-your-custom-configuration[].
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the module you are trying to link to is not in the Installing on OCP title, but rather in the Configuring title. Since we are already in the Configuring title, you should be able to link with an internal xref. Try the folloiwng:

Suggested change
. In {ocp-short}, author your ConfigMap or Secret custom resource in a YAML file. For more information, see link:{installing-on-ocp-book-url}#provisioning-your-custom-configuration[].
. In {ocp-short}, author your ConfigMap or Secret custom resource in a YAML file. For more information, see xref:provisioning-your-custom-configuration[Provisioning and using your custom {product} configuration].

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if "author" is the best word in this context. Maybe "configure"? No change, just something to consider and to ensure we are writing consistently across the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see! Thanks so much for the help with the Cross references, Lindsey. I'll keep that in mind.
Yes, I also found "author" a bit odd. I went with your suggestion and I found a mention of this in another doc:
https://docs.redhat.com/en/documentation/red_hat_developer_hub/1.4/html/telemetry_data_collection/assembly-rhdh-telemetry#proc-enabling-telemetry-using-operator_title-telemetry

It says, "Create xyz ConfigMap with the following YAML code"...

----
====

. Declare the ConfigMap or Secret file in your `{product-custom-resource-type}` custom resource.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
. Declare the ConfigMap or Secret file in your `{product-custom-resource-type}` custom resource.
. Declare the ConfigMap or Secret file in your `{product-custom-resource-type}` custom resource. For example:

Copy link
Member

Choose a reason for hiding this comment

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

Is Declare the right word? Maybe. But the linter for 8th grade language might flag it and suggest that a simpler word can be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so either. I rewrote it as per the text I found here:
https://docs.redhat.com/en/documentation/red_hat_developer_hub/1.4/html/telemetry_data_collection/assembly-rhdh-telemetry#proc-enabling-telemetry-using-operator_title-telemetry

But I am a little doubtful about the accuracy of 'configMaps name' and 'secrets name'. Also, I feel the line - "Set the value of the 'configMaps name' or 'secrets name' to the name of the ConfigMap or Secret file......." could be slightly confusing. Would it be a good idea to split this into two separate lines, by saying, "Set the value of the 'configMaps name' to the name of the ConfigMap file or the 'secrets name' to the name of the Secret file in your {product-custom-resource-type} CR." ?

@pabel-rh
Copy link
Contributor Author

@linfraze @jmagak , thank you so much for your valuable reviews. I have incorporated your comments.
@linfraze , for one of the comments you've provided, I've requested your suggestions on the alternate text. Would you please take a look?

apiVersion: v1
kind: ConfigMap
metadata:
name: {my-product-cr-name}
Copy link
Member

Choose a reason for hiding this comment

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

name: {my-product-configmap}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a slight change in the attributes. I've changed it to {my-app-config-config-map} which displays "my-rhdh-app-config".

extraFiles:
mountPath: /my/path
configMaps:
- name: {my-app-config-config-map}
Copy link
Member

Choose a reason for hiding this comment

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

name: {my-product-configmap}

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

Successfully merging this pull request may close these issues.

5 participants