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

google-cloud-sdk: fix gcloud storage sign-url error due to missing pyopenssl dependency #301986

Merged

Conversation

mrene
Copy link
Contributor

@mrene mrene commented Apr 6, 2024

#141773 changes the python dependencies to include openssl instead of pyopenssl which seems like a mistake due to openssl not existing inside pythonPackages (and the with binding picking up the top level dependency instead). The author referenced #135045 which was caused by pyopenssl crashing on aarch64-darwin (pyca/pyopenssl#873), due to cffi's behaviour. Since then, #187636 has fixed cffi and hence makes pyopenssl compatible with aarch64-darwin.

Despite the previous change, pyopenssl is still required to sign urls to Google Cloud Storage buckets when using a service account private key file. Attempting to do so with the current package results in:

❯ gcloud storage sign-url --private-key-file /path/to/file.json gs://bucket/file
ERROR: (gcloud.storage.sign-url) This command requires the pyOpenSSL library. Please install it and set the environment variable CLOUDSDK_PYTHON_SITEPACKAGES to 1 before re-running this command.

Changing the dependency back to pyopenssl fixes this issue, and the steps to reproduce #135045 still succeed on aarch64-darwin.

Unfortunately, gsutil will still show pyopenssl as missing because they rely on the presence of a now-deleted pyopenssl function and fall back to the same (confusing) error message (GoogleCloudPlatform/gsutil#1753).

(I have attempted to write a test for this, but gcloud absolutely want to be authenticated to google cloud in order to reach a point where the error would've been displayed)

Description of changes

Fix the pyopenssl dependency by referencing the right package

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

@ahirner
Copy link
Contributor

ahirner commented Apr 6, 2024

Thanks for analysing the issue thoroughly. Those google tools contain some of the worst code I've seen (not saying this lightly).
FWIW, I do have a complete fix, not sure it's a good enough fix for nixpkgs.
https://github.com/ahirner/nix-trickle/blob/main/overlays/google-cloud-sdk.nix

@marcusramberg
Copy link
Contributor

Result of nixpkgs-review pr 301986 run on aarch64-darwin 1

1 package marked as broken and skipped:
  • google-cloud-sdk-gce
3 packages built:
  • cve-bin-tool
  • cve-bin-tool.dist
  • google-cloud-sdk

Copy link
Contributor

@marcusramberg marcusramberg left a comment

Choose a reason for hiding this comment

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

Tested and confirmed this fixes the pyOpenSSL error with sign-url.

@marcusramberg marcusramberg merged commit 711b534 into NixOS:master May 12, 2024
26 checks passed
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.

3 participants