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

grafana: fix srcStatic hash #251479

Merged
merged 1 commit into from
Aug 26, 2023
Merged

grafana: fix srcStatic hash #251479

merged 1 commit into from
Aug 26, 2023

Conversation

ipetkov
Copy link
Contributor

@ipetkov ipetkov commented Aug 26, 2023

Description of changes

Fix grafana hash so it can build #251138

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ipetkov
Copy link
Contributor Author

ipetkov commented Aug 26, 2023

Result of nixpkgs-review run on x86_64-linux 1

1 package built:
  • grafana

Copy link
Member

@fpletz fpletz left a comment

Choose a reason for hiding this comment

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

Also fails for me. The tarball seems to have been modified.

@NickCao NickCao merged commit a422c34 into NixOS:master Aug 26, 2023
@ipetkov ipetkov deleted the fix-grafana branch August 26, 2023 20:41
@delroth
Copy link
Contributor

delroth commented Aug 28, 2023

Does someone (@Ma27 maybe?) happen to have the original grafana-10.1.0.linux-amd64.tar.gz in one of their Nix stores? Unfortunately it never made it to Hydra. I'd like to diff and figure out what happened there.

I think it should be /nix/store/mysg53a5zfbh4wg4s025f7klw65q7va7-grafana-10.1.0.linux-amd64.tar.gz.

@Ma27
Copy link
Member

Ma27 commented Aug 29, 2023

Sorry, had a similar thought when I've first seen this, but forgot about it.

First of all, to give everyone a chance to verify, here's the old tarball from my store: https://wolke.mbosch.me/s/YR65bdtY6d4MeSe (out path is /nix/store/mysg53a5zfbh4wg4s025f7klw65q7va7-grafana-10.1.0.linux-amd64.tar.gz, this is what I get when running nix-build -A grafana.srcStatic on bff4a35).

A diffoscope with everything in the unpacked tarball is still running (because diffing bin/ is very slow), but this one is uninteresting for us anyways because we don't use that, but our own binaries.
A second run with bin excluded gives mostly modify date differences and the following: https://gist.github.com/Ma27/c30062629db54a3f1c0dc3c41eea1f32

Not sure why they'd republish, but this seems like an impurity on building the release tarball to me.

Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Jan 23, 2024
Up until now, the frontend was taken from `srcStatic`, i.e. prebuilt
from upstream. I recall at least three cases[1][2][3] where we got a hash
mismatch eventually.

Rather than spending time finding out whether or not it's a supply-chain
attack or just a build issue, I decided to implement a source-build now
with the following benefits:

* It's now actually possible to apply patches for Grafana's frontend.
* We rely a little less on third-party build systems.

Of course, patching potential vulnerabilities in transitive frontend
dependencies is still hard (let alone discovering that this package is
affected!), but that's a fundamental issue we have in nixpkgs and I
won't invent a half-baked solution just for this package, I still
consider this a step into the right direction.

The build itself mainly orients on the `yarn` commands used in the
upstream Makefile[4]. However, we can't use `fetchYarnDeps` here because
yarn v2 (a.k.a. `berry`) is in use which is why the same was done as in
`hedgedoc`, writing a custom FoD that downloads all dependencies and
writes the offline cache into `$out`[5].

Additionally there are two more notable differences to upstream:

* We patch out every dependency to `@grafana/e2e` and `cypress`. The
  first is a dependency on the latter in another version and the latter
  downloads random blobs from the Internet in postInstall. Since it's a
  testing framework (and the `e2e` package apparently a testing
  library), I decided it's not worth the effort and patched it out
  everywhere.

* There was a `zoneinfo.zip` in `$out/share/grafana/tools` that was
  installed from `srcStatic`. This only seems to be used on Windows[6]
  and that's not supported by this package, so I decided to drop it.

[1] NixOS#251479
[2] NixOS#130201
[3] NixOS#104794
[4] https://github.com/grafana/grafana/blob/v10.3.1/Makefile
[5] NixOS#245170
[6] https://github.com/grafana/grafana/blob/v10.3.1/pkg/setting/setting.go#L1012-L1014
Ma27 added a commit to Ma27/nixpkgs that referenced this pull request May 26, 2024
Up until now, the frontend was taken from `srcStatic`, i.e. prebuilt
from upstream. I recall at least three cases[1][2][3] where we got a hash
mismatch eventually.

Rather than spending time finding out whether or not it's a supply-chain
attack or just a build issue, I decided to implement a source-build now
with the following benefits:

* It's now actually possible to apply patches for Grafana's frontend.
* We rely a little less on third-party build systems.

Of course, patching potential vulnerabilities in transitive frontend
dependencies is still hard (let alone discovering that this package is
affected!), but that's a fundamental issue we have in nixpkgs and I
won't invent a half-baked solution just for this package, I still
consider this a step into the right direction.

The build itself mainly orients on the `yarn` commands used in the
upstream Makefile[4]. However, we can't use `fetchYarnDeps` here because
yarn v2 (a.k.a. `berry`) is in use which is why the same was done as in
`hedgedoc`, writing a custom FoD that downloads all dependencies and
writes the offline cache into `$out`[5].

Additionally there are two more notable differences to upstream:

* We patch out every dependency to `@grafana/e2e` and `cypress`. The
  first is a dependency on the latter in another version and the latter
  downloads random blobs from the Internet in postInstall. Since it's a
  testing framework (and the `e2e` package apparently a testing
  library), I decided it's not worth the effort and patched it out
  everywhere.

* There was a `zoneinfo.zip` in `$out/share/grafana/tools` that was
  installed from `srcStatic`. This only seems to be used on Windows[6]
  and that's not supported by this package, so I decided to drop it.

[1] NixOS#251479
[2] NixOS#130201
[3] NixOS#104794
[4] https://github.com/grafana/grafana/blob/v10.3.1/Makefile
[5] NixOS#245170
[6] https://github.com/grafana/grafana/blob/v10.3.1/pkg/setting/setting.go#L1012-L1014

(cherry picked from commit 608db26)
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.

5 participants