distributed-system-analysis / pbench

A benchmarking and performance analysis framework
http://distributed-system-analysis.github.io/pbench/
GNU General Public License v3.0
188 stars 108 forks source link

Add invalid algorithm exception check #3399

Closed npalaska closed 1 year ago

npalaska commented 1 year ago

We need to catch an invalid algorithm error when we decode the SSO token and raise it as OpenIDTokenInvalid. This is because we are using HS256 for our internal api key encode-decode.

PBENCH_1136

dbutenhof commented 1 year ago

Intriguing: you have "real" unit test failures related to authentication. Did you run the full server unit test suite before submitting? Is this one of those bizarre differences between local and CI environments? 😦

WARNING pbench.server.api:init.py:1731 Datasets: Unauthenticated client is not authorized to DELETE a resource owned by drb with private access ___ TestDatasetsDetail.test_http_exception[exceptions1] __ [gw25] linux -- Python 3.9.16 /var/tmp/jenkins/tox/py39/bin/python Traceback (most recent call last): File "/var/tmp/jenkins/tox/py39/lib/python3.9/site-packages/pbench/test/unit/server/query_apis/commons.py", line 445, in test_http_exception query_api( File "/var/tmp/jenkins/tox/py39/lib/python3.9/site-packages/pbench/test/unit/server/query_apis/conftest.py", line 67, in query_api response = client_method( File "/var/tmp/jenkins/tox/py39/lib/python3.9/site-packages/responses/init.py", line 929, in exit__ self.stop(allow_assert=success) File "/var/tmp/jenkins/tox/py39/lib/python3.9/site-packages/responses/init.py", line 1126, in stop raise AssertionError( AssertionError: Not all requests have been executed [('GET', 'http://elasticsearch.example.com:7080/unit-test.v6.run-data.2020-08/_search?ignore_unavailable=true')]

npalaska commented 1 year ago

Intriguing: you have "real" unit test failures related to authentication. Did you run the full server unit test suite before submitting? Is this one of those bizarre differences between local and CI environments?

This is very bizarre, I don't see it when I ran locally with pytest before submitting. I am trying to see what's really happening here.

dbutenhof commented 1 year ago

Intriguing: you have "real" unit test failures related to authentication. Did you run the full server unit test suite before submitting? Is this one of those bizarre differences between local and CI environments?

This is very bizarre, I don't see it when I ran locally with pytest before submitting. I am trying to see what's really happening here.

I always do a jenkins/run tox server python before raising a PR; a local pytest is convenient for development but can have a lot of differences from the CI environment, and that can trip you up.

npalaska commented 1 year ago

I always do a jenkins/run tox server python before raising a PR; a local pytest is convenient for development but can have a lot of differences from the CI environment, and that can trip you up.

Yeah, I usually do that but I didn't think that it's gonna break everything. However, what I don't understand is that the unit tests are failing on changes from the main branch as well so I not sure whether the failures here are related to these changes. :fearful:

dbutenhof commented 1 year ago

I always do a jenkins/run tox server python before raising a PR; a local pytest is convenient for development but can have a lot of differences from the CI environment, and that can trip you up.

Yeah, I usually do that but I didn't think that it's gonna break everything. However, what I don't understand is that the unit tests are failing on changes from the main branch as well so I not sure whether the failures here are related to these changes. fearful

Indeed. Riya's cache manager PR has similar failures; and running the test set locally now also fails. I wonder if we had another package upgrade behind our backs? 😦

In fact, something seems to have re-triggered every PR, and they've all failed. Wow.

dbutenhof commented 1 year ago

I just compared packages between a re-run that failed this afternoon and an earlier run that worked. The differences are:

+blinker==1.6.2
-Flask==2.2.3
+Flask==2.3.0
-Werkzeug==2.2.3
+Werkzeug==2.3.0

I'm assuming blinker must be a new dependency of flask, but I didn't investigate; werkzeug is a flask dependency.

So ...

dbutenhof commented 1 year ago

And ... the weird thing is that changing our flask dependency in requirements.txt to Flask==2.2.3 doesn't "take" ... the containerized tests still run with 2.3.0 and fail. I'm assuming those two facts are connected...

dbutenhof commented 1 year ago

Intriguing: there's suddenly a Flask 2.3.1 with the one change "Restore deprecated from flask import Markup. #5084". Hmm...

npalaska commented 1 year ago

Intriguing: there's suddenly a Flask 2.3.1 with the one change "Restore deprecated from flask import Markup. #5084". Hmm...

Yes I was gonna say they just upgraded to 2.3.1, I'll try this version and see if it fixes everything. but... its not available on PyPi I guess.

dbutenhof commented 1 year ago

but... its not available on PyPi I guess.

Yeah, I've been trying ... maybe it'll fix the problem ... and maybe it'll be available soon. Sigh.

webbnh commented 1 year ago

See this comment.

npalaska commented 1 year ago

See this comment.

Okay that explains.

dbutenhof commented 1 year ago

See this comment.

The context makes me fear that 2.3.1 is not a fix for whatever we're hitting. And for some reason I'm having no luck locking Flask to 2.2.x... it's ignoring me and using 2.3.0 anyway.