Prevent bad link in opstream from crashing sidecar #956
Conversation
* Bump deps (#950) Co-authored-by: GitHub Actions Bot <[email protected]> * 2.2.4 changelog --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: GitHub Actions Bot <[email protected]> Co-authored-by: rafapaezbas <[email protected]>
Co-authored-by: rafapaezbas <[email protected]>
* Bump deps (#954) Co-authored-by: GitHub Actions Bot <[email protected]> * 2.2.6 changelog --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: GitHub Actions Bot <[email protected]> Co-authored-by: rafapaezbas <[email protected]>
Didericis
changed the title
Didericis
force-pushed
the
test/fix-malformed-args-error
branch
8 times, most recently
from
2029981 to
5b99fac
Compare
Didericis
changed the title
Didericis
force-pushed
the
test/fix-malformed-args-error
branch
3 times, most recently
from
4f8af77 to
58dce4f
Compare
test/15-opstream.test.js
Outdated
| const Helper = require('./helper') | ||
|
|
||
| test('running ops with invalid link param does not crash the sidecar', async (t) => { | ||
| const opMethods = [ |
There was a problem hiding this comment.
Ideally this would be pointing to a shared location so when ops get added they get tested by this.
I tried moving the code here into subsystems/sidecar/ops/index.js and then referencing that in this test, but that caused tests to be flakey/introduced a timing error.
Decided to just keep this a simple list despite the brittleness (as long as at least one op is tested via this the test is still valuable).
There was a problem hiding this comment.
nice. does it also pass if I dont pass an obj? eg: stage('str') , info(int) ....
There was a problem hiding this comment.
check if all cases are covered bc there is a conditional here
There was a problem hiding this comment.
nice. does it also pass if I dont pass an obj? eg: stage('str') , info(int) ....
Will test those cases as well, but yeah, works as expected/fails if it's invalid. Will add more coverage.
There was a problem hiding this comment.
@geordangesink added some more cases, but RE that line specifically, one thing to note is that the normalize function doesn't actually decode the link or do the full link validation.
I thought about doing full link validation when the param is present, but think it's best to have that handled in specific ops where the link is used. It looks like some ops like gc don't actually use the link param, so parsing a param that gets discarded would be wasteful. But all the major types that could be passed are explicitly covered now.
Didericis
force-pushed
the
test/fix-malformed-args-error
branch
4 times, most recently
from
25dcf87 to
d271814
Compare
Didericis
force-pushed
the
test/fix-malformed-args-error
branch
from
d271814 to
1c10226
Compare
davidmarkclements
force-pushed
the
main
branch
from
38a8e8c to
b9fe9ce
Compare
Didericis
force-pushed
the
test/fix-malformed-args-error
branch
from
c479b59 to
afc942d
Compare
davidmarkclements
force-pushed
the
main
branch
from
e50a4a9 to
d8fe8e7
Compare
Depends on this pear-link pr