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

fix(zitadel): put master key in envvars #556

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

Smana
Copy link
Owner

@Smana Smana commented Nov 13, 2024

PR Type

enhancement, configuration changes


Description

  • Removed the hardcoded masterkey from the Zitadel Helm release configuration to enhance security.
  • Added a comment to indicate that all configuration items are now loaded from a secret, improving the management of sensitive data.
  • Updated the comment for backoffLimit to specify waiting for the CNPG database instance, clarifying the initialization process.

Changes walkthrough 📝

Relevant files
Configuration changes
helmrelease.yaml
Remove hardcoded master key and update configuration comments

security/base/zitadel/helmrelease.yaml

  • Removed hardcoded masterkey from the configuration.
  • Added a comment indicating configuration items are loaded from a
    secret.
  • Updated comment for backoffLimit to reflect CNPG database readiness.
  • +3/-3     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Configuration Change
    Ensure that the removal of the hardcoded masterkey and the addition of environment variables are properly managed and do not disrupt existing deployments. Verify that the new configuration aligns with security best practices.

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Security
    Confirm secure management of sensitive configurations using secrets

    Verify that the removal of the masterkey from the values section and its management
    through envVarsSecret adheres to security best practices, ensuring that sensitive
    data is not exposed in the configuration files.

    security/base/zitadel/helmrelease.yaml [46]

    -envVarsSecret: "zitadel-envvars"
    +envVarsSecret: "zitadel-envvars" # Ensure this secret is managed securely
    Suggestion importance[1-10]: 7

    Why: This suggestion is crucial for ensuring that sensitive data like masterkey is securely managed through secrets. It addresses a significant security practice, although the improved code does not change, the emphasis on verification is valuable.

    7
    Performance
    Adjust the backoffLimit to better match the typical initialization time of the CNPG database

    Ensure that the backoffLimit value is appropriate for the expected initialization
    time of the CNPG database instance. If the database typically initializes faster,
    consider lowering this value to speed up the deployment process.

    security/base/zitadel/helmrelease.yaml [21]

    -backoffLimit: 30 # Wait for the CNPG database instance to be ready
    +backoffLimit: 20 # Adjusted based on typical CNPG database initialization time
    Suggestion importance[1-10]: 5

    Why: The suggestion to adjust the backoffLimit is relevant as it optimizes the deployment process based on typical database initialization times. However, without specific metrics on initialization times, the exact value suggested is somewhat arbitrary.

    5

    @Smana Smana force-pushed the fix_remove_hardcoded_zitadel_secret branch from 1348dcf to f2f9671 Compare November 13, 2024 08:07
    @Smana Smana merged commit 5ddb1ee into main Nov 13, 2024
    3 checks passed
    @Smana Smana deleted the fix_remove_hardcoded_zitadel_secret branch November 13, 2024 08:08
    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.

    2 participants