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.

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

Rendering: Fix Windows plugin signature check #76123

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

AgnesToulet
Copy link
Contributor

@AgnesToulet AgnesToulet commented Oct 6, 2023

What is this feature?
With the recent puppeteer upgrade in the grafana-image-renderer plugin, the path to the debug.log file was updated from chrome/debug.log to chrome/win64-<chrome version>/chrome-win64/debug.log. This PR updates the signature check accordingly (but still support old versions of the image renderer).

Why do we need this feature?
Without this, new versions of the image renderer plugin won't be able to load once the debug.log is created by the Chrome process.

Who is this feature for?
Windows users using the grafana-image-renderer plugin.

Which issue(s) does this PR fix?:

Fixes #

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@AgnesToulet AgnesToulet requested a review from a team as a code owner October 6, 2023 12:57
@AgnesToulet AgnesToulet requested review from wbrowne, marefr and xnyo and removed request for a team October 6, 2023 12:57
@grafana-delivery-bot grafana-delivery-bot bot added this to the 10.2.x milestone Oct 6, 2023
Copy link
Member

@wbrowne wbrowne left a comment

Choose a reason for hiding this comment

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

I pray for the day we can delete this check 😭

LGTM!

@@ -209,7 +209,7 @@ func (s *Signature) Calculate(ctx context.Context, src plugins.PluginSource, plu
f = toSlash(f)

// Ignoring unsigned Chromium debug.log so it doesn't invalidate the signature for Renderer plugin running on Windows
if runningWindows && plugin.JSONData.Type == plugins.TypeRenderer && f == "chrome-win/debug.log" {
if runningWindows && plugin.JSONData.Type == plugins.TypeRenderer && strings.HasSuffix(f, "debug.log") {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Maybe filepath.Base is the most appropriate func to use here now 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I updated the PR.

@AgnesToulet
Copy link
Contributor Author

Yes, I agree! Do we have an issue about how we could fix this?

@AgnesToulet AgnesToulet merged commit 46c1c03 into main Oct 6, 2023
20 checks passed
@AgnesToulet AgnesToulet deleted the agnestoulet/fix-renderer-debug branch October 6, 2023 13:48
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants