Skip to content

MM-67050: Add certificate mounting and KUBECONFIG support for self-signed cert environments#75

Open
bgardner8008 wants to merge 8 commits intomasterfrom
MM-67050-self-signed-cert
Open

MM-67050: Add certificate mounting and KUBECONFIG support for self-signed cert environments#75
bgardner8008 wants to merge 8 commits intomasterfrom
MM-67050-self-signed-cert

Conversation

@bgardner8008
Copy link
Copy Markdown
Contributor

Summary

  • Add Kubernetes certificate volume mounting via JOBS_K8S_CERT_CONFIGMAP or JOBS_K8S_CERT_SECRET environment variables, mounting certs read-only at /certs in job pods
  • Enforce mutual exclusivity between ConfigMap and Secret cert options
  • Add KUBECONFIG env var support for local development (falls back to in-cluster config)
  • Add JOBS_DOCKER_CERT_PATH for bind-mounting host cert directory into Docker job containers
  • Expand seccomp security profile with additional syscalls (openat2, close_range, epoll_pwait2, etc.)
  • Add debug logging for job creation with jobID
  • Refactor volume/mount construction into getVolumesAndMounts() helper with unit tests

Ticket Link

https://mattermost.atlassian.net/browse/MM-67050

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bffab7ca-b8a2-491f-a1be-f839f419903b

📥 Commits

Reviewing files that changed from the base of the PR and between 066a496 and be84011.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • go.mod
  • service/docker/service.go
✅ Files skipped from review due to trivial changes (2)
  • go.mod
  • service/docker/service.go

📝 Walkthrough

Walkthrough

The changes extend job service containerization by adding optional certificate path mounting for both Docker and Kubernetes backends, refactoring Kubernetes volume configuration into a reusable helper function with conditional certificate source handling, improving kubeconfig initialization with fallback to in-cluster configuration, and adding corresponding test coverage. Two indirect Go dependencies are introduced.

Changes

Cohort / File(s) Summary
Dependency Management
go.mod
Added two indirect dependencies: github.com/imdario/mergo v0.3.12 and github.com/spf13/pflag v1.0.5.
Docker Service Enhancements
service/docker/service.go
Modified CreateJob to build a mounts slice conditionally appending a read-only bind mount to /certs when JOBS_DOCKER_CERT_PATH is set, and changed security options assignment to directly pass []string{dockerSecurityOpts}.
Kubernetes Service Updates
service/kubernetes/service.go
Updated kubeconfig initialization with fallback logic from kubeconfig file to in-cluster configuration; added debug logging for job creation (recording/transcription) and config source selection; refactored job storage volume creation to use new getVolumesAndMounts helper.
Kubernetes Utilities
service/kubernetes/utils.go, service/kubernetes/utils_test.go
Introduced getVolumesAndMounts helper function that constructs Kubernetes pod volumes and mounts with conditional certificate source configuration (ConfigMap or Secret) and PVC support; added comprehensive test coverage validating default behaviour, PVC configuration, certificate sources, combinations, and mutual exclusivity validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main changes: adding certificate mounting and KUBECONFIG support for Kubernetes self-signed certificate environments, which aligns with the primary objectives of the pull request.
Description check ✅ Passed The description is well-organized and directly related to the changeset, covering all major modifications including certificate mounting, KUBECONFIG support, Docker cert path, seccomp profile expansion, logging additions, and the new helper function refactoring.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch MM-67050-self-signed-cert

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

Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
service/kubernetes/utils.go (1)

193-229: Consider adding certificate env vars to JobServiceConfig for consistency.

The certificate environment variables (JOBS_K8S_CERT_CONFIGMAP, JOBS_K8S_CERT_SECRET) are read inline via os.Getenv, whereas similar settings like PersistentVolumeClaimName are part of JobServiceConfig (see service/kubernetes/service.go:54-61). Consider adding these to the config struct for consistency with the existing pattern.

That said, the current implementation is functional and the read-only mount for certificates is correctly configured.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@service/kubernetes/utils.go` around lines 193 - 229, This code reads
certificate settings via os.Getenv inline (certConfigMap, certSecret); add two
fields to JobServiceConfig (e.g., CertConfigMap and CertSecret) in the
JobServiceConfig struct and populate them when building the config (same place
PersistentVolumeClaimName is populated), then update the logic in utils.go to
use config.CertConfigMap and config.CertSecret (instead of
certConfigMap/certSecret from os.Getenv) when creating the "certs" Volume and
VolumeMount; ensure names (CertConfigMap, CertSecret) match the new struct
fields and update any construction/usage sites accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@service/kubernetes/utils.go`:
- Around line 193-229: This code reads certificate settings via os.Getenv inline
(certConfigMap, certSecret); add two fields to JobServiceConfig (e.g.,
CertConfigMap and CertSecret) in the JobServiceConfig struct and populate them
when building the config (same place PersistentVolumeClaimName is populated),
then update the logic in utils.go to use config.CertConfigMap and
config.CertSecret (instead of certConfigMap/certSecret from os.Getenv) when
creating the "certs" Volume and VolumeMount; ensure names (CertConfigMap,
CertSecret) match the new struct fields and update any construction/usage sites
accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 89fa64e9-eaef-479b-b206-25f163c2c5bb

📥 Commits

Reviewing files that changed from the base of the PR and between 9ad35ec and 066a496.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (6)
  • go.mod
  • service/docker/security_opts.go
  • service/docker/service.go
  • service/kubernetes/service.go
  • service/kubernetes/utils.go
  • service/kubernetes/utils_test.go

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