Skip to content

Conversation

@emsearcy
Copy link
Contributor

Summary

This PR adds a configurable algorithm option for heimdall key generation that defaults to "rsa" instead of hardcoded behavior.

Changes Made

  • Fixed configuration inconsistency: Changed key_type to algorithm in values.yaml to match what the template was already expecting
  • Default algorithm: The algorithm now defaults to "rsa" (not ecdsa) as specified
  • Updated documentation: Modified the template comment to reflect the configurable algorithm option instead of hardcoded P-256 curve reference
  • Improved consistency: Resolved mismatch between values.yaml field name and template reference

Configuration

Users can now configure the private key algorithm by setting:

lfx:
  generateHeimdallSignerCert:
    algorithm: rsa  # or ecdsa, ed25519, etc.

The algorithm defaults to "rsa" if not specified.

Testing

  • Verified configuration field names match between values.yaml and template
  • Confirmed default value is "rsa" as requested
  • Updated comments to reflect new configurable behavior

🤖 Generated with GitHub Copilot (via Zed)

- Change key_type to algorithm in values.yaml to match template expectations
- Default algorithm is now 'rsa' instead of hardcoded configuration
- Update template comment to reflect configurable algorithm option
- Fix inconsistency between values.yaml field name and template reference

🤖 Generated with [GitHub Copilot](https://github.com/features/copilot) (via Zed)

Signed-off-by: Eric Searcy <[email protected]>
@emsearcy emsearcy requested a review from a team as a code owner December 12, 2025 22:44
Copilot AI review requested due to automatic review settings December 12, 2025 22:44
@coderabbitai
Copy link

coderabbitai bot commented Dec 12, 2025

Walkthrough

The pull request makes the Heimdall signer certificate key generation algorithm configurable. A new configuration parameter is introduced in values.yaml with a default value of "rsa", and the Heimdall signer certificate template is updated to use this configurable algorithm instead of a hardcoded "ecdsa" value.

Changes

Cohort / File(s) Summary
Heimdall certificate configuration
charts/lfx-platform/templates/heimdall/heimdall-signer-cert.yaml, charts/lfx-platform/values.yaml
Updated certificate template to use configurable algorithm from .Values.lfx.generateHeimdallSignerCert.algorithm instead of hardcoded "ecdsa"; added corresponding configuration parameter with default value "rsa" to values file; clarified template comment.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify the parameter path in the template matches the values file structure correctly
  • Ensure the algorithm value change from "ecdsa" to "rsa" is intentional and compatible with dependent systems

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: making the heimdall key generation algorithm configurable, which aligns with the core modifications in both the template and values.yaml files.
Description check ✅ Passed The description is directly related to the changeset, providing clear details about the configuration changes, the field rename from key_type to algorithm, the default value of rsa, and updated documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the Helm chart by making the private key generation algorithm configurable for the Heimdall signer certificate. Previously, the algorithm was hardcoded to "ecdsa" in the template. Now users can specify their preferred algorithm through the values.yaml configuration file, with "rsa" as the default.

Key Changes:

  • Added configurable algorithm parameter to lfx.generateHeimdallSignerCert in values.yaml with "rsa" as the default
  • Updated the template to use the configurable algorithm value instead of hardcoded "ecdsa"
  • Updated template comment to reflect the generic nature of the private key generation (removed specific P-256 curve reference)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
charts/lfx-platform/values.yaml Added algorithm: rsa configuration field with explanatory comment under lfx.generateHeimdallSignerCert
charts/lfx-platform/templates/heimdall/heimdall-signer-cert.yaml Updated genPrivateKey call to use configurable algorithm parameter and generalized the comment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
charts/lfx-platform/values.yaml (1)

24-25: Document supported algorithm values in the configuration comment.

The Sprig genPrivateKey function accepts rsa, dsa, and ecdsa, but the comment and PR description mention ed25519 as an option. If no argument is supplied, it defaults to RSA. Consider updating the comment to clarify which algorithms are supported to prevent configuration errors from invalid values.

  generateHeimdallSignerCert:
    enabled: true
    name: heimdall-signer-cert
-   # algorithm will be passed as the parameter to Sprig's genPrivateKey.
-   algorithm: rsa
+   # algorithm will be passed as the parameter to Sprig's genPrivateKey.
+   # Supported values: rsa, dsa, ecdsa (defaults to rsa if not specified)
+   algorithm: rsa
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between efc0a29 and 213ded9.

📒 Files selected for processing (2)
  • charts/lfx-platform/templates/heimdall/heimdall-signer-cert.yaml (1 hunks)
  • charts/lfx-platform/values.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Upload results
  • GitHub Check: MegaLinter
🔇 Additional comments (1)
charts/lfx-platform/templates/heimdall/heimdall-signer-cert.yaml (1)

6-9: LGTM!

The template has been properly updated to use the configurable algorithm from values. The comment update from "P-256 curve key" to "private key" is appropriate, and the Helm template syntax correctly accesses .Values.lfx.generateHeimdallSignerCert.algorithm.

@emsearcy emsearcy merged commit 16a9039 into linuxfoundation:main Dec 12, 2025
10 checks passed
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.

4 participants