Skip to content

Conversation

mehdimas
Copy link
Contributor

@mehdimas mehdimas commented Jul 29, 2025

This pull request enhances the Kratos Helm chart, specifically focusing on the cleanup cronjob.

  1. Support for Extra Containers: Added the ability to define extra containers for the Kratos cleanup cronjob, allowing users to specify sidecar containers such as a cloud SQL proxy for database connections.

  2. Service Account Configuration:

    • Introduced support for automountServiceAccountToken, shareProcessNamespace, and a dedicated service account for the cleanup cronjob.
    • Updated the cronjob-cleanup.yaml to include these configurations, ensuring the cronjob can operate with specific permissions and namespace sharing as needed.
  3. Values File: Added new fields under cronjob.cleanup in values.yaml to configure automountServiceAccountToken, shareProcessNamespace, and service account settings.

  4. Helper Function Update: Updated _helpers.tpl to include a new helper function kratos.cleanup.serviceAccountName for generating the service account name for the cleanup cronjob.

  5. RBAC Configuration: Created a new rbac-cleanup.yaml file to define a ServiceAccount for the cleanup cronjob, similar to the existing job configuration.

These changes expand the flexibility of the Kratos cleanup cronjob, allowing for more customized deployments.

Related Issue or Design Document

This PR comprises of simple changes to existing templates and the Kratos values.yaml file following the same pattern used for other jobs (e.g. the migration job).

Checklist

  • I have read the contributing guidelines and signed the CLA.
  • I have referenced an issue containing the design document if my change introduces a new feature.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability.
    If this pull request addresses a security vulnerability,
    I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation within the code base (if appropriate).

Further comments

Copy link
Collaborator

@Demonsthere Demonsthere left a comment

Choose a reason for hiding this comment

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

Hi there!
All of those features sound great, and I think they would be an overall benefit to the system and not only kratos :) Could you expand the features to hydra as well?

Additionally:

  • please run make helm-docs to update the readme after changes to the values.yaml
  • please add a new test case - values.yaml to the hacks/values directory with examples of the new fields, like extraContainres for the cronjob. This allows us to rener the manifests and verify them in the CI :)

@mehdimas mehdimas force-pushed the feat/cleanup-cron-extra-containers branch 6 times, most recently from 164573f to 55691b7 Compare August 11, 2025 19:49
# image: ...

# -- Set automounting of the SA token
automountServiceAccountToken: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Matched the default value for Hydra's janitor cron job.

@mehdimas
Copy link
Contributor Author

@Demonsthere and @piotrmsc I've addressed the feedback:

  • Updated the README documentation
  • Added values to hacks/values to test the changes

The Hydra Janitor cron job already supports extra containers and extra init containers, but I added test values to hacks/values to confirm the behavior. I also added support for the shareProcessNamespace field to keep this consistent with the functionality I added to the Kratos cleanup cron job.

In addition, I added support for extra init containers to the Kratos cleanup cron job because they were supported for Hydra Janitor.

@mehdimas mehdimas force-pushed the feat/cleanup-cron-extra-containers branch from 55691b7 to 8e5e1cc Compare August 12, 2025 13:42
@mehdimas mehdimas requested a review from Demonsthere August 12, 2025 13:43
Copy link
Collaborator

@Demonsthere Demonsthere left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@Demonsthere Demonsthere merged commit 8e8d1a1 into ory:master Aug 14, 2025
15 checks passed
@mehdimas mehdimas deleted the feat/cleanup-cron-extra-containers branch August 14, 2025 15:28
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