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

Add the doc on Secret customization #933

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

IwonaLanger
Copy link
Contributor

@IwonaLanger IwonaLanger commented Dec 19, 2024

Description

Changes proposed in this pull request:

  • Add the document on how to customize the sap-btp-manager Secret
  • Update 03-10-preconfigured-secret.md
  • Adjust 03-70-delete-bindings-and-instances.md

Related issue(s)

See #901

@IwonaLanger IwonaLanger added documentation Improvements or additions to documentation area/documentation Issues or PRs related to documentation kind/enhancement Categorizes issue or PR as related to modifying or improving an existing feature labels Dec 19, 2024
@IwonaLanger IwonaLanger self-assigned this Dec 19, 2024
@kyma-bot kyma-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cla: yes Indicates the PR's author has signed the CLA. labels Dec 19, 2024
Copy link

Add one of following labels

- kind/feature -> Use it when you want to submit a new feature

- kind/enhancement -> Use it when you modify or improve an existing feature

- kind/bug -> Use it when you fix a bug

@kyma-bot kyma-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 19, 2024
@IwonaLanger IwonaLanger marked this pull request as ready for review December 19, 2024 12:50
@IwonaLanger IwonaLanger requested review from a team as code owners December 19, 2024 12:50
@kyma-bot kyma-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 19, 2024
@kyma-gopher-bot kyma-gopher-bot enabled auto-merge (squash) December 19, 2024 12:50
docs/user/03-11-customize_secret.md Outdated Show resolved Hide resolved
docs/user/03-11-customize_secret.md Outdated Show resolved Hide resolved
docs/user/03-11-customize_secret.md Outdated Show resolved Hide resolved
docs/user/03-11-customize_secret.md Outdated Show resolved Hide resolved
docs/user/03-11-customize_secret.md Outdated Show resolved Hide resolved
docs/user/03-11-customize_secret.md Outdated Show resolved Hide resolved
NHingerl
NHingerl previously approved these changes Dec 19, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Dec 19, 2024
@IwonaLanger IwonaLanger removed the request for review from szwedm January 24, 2025 11:31

Your customized `sap-btp-manager` Secret is now the default Secret of the SAP BTP Operator module. It generates the SAP BTP service operator's resources, as shown in the following diagram:

![Customized module credentials](../assets/module_credentials_customized.drawio.svg)
Copy link
Member

Choose a reason for hiding this comment

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

Could we revert the direction of arrows between sap-btp-service-operator and sap-btp-manager secrets? Now it looks like sap-btp-manager points to values set in sap-btp-service-operator.

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 think that is correct as "It generates the SAP BTP service operator's resources."

Copy link
Contributor

Choose a reason for hiding this comment

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

Values from sap-btp-manager are the source during reconciliation and sap-btp-service-operator is the target.


The following parameters manage cluster access:

| Parameter | Description |
|-------------------------------|-----------------------------------------------------------------------------------------------|
| **CLUSTER_ID** | Generated when Kyma runtime is created. |
| **MANAGEMENT_NAMESPACE** | Always set to `kyma-system`. |
| **MANAGEMENT_NAMESPACE** | By default, set to `kyma-system`. |
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure that this config does and I cannot find it in out documentation. Maybe we could clarify it at least as a followup?

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'm not sure what to clarify here. Can you please clarify? This section is based on this source and modified according to PK's input.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More info has been added.

* Label the Secret with `kyma-project.io/skip-reconciliation: 'true'`.
* Provide the following credentials from your SAP Service Manager instance: **clientid**, **clientsecret**, **sm_url**, and **tokenurl**.
* Optionally, provide your **cluster_id**. Otherwise, it is generated automatically.
* Optionally, add the **credentials_namespace** parameter and provide the name of your custom namespace for Secrets with credentials to communicate with the SAP Service Manager.
Copy link
Member

Choose a reason for hiding this comment

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

After reading this sentence I understood that the custom namespace should already contain credentials to use. Is that correct understanding? If not, could you change to something like "...the name of your custom namespace where sap-btp-operator-service Secret will be generated."?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's correct, we assume the namespace already exists and it could contain anything user wants there. sap-btp-service-operator Secret also exists at this point, we just move it to the other namespace selected by user, it should remain the default Secret for communication with SAP Service Manager.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a rule. Using "will" is against the content guidelines.

Copy link
Member

Choose a reason for hiding this comment

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

So the credentials_namespace is the namespace where to move the secret to or from? In my previous, comment by "custom namespace" I meant credentials_namespace.

Copy link
Contributor

Choose a reason for hiding this comment

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

The credentials_namespace is the namespace for secrets, it does not concern only the sap-btp-service-operator secret. The sentence ...the name of your custom namespace where sap-btp-operator-service Secret will be generated." does not hold the true meaning of the namespace. User can create sap-btp-manager secret with the setting already provided and there won't be any movement of the secrets.


The following parameters manage cluster access:

| Parameter | Description |
|-------------------------------|-----------------------------------------------------------------------------------------------|
| **CLUSTER_ID** | Generated when Kyma runtime is created. |
| **MANAGEMENT_NAMESPACE** | Always set to `kyma-system`. |
| **MANAGEMENT_NAMESPACE** | By default, set to `kyma-system`. |
| **ALLOW_CLUSTER_ACCESS** | You can use every namespace for your operations. The parameter is always set to `true`.<br>If you change it to `false`, the setting is automatically reverted. |
Copy link
Contributor

Choose a reason for hiding this comment

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

@IwonaLanger please add here RELEASE_NAMESPACE too. Value: "By default, set to kyma-system."

@szwedm szwedm force-pushed the add_customization_doc branch from 65570ec to f529f8d Compare January 24, 2025 15:15
@kyma-bot kyma-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 27, 2025
Copy link

@NHingerl NHingerl left a comment

Choose a reason for hiding this comment

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

just minor suggestions

docs/user/03-10-preconfigured-secret.md Outdated Show resolved Hide resolved
docs/user/03-11-customize_secret.md Show resolved Hide resolved
docs/user/03-70-delete-bindings-and-instances.md Outdated Show resolved Hide resolved
docs/user/03-70-delete-bindings-and-instances.md Outdated Show resolved Hide resolved
You can't delete your Kyma cluster if any non-deleted service instances in it use the credentials from the SAP Service Manager resources created automatically, as described in [Preconfigured Credentials and Access](03-10-preconfigured-secret.md#credentials). In this case, the existing service instances block the cluster's deletion. Delete your service instances and bindings in Kyma dashboard before you attempt to delete the cluster from the SAP BTP cockpit.

You can delete your Kyma cluster even if your service instances still exist, provided they all use credentials of SAP Service Manager service instances other than the one created automatically, as described in [Preconfigured Credentials and Access](03-10-preconfigured-secret.md#credentials). In this case, the non-deleted service instances do not block the cluster's deletion.

Choose a reason for hiding this comment

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

Suggested change
You can delete your Kyma cluster even if your service instances still exist, provided they all use credentials of SAP Service Manager service instances other than the one created automatically, as described in [Preconfigured Credentials and Access](03-10-preconfigured-secret.md#credentials). In this case, the non-deleted service instances do not block the cluster's deletion.
However, if the service instances use credentials of SAP Service Manager service instances other than the one created automatically, you can delete your Kyma cluster even if your service instances still exist (see [Preconfigured Credentials and Access](03-10-preconfigured-secret.md#credentials)). In this case, the non-deleted service instances do not block the cluster's deletion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skipping "all" and moving the link changes the message.

@szwedm szwedm self-assigned this Jan 28, 2025
Copy link

@NHingerl NHingerl left a comment

Choose a reason for hiding this comment

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

It might look nicer if the three boxes had the same size:
image

Sorry I didn't notice earlier :(

docs/user/03-10-preconfigured-secret.md Outdated Show resolved Hide resolved
docs/user/03-11-customize_secret.md Show resolved Hide resolved
@kyma-bot kyma-bot added the lgtm Looks good to me! label Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Issues or PRs related to documentation cla: yes Indicates the PR's author has signed the CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. documentation Improvements or additions to documentation kind/enhancement Categorizes issue or PR as related to modifying or improving an existing feature lgtm Looks good to me! 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.

8 participants