Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mgr/dashboard: Updating the inbuilt ssl providers error #38484

Merged
merged 2 commits into from Dec 9, 2020
Merged

mgr/dashboard: Updating the inbuilt ssl providers error #38484

merged 2 commits into from Dec 9, 2020

Conversation

nizamial09
Copy link
Member

@nizamial09 nizamial09 commented Dec 8, 2020 •
edited

Updating the inbuilt ssl providers error according to the upstream changes.
https://github.com/cherrypy/cheroot/blob/master/cheroot/ssl/builtin.py#L303

Fixes: https://tracker.ceph.com/issues/48490
Signed-off-by: Nizamudeen A nia@redhat.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@nizamial09 nizamial09 requested a review from a team as a code owner December 8, 2020 10:24
@nizamial09 nizamial09 requested review from Exotelis and removed request for a team December 8, 2020 10:24
@nizamial09 nizamial09 added this to In progress in Dashboard via automation Dec 8, 2020
@nizamial09 nizamial09 added bug-fix and removed pybind labels Dec 8, 2020
Dashboard automation moved this from In progress to Reviewer approved Dec 8, 2020
votdev
votdev approved these changes Dec 8, 2020
View reviewed changes
Copy link
Member

@votdev votdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except my comment.

src/pybind/mgr/dashboard/cherrypy_backports.py Outdated
_block_errors = ('unknown protocol', 'unknown error',
'unknown ca', 'unknown_ca',
'inappropriate fallback', 'https proxy request',
'wrong version number', 'bad_certificate',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does bad_certificate come from? I can't find it in https://github.com/cherrypy/cheroot/blob/master/cheroot/ssl/builtin.py.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it there, i don't know if its a valid one but I am seeing bad certificate error in downstream. So I thought this will fix that issue too. @votdev

Copy link
Contributor

@tchaikov tchaikov Dec 8, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nizamial09 could you reorder these errors to align with _block_errors in upstream? if you believe that 'bad_certificate' should be added as well, could you please create a dedicated commit for adding it, and make sure it is reproducible and upstream it to https://github.com/cherrypy/cheroot ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I'll check if its reproducible in upstream and create a PR.

Copy link
Member Author

@nizamial09 nizamial09 Dec 8, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproducible in upstream. The error was actually sslv3 alert bad certificate. Updated this PR.

ssl.SSLError: [SSL: SSLV3_ALERT_BAD_CERTIFICATE] sslv3 alert bad certificate (_ssl.c:897)

@github-actions github-actions bot added the pybind label Dec 8, 2020
@nizamial09
mgr/dashboard: Adding the alert bad certificate error to the ssl prov…
4cbe89f 
…iders error

upstream tracked in cherrypy/cheroot#348
Fixes: https://tracker.ceph.com/issues/48490
Signed-off-by: Nizamudeen A <nia@redhat.com>
callithea
callithea approved these changes Dec 8, 2020
View reviewed changes
Copy link
Member

@callithea callithea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@nizamial09
Copy link
Member Author

jenkins test make check

@nizamial09
Copy link
Member Author

jenkins test dashboard

@nizamial09
Copy link
Member Author

jenkins test make check

@epuertat epuertat moved this from Reviewer approved to Ready-to-merge in Dashboard Dec 9, 2020
@epuertat epuertat merged commit 3629e81 into ceph:master Dec 9, 2020
Dashboard automation moved this from Ready-to-merge to Done Dec 9, 2020
@epuertat epuertat deleted the update-ssl-error-1 branch December 9, 2020 16:25
This was referenced Dec 9, 2020
Merged
Merged
@epuertat epuertat mentioned this pull request Jan 7, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tchaikov tchaikov tchaikov approved these changes

@votdev votdev votdev approved these changes

@alfonsomthd alfonsomthd alfonsomthd approved these changes

@callithea callithea callithea approved these changes

@epuertat epuertat Awaiting requested review from epuertat

@Exotelis Exotelis Awaiting requested review from Exotelis

Assignees
No one assigned
Labels
bug-fix dashboard pybind
Projects
Ceph-Dashboard
Archived in project
Dashboard
  
Done
Milestone
No milestone
6 participants
@nizamial09 @tchaikov @votdev @alfonsomthd @callithea @epuertat