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

Volume mount Support #72

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Volume mount Support #72

wants to merge 7 commits into from

Conversation

anirudhprasad-sap
Copy link
Contributor

No description provided.

Copy link

sonarcloud bot commented Mar 8, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
60.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@anirudhprasad-sap
Copy link
Contributor Author

anirudhprasad-sap commented Mar 21, 2024

An evaluation was done to store service secrets as volume mounts to support credential rotation. But we have the following issues-

  1. CAP doesn't support credential rotation - #/cap/issues/issues/15618. The recommendation is to restart pods but this can be done now also.
  2. Approuter uses xsenv api's that don't have the disable cache options. This would mean adoption in app router component as well to support credential rotation.

Because of these drawbacks, it doesn't make sense to support volume mounts for secrets right now. We will revisit the topic once the above points are resolved.

@Pavan-SAP Pavan-SAP deleted the volumeMount branch May 21, 2024 15:49
@Pavan-SAP Pavan-SAP restored the volumeMount branch October 2, 2024 12:18
@Pavan-SAP Pavan-SAP reopened this Oct 2, 2024
@@ -618,8 +647,8 @@ func getContainer(params *DeploymentParameters) []corev1.Container {
ImagePullPolicy: params.WorkloadDetails.DeploymentDefinition.ImagePullPolicy,
Command: params.WorkloadDetails.DeploymentDefinition.Command,
Env: getEnv(params),
Copy link
Contributor

Choose a reason for hiding this comment

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

We also need to enhance the env in this case to add SERVICE_BINDING_ROOT in the xsenv scenarios.

Copy link

sonarcloud bot commented Oct 8, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
65.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

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

Successfully merging this pull request may close these issues.

2 participants