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: kubeconfig: don't store absolute path to gcloud binary #63037

Merged
merged 2 commits into from Jun 19, 2019

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Jun 12, 2019

Motivation for this change

I got bitten too often by a hardcoded and disappearing path to gcloud in the kubectl config.

This PR fixes the google-cloud-sdk derivation to allow to apply patches on top of it, then patches google-cloud-sdk to not store absolute paths in configuration files.

Description from that patch:

The gcloud beta container clusters get-credentials $cluster \ --region $region --project $project
command can be used to write kubectl config files.

In that file, normally the absolute path to the gcloud binary is
stored.

This is a bad idea in NixOS. We might eventually garbage-collect that
specific gcloud binary - and in general, would expect a nix-shell
provided gcloud to be used.

In its current state, token renewal would just start to break with the
following error message:

Unable to connect to the server: error executing access token command "/nix/store/…/gcloud config config-helper --format=json": err=fork/exec /nix/store/…/gcloud: no such file or directory output= stderr=

Avoid this by storing just gcloud inside cmd-path, which causes
kubectl to lookup the gcloud command from $PATH, which is more likely to
keep working.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • x ] Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc @arianvp

@@ -68,7 +67,7 @@ in stdenv.mkDerivation rec {
disable_update_check = true" >> $out/google-cloud-sdk/properties

# setup bash completion
mkdir -p "$out/etc/bash_completion.d/"
mkdir -p $out/etc/bash_completion.d/
Copy link
Member

Choose a reason for hiding this comment

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

that change isn't necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that was just to remove the superfluous double quotes and match style with the rest - we don't have spaces in $out etc.

In fact, I should change the line below that one too :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. :-)

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

2 participants