fix(k8s): fix error ordering in ProcessEnvs and late validation#3807
fix(k8s): fix error ordering in ProcessEnvs and late validation#3807Elvand-Lie wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Elvand-Lie The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
knative-prow
Bot
added
size/M
|
Hi @Elvand-Lie. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3807 +/- ##
==========================================
+ Coverage 53.42% 53.48% +0.06%
==========================================
Files 200 200
Lines 23426 23418 -8
==========================================
+ Hits 12515 12526 +11
+ Misses 9655 9640 -15
+ Partials 1256 1252 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ProcessEnvs appends the result of createEnvVarSource to the envVars slice before checking the error. Reordered so the error check happens first, following standard Go error handling. createEnvFromSource and createEnvVarSource both validate that sourceName is non-empty only after using it to construct resource references and inserting it into the referenced sets. Moved the empty-name validation before the switch statements. Fixes knative#3798
Elvand-Lie
force-pushed
the
fix/processenvs-error-ordering
branch
from
66defd6 to
015a7da
Compare
lkingland
added
ok-to-test
2 participants
Problem
ProcessEnvs error ordering:
ProcessEnvs appends the result of createEnvVarSource to the envVars slice before checking the returned error. This violates standard Go error-handling pattern.
createEnvFromSource/createEnvVarSource late validation:
Both functions validate that sourceName is non-empty only after using it to construct resource references and inserting it into the referenced sets.
Fix
Reordered ProcessEnvs so the error check comes first. Moved the empty-name validation before the switch statements in createEnvFromSource and createEnvVarSource so the sets are not polluted with empty strings.
Tests added
Fixes #3798