Skip to content

Switch from webpush to web-push gem to support OpenSSL::V3#10139

Closed
alecslupu wants to merge 11 commits intodecidim:developfrom
i-need-another-coffee:test-pipeline-integrity
Closed

Switch from webpush to web-push gem to support OpenSSL::V3#10139
alecslupu wants to merge 11 commits intodecidim:developfrom
i-need-another-coffee:test-pipeline-integrity

Conversation

@alecslupu
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu commented Dec 8, 2022 •
edited
Loading

🎩 What? Why?

While working on #10016 i have noticed the pipelines were failing due to unrelated issues.

Please refer to zaru/webpush#106 for additional details

Failure/Error: vapid_key = Webpush.generate_key

OpenSSL::PKey::PKeyError:
  pkeys are immutable on OpenSSL 3.0

Additionally the following errors have been seen:

  3) Decidim::Attachment validations when the file is a malicious image is expected not to be valid
     Failure/Error: record.errors.add attribute, I18n.t("carrierwave.errors.file_resolution_too_large") if image.dimensions.any? { |dimension| dimension > uploader.max_image_height_or_width }

     MiniMagick::Error:
       `identify -format %m %w %h %b /tmp/mini_magick20221208-3862-62fi87[0]` failed with error:
       identify-im6.q16: width or height exceeds limit `/tmp/mini_magick20221208-3862-62fi87' @ error/cache.c/OpenPixelCache/3909.

 Error: PDF could not be generated!
 Command Error: Calling `DidYouMean::SPELL_CHECKERS.merge!(error_name => spell_checker)' has been deprecated. Please call `DidYouMean.correct_error(error_name, spell_checker)' instead.
/home/runner/work/decidim/decidim/vendor/bundle/ruby/3.1.0/gems/wkhtmltopdf-binary-0.12.6.5/bin/wkhtmltopdf:61:in `<main>': Invalid platform, must be running on Ubuntu 16.04/18.04/20.04 CentOS 6/7/8, Debian 9/10, archlinux amd64, or intel-based Cocoa macOS (missing binary: /home/runner/work/decidim/decidim/vendor/bundle/ruby/3.1.0/gems/wkhtmltopdf-binary-0.12.6.5

📌 Related Issues

Link your PR to an issue

Testing

Describe the best way to test or validate your PR.

📷 Screenshots

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

@alecslupu alecslupu mentioned this pull request Dec 8, 2022
Merged
3 tasks
@alecslupu alecslupu changed the title Add random text to trigger the full pipelines Switch from webpush to web-push gem to support OpenSSL::V3 Dec 8, 2022
@alecslupu alecslupu force-pushed the test-pipeline-integrity branch from 26e15ed to 1d3bbb4 Compare December 8, 2022 18:28
@alecslupu alecslupu marked this pull request as ready for review December 8, 2022 19:31
@alecslupu alecslupu requested a review from ahukkanen December 8, 2022 19:32
@alecslupu alecslupu mentioned this pull request Dec 9, 2022
Merged
@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Dec 9, 2022

After some chat with @ahukkanen, we have decidided that is much better to use some older version of Github actions until some of the issues in the bindings are getting fixed.

Closing this PR in favor of #10142

@alecslupu alecslupu closed this Dec 9, 2022
@ahukkanen ahukkanen mentioned this pull request Dec 9, 2022
Closed
3 tasks
@ahukkanen
Copy link
Copy Markdown
Contributor

ahukkanen commented Dec 9, 2022 •
edited
Loading

We should eventually make this change because the webpush gem is no longer maintained.

But before doing the fix, we need to get full OpenSSL 3.0 support upstream. The maintainer wants to implement this change, see:
zaru/webpush#106 (comment)

But there were some issues implementing it as-is in that PR.

We recently had similar issue with the bulletin board and the jwt gem. In that gem, the issue was resolved like this:
jwt/ruby-jwt#496

Maybe those changes could help implementing support for the web-push gem as well? Not sure, as I have not looked closely in the PR linked above.

@alecslupu alecslupu mentioned this pull request Jan 7, 2023
Merged
@ahukkanen ahukkanen mentioned this pull request Feb 6, 2023
Merged
@ahukkanen ahukkanen mentioned this pull request Mar 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahukkanen ahukkanen Awaiting requested review from ahukkanen

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alecslupu @ahukkanen