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

[Feat] Volume mount service secrets on workloads #72

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

Conversation

anirudhprasad-sap
Copy link
Contributor

@anirudhprasad-sap anirudhprasad-sap commented Mar 8, 2024

Volume mount service secrets on workloads instead of using VCAP. Enabled by setting annotation sme.sap.com/use-credential-volume-mount: "true" on the CAPApplicationVersion resource.

Test controller image - ghcr.io/anirudhprasad-sap/cap-operator/controller:vol-mnt-3 ghcr.io/anirudhprasad-sap/cap-operator/controller:vol-mnt-4

@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
@anirudhprasad-sap anirudhprasad-sap changed the title Volume mount Support [Feat] Volume mount service secrets on workloads Oct 15, 2024
@anirudhprasad-sap
Copy link
Contributor Author

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.

Even though the above issue still exists, we decided to merge it. This feature can be enabled by setting annotation sme.sap.com/use-volume-mount: "true" on the CAPApplicationVersion.

@SAP SAP deleted a comment from sonarcloud bot Oct 15, 2024
@anirudhprasad-sap anirudhprasad-sap marked this pull request as ready for review October 15, 2024 12:33
Pavan-SAP

This comment was marked as resolved.

volume mount annotation changed
@anirudhprasad-sap
Copy link
Contributor Author

can we rename the annotation to say one of : sme.sap.com/services-use-volume-mount sme.sap.com/use-credential-volume-mount sme.sap.com/use-services-volume-mount

the existing one is a bit too generic IMO.

I updated the annotation to sme.sap.com/use-credential-volume-mount - 6a3e6a8

Copy link

sonarcloud bot commented Oct 24, 2024

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