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

Refactor argument placeholder to support SPI #34243

Closed
wants to merge 3 commits into from

Conversation

JoshuaChen
Copy link
Contributor

@JoshuaChen JoshuaChen commented Jan 4, 2025

In some cloud compliance scenarios, using environment properties or system properties to manage secrets is not allowed. Instead, we need to use approved property sources to handle these sensitive values.

To meet this requirement, I refactored the argument placeholder so it can support reading from other property sources through a plugin module. This ensures compliance with the rules.


Before committing this PR, I'm sure that I have checked the following options:

  • My code follows the code of conduct of this project.
  • I have self-reviewed the commit code.
  • I have (or in comment I request) added corresponding labels for the pull request.
  • I have passed maven check locally : ./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.
  • I have made corresponding changes to the documentation.
  • I have added corresponding unit tests for my changes.
  • I have updated the Release Notes of the current development version. For more details, see Update Release Note

Copy link
Member

@strongduanmu strongduanmu left a comment

Choose a reason for hiding this comment

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

The scenario you introduced does not seem to be something ShardingSphere should consider. ShardingSphere should be as simple and standard as possible instead of considering personal scenarios.

@JoshuaChen
Copy link
Contributor Author

JoshuaChen commented Jan 5, 2025

Um~I believe compliance requirements are not personal preferences but mandatory rules. In China, we have security classification protection requirements, while multinational companies must comply with various regulations in different countries.

Here’s a simple example: In AWS, sensitive keys, like database passwords, should be stored in AWS Secrets Manager. They should not appear in environment variables or system properties because this could expose the database password to unauthorized users. In private infrastructure, such keys should be stored in tools like HashiCorp Vault or similar products.

Additionally, for an open-source product, I agree that it should remain simple enough to use. However, I also believe it needs a certain level of flexibility so that others can extend it based on their specific needs. Within the scope of this PR, I am only providing the ability for others to extend properties with additional data sources.

Furthermore, certain compliance requirements mandate that database passwords be regularly reset. In AWS, AWS Secrets Manager supports the functionality to automatically rotate passwords. You can find more information in the following link: Rotate Amazon RDS Database Credentials Automatically with AWS Secrets Manager.

@terrymanu
Copy link
Member

I believe that security requirements are important, but ShardingSphere is an infrastructure component, and it should not include more things that are not directly related to the ShardingSphere project itself.

Integrating Secrets Manager with ShardingSphere is the responsibility of the business application (or another project that integrates Secrets Manager and ShardingSphere), rather than embedding Secrets Manager into ShardingSphere.

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.

3 participants