Repository navigation
fix(k8s): fix error ordering in ProcessEnvs and late validation - #3807
Elvand-Lie wants to merge 1 commit into
Conversation
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 54.00% 54.06% +0.05%
==========================================
Files 200 200
Lines 23687 23679 -8
==========================================
+ Hits 12792 12801 +9
+ Misses 9660 9647 -13
+ Partials 1235 1231 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
66defd6 to
015a7da
Compare
lkingland
added
ok-to-test
|
/approve |
knative-prow
Bot
added
lgtm
|
there is conflict @Elvand-Lie |
knative-prow-robot
added
the
needs-rebase
015a7da to
f523d59
Compare
|
New changes are detected. LGTM label has been removed. |
knative-prow-robot
removed
the
needs-rebase
|
Rebased onto main. The conflict was from the ProcessVolumes nil-path tests that were merged in the meantime. Kept both test sets in. @matejvasek |
f523d59 to
a3ae155
Compare
knative-prow-robot
added
the
needs-rebase
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 Signed-off-by: Elvand Lie Nababan <elvandlie@gmail.com>
a3ae155 to
13e66f6
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Elvand-Lie, matejvasek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
removed
the
needs-rebase
|
Rebased onto main. The only conflict was tests appended at the end of |
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