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(cloud-native): secure mounted configuration schema #10577

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

Conversation

iromli
Copy link
Contributor

@iromli iromli commented Jan 8, 2025

Prepare


Description

Target issue

closes #10550

Implementation Details


Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

Copy link

sonarqubecloud bot commented Jan 8, 2025

Copy link

sonarqubecloud bot commented Jan 8, 2025

Copy link

sonarqubecloud bot commented Jan 8, 2025

Copy link

sonarqubecloud bot commented Jan 8, 2025

Copy link

sonarqubecloud bot commented Jan 8, 2025

Copy link

sonarqubecloud bot commented Jan 8, 2025

Copy link

sonarqubecloud bot commented Jan 8, 2025

@mo-auto mo-auto added area-documentation Documentation needs to change as part of issue or PR comp-docker-jans-all-in-one Touching folder /docker-jans-all-in-one labels Jan 8, 2025
Copy link
Member

@moabu moabu left a comment

Choose a reason for hiding this comment

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

Key properties should be specified. We should indicate that only symmetric is supported and show what sizes are supported to limit possible errors and issues. Also we need to handle the case when an error occurs handling the key and act as if its empty when that happens skipping encryption/decryption.

@iromli
Copy link
Contributor Author

iromli commented Jan 10, 2025

Hi @moabu , thanks for the review. I made several changes to address potential issue.

Key properties should be specified. We should indicate that only symmetric is supported and show what sizes are supported to limit possible errors and issues.

The new cnConfiguratorKey attribute is now specified in docs and validated in values.schema.json.

Also we need to handle the case when an error occurs handling the key and act as if its empty when that happens skipping encryption/decryption.

Invalid key or invalid text is handled by jans-pycloudlib, for example: https://github.com/JanssenProject/jans/blob/main/jans-pycloudlib/jans/pycloudlib/secret/file_secret.py#L17-L19. The OCI image entrypoint will catch the errors and treat them as warning message instead so the pod will still running regardless of failed decryption process.

Though there's an exception, the configurator job will throw error and kill the pod to ensure only valid configuration schema is mounted to pod (useful on first installation).

@iromli iromli marked this pull request as ready for review January 10, 2025 17:49
@iromli iromli requested a review from misba7 January 10, 2025 17:50
@mo-auto mo-auto added the comp-docs Touching folder /docs label Jan 10, 2025
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.

feat(cloud-native): secure mounted configuration schema
3 participants