MM-67050: Add certificate mounting and KUBECONFIG support for self-signed cert environments#75
MM-67050: Add certificate mounting and KUBECONFIG support for self-signed cert environments#75bgardner8008 wants to merge 8 commits intomasterfrom
Conversation
…to support CA file using K8S
…feature. Calls must connect to offloader using http or https with known cert
…utual exclusivity
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThe 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 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 viaos.Getenv, whereas similar settings likePersistentVolumeClaimNameare part ofJobServiceConfig(seeservice/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
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
go.modservice/docker/security_opts.goservice/docker/service.goservice/kubernetes/service.goservice/kubernetes/utils.goservice/kubernetes/utils_test.go
Summary
JOBS_K8S_CERT_CONFIGMAPorJOBS_K8S_CERT_SECRETenvironment variables, mounting certs read-only at/certsin job podsKUBECONFIGenv var support for local development (falls back to in-cluster config)JOBS_DOCKER_CERT_PATHfor bind-mounting host cert directory into Docker job containersgetVolumesAndMounts()helper with unit testsTicket Link
https://mattermost.atlassian.net/browse/MM-67050