Skip to content

[deploy] Fix helm chart values#4188

Merged
imnasnainaec merged 1 commit intomasterfrom
bugfix/helm-chart-values
Mar 4, 2026
Merged

[deploy] Fix helm chart values#4188
imnasnainaec merged 1 commit intomasterfrom
bugfix/helm-chart-values

Conversation

@imnasnainaec
Copy link
Collaborator

@imnasnainaec imnasnainaec commented Mar 3, 2026

Backend is broken in new deployments with Rancher. These didn't cause a problem in existing QA and production deployments because the correct environment variables were either grandfathered in or added manually.

3 backend values were being looked for in global, but were defined outside global in https://github.com/sillsdev/TheCombine/blob/master/deploy/helm/thecombine/charts/backend/values.yaml.


This change is Reviewable

Summary by CodeRabbit

  • Chores
    • Streamlined backend configuration value references for email verification, password reset, and project invite expiry settings.
    • Enhanced AWS S3 credentials naming to support dynamic environment-specific configuration.

@imnasnainaec imnasnainaec self-assigned this Mar 3, 2026
@imnasnainaec imnasnainaec added bug Something isn't working 🟨Medium Medium-priority PR labels Mar 3, 2026
@imnasnainaec imnasnainaec changed the title Fix helm chart values [deploy] Fix helm chart values Mar 3, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

📝 Walkthrough

Walkthrough

Updates Helm chart configurations by changing ConfigMap environment variables to reference top-level values instead of global values and converting a static Secret resource name to a dynamic templated reference.

Changes

Cohort / File(s) Summary
Backend ConfigMap
deploy/helm/thecombine/charts/backend/templates/backend-config-map.yaml
Refactored three environment variables (COMBINE_EXPIRE_EMAIL_VERIFY_MINUTES, COMBINE_EXPIRE_PASSWORD_RESET_MINUTES, COMBINE_EXPIRE_PROJECT_INVITE_DAYS) from block-style multi-line references to global.* values into single-line inline references to top-level .Values.* paths; removed the | lower filter.
Maintenance S3 Credentials
deploy/helm/thecombine/charts/maintenance/templates/aws-s3-credentials.yaml
Changed Secret metadata name from static value aws-s3-credentials to dynamic template reference {{ .Values.global.awsS3Access | quote }}.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Values hop from global heights to the top,
Helm charts grow more dynamic, we simply won't stop,
Static names dance into templates so grand,
Configuration migration across the land! 📋✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title '[deploy] Fix helm chart values' accurately describes the main change: fixing helm chart values in deployment configurations. It is clear, specific, and directly relates to the primary focus of the changeset.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/helm-chart-values

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.

❤️ Share

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

@codecov
Copy link

codecov bot commented Mar 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.15%. Comparing base (0ce9ad4) to head (0bf8d00).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4188   +/-   ##
=======================================
  Coverage   75.15%   75.15%           
=======================================
  Files         302      302           
  Lines       11099    11099           
  Branches     1394     1394           
=======================================
  Hits         8341     8341           
  Misses       2357     2357           
  Partials      401      401           
Flag Coverage Δ
backend 86.52% <ø> (ø)
frontend 66.34% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

@jasonleenaylor reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).

@imnasnainaec imnasnainaec merged commit 1033e74 into master Mar 4, 2026
23 checks passed
@imnasnainaec imnasnainaec deleted the bugfix/helm-chart-values branch March 4, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working deployment 🟨Medium Medium-priority PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants