bugfix/3364/add-annotations-to-webapp-sa#3367
bugfix/3364/add-annotations-to-webapp-sa#3367LeeheAlkalyDarrow wants to merge 1 commit intotriggerdotdev:mainfrom
Conversation
|
|
Hi @LeeheAlkalyDarrow, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis pull request adds a new Helm helper template Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
| Create the name of the webapp service account to use | ||
| */}} | ||
| {{- define "trigger-v4.webappServiceAccountName" -}} | ||
| {{- if .Values.supervisor.serviceAccount.create }} |
There was a problem hiding this comment.
🔴 Copy-paste error: webappServiceAccountName checks supervisor.serviceAccount.create instead of webapp.serviceAccount.create
The new trigger-v4.webappServiceAccountName helper at hosting/k8s/helm/templates/_helpers.tpl:566 checks .Values.supervisor.serviceAccount.create instead of .Values.webapp.serviceAccount.create. This was clearly copied from the supervisor helper at hosting/k8s/helm/templates/_helpers.tpl:526-532 without updating the condition.
With default values, supervisor.serviceAccount.create is true, so this helper always returns the auto-generated name <fullname>-webapp. However, webapp.yaml:1 correctly checks .Values.webapp.serviceAccount.create (which has no default in values.yaml and evaluates to nil/false), so the ServiceAccount resource is not created. This results in the Deployment (line 65) and RoleBinding (line 36) referencing a ServiceAccount that doesn't exist, causing pod scheduling failures.
| {{- if .Values.supervisor.serviceAccount.create }} | |
| {{- if .Values.webapp.serviceAccount.create }} |
Was this helpful? React with 👍 or 👎 to provide feedback.
Closes #
✅ Checklist
Testing
[Describe the steps you took to test this change]
Changelog
[Short description of what has changed]
Screenshots
[Screenshots]
💯