-
Notifications
You must be signed in to change notification settings - Fork 3
Generate certificates with easy-rsa #111
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request updates multiple aspects of the project. In the CI workflows, it introduces a matrix strategy for both end-to-end tests and deployment jobs, allowing parallel runs on two Ubuntu OS variants. The certificate logic throughout the code is simplified by replacing dynamic self-signed generation with static loading from a mounted volume, and helm chart templates are modified to centralize endpoint configuration and enhance TLS support. Additional changes in the secrets job update the image, command structure, and TTL settings, while several helper functions for certificate creation are removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Matrix as deploy-kind-matrix Job
participant CI as GitHub Actions
participant Deploy as deploy-kind Job
Matrix->>CI: Run on ubuntu-24.04 and ubuntu-24.04-arm concurrently
CI->>Deploy: Signal matrix job completion with status
Deploy->>CI: Execute steps and exit if any status indicates failure
sequenceDiagram
participant Service as Controller/Router Service
participant FS as File System (/etc/jumpstarter/tls)
Service->>FS: Call LoadCertificate()
FS-->>Service: Return TLS certificate (or error if loading fails)
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
8e204c4
to
7e93ae8
Compare
There was a problem hiding this 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 (3)
.github/workflows/e2e.yaml (1)
10-15
: Matrix Strategy Setup and Runner Label Caution
The matrix strategy is correctly implemented for the end-to-end tests, which will now run on bothubuntu-24.04
andubuntu-24.04-arm
. However, please verify that the runner label"ubuntu-24.04-arm"
is valid in your environment. According to common runner labels, this value may be unrecognized unless it is a custom label configured via theactionlint.yaml
file.🧰 Tools
🪛 actionlint (1.7.4)
14-14: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/pr-kind.yaml (1)
8-14
: Deploy-Kind-Matrix Job and Runner Label Warning
The introduction of thedeploy-kind-matrix
job with a matrix strategy is well implemented, enabling parallel deployment tests on different OS configurations. However, similar to the e2e workflow, the use of"ubuntu-24.04-arm"
as a runner label could trigger issues if it is not recognized by GitHub’s standard labels. If this is intended as a custom label for self-hosted runners, ensure that your configuration inactionlint.yaml
is updated accordingly.🧰 Tools
🪛 actionlint (1.7.4)
13-13: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yaml (1)
19-19
: Consider using a specific version tag instead of 'latest'Using the
latest
tag for production deployments can lead to unexpected behavior when the image is updated, as it makes builds non-deterministic and less reproducible.- image: quay.io/jumpstarter-dev/jumpstarter-utils:latest + image: quay.io/jumpstarter-dev/jumpstarter-utils:<specific-version>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/e2e.yaml
(1 hunks).github/workflows/pr-kind.yaml
(1 hunks)cmd/main.go
(1 hunks)deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/_endpoints.tpl
(1 hunks)deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-deployment.yaml
(2 hunks)deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yaml
(1 hunks)internal/service/controller_service.go
(1 hunks)internal/service/endpoints.go
(0 hunks)internal/service/router_service.go
(1 hunks)internal/service/selfsigned.go
(1 hunks)
💤 Files with no reviewable changes (1)
- internal/service/endpoints.go
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/e2e.yaml
14-14: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/pr-kind.yaml
13-13: label "ubuntu-24.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: deploy-kind-matrix (ubuntu-24.04-arm)
- GitHub Check: e2e-tests (ubuntu-24.04-arm)
🔇 Additional comments (12)
.github/workflows/e2e.yaml (1)
17-21
: Job Steps and Action Usage Reviewed
The subsequent steps that invokejumpstarter-dev/jumpstarter-e2e@main
with appropriate inputs are correctly configured. No issues detected in these lines..github/workflows/pr-kind.yaml (1)
22-35
: Deploy-Kind Job: Conditional Failure Trigger
Thedeploy-kind
job that depends ondeploy-kind-matrix
is structured to always run and then conditionally exit with a failure if any matrix job returns"failure"
,"cancelled"
, or"skipped"
. This conditional handling is an effective way to catch and signal issues in the deployment matrix. Ensure that the exit behavior (usingexit 1
) is consistent with your downstream failure handling processes.cmd/main.go (1)
137-137
: Improved certificate handling for the internal OIDC providerThe change to use
NewSelfSignedLocalhostCertificate()
instead of the more generic function streamlines the certificate generation process by using appropriate defaults for localhost, which is suitable for an internal OIDC provider.internal/service/controller_service.go (1)
702-702
: Excellent architectural improvement for certificate managementLoading certificates from a mounted secret rather than generating them dynamically is a significant improvement that:
- Decouples certificate management from application code
- Enables easier certificate rotation
- Allows for properly managed certificates rather than self-signed ones
- Simplifies the codebase by removing complex logic for certificate generation
This change brings the application more in line with Kubernetes best practices for managing sensitive data.
internal/service/router_service.go (1)
115-115
: Consistent certificate management approach across servicesThis change follows the same pattern implemented in the controller service, loading certificates from a mounted secret rather than generating them dynamically. The consistent approach across services simplifies operational management and provides the same benefits of decoupled certificate management, easier rotation, and code simplification.
deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-deployment.yaml (2)
51-54
: Well-implemented TLS certificate mountingThe addition of a TLS secret volume and its corresponding mount path provides the necessary infrastructure for the refactored certificate loading approach. Setting the mount as read-only is a good security practice for sensitive certificate data. This change coordinates perfectly with the code modifications in both service files.
Also applies to: 79-82
62-64
: Improved endpoint configurationReplacing inline endpoint configuration with template functions centralizes this logic and makes it more maintainable. This change helps ensure consistent endpoint configuration across the deployment.
deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/_endpoints.tpl (1)
1-2
: Well-structured endpoint templating with proper fallbacksThese template definitions provide a clear hierarchy of configuration sources with appropriate fallbacks. The approach centralizes endpoint logic and offers flexibility through multiple configuration options while providing clear error messaging when required values are missing.
deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yaml (2)
15-15
: Good addition of TTL for job cleanupSetting
ttlSecondsAfterFinished: 600
ensures the job will be automatically cleaned up 10 minutes after completion, preventing accumulation of completed jobs in the cluster.
25-47
: Improved certificate generation with EasyRSAThe certificate generation approach using EasyRSA is more robust and standardized than a custom implementation. Good practices include:
- Extracting hostnames dynamically from template endpoints
- Properly configuring Subject Alternative Names for both hostnames
- Creating separate secrets for CA and server certificates
- Checking if secrets already exist to avoid unnecessary updates
internal/service/selfsigned.go (2)
15-29
: Good separation of concerns for certificate handlingThe
LoadCertificate
function follows the single responsibility principle by focusing solely on loading certificates from files. This approach allows certificates to be managed externally (via EasyRSA in the secrets job) rather than generating them dynamically in the application code.
31-40
: Simplified localhost certificate generationThe function has been appropriately simplified to focus solely on localhost development scenarios with hardcoded values, making its purpose clearer.
Summary by CodeRabbit
New Features
Refactor