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

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@flokli
Copy link
Contributor

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/

This comment has been minimized.

Copy link
@zimbatm

zimbatm Jun 13, 2019

Member

that change isn't necessary

This comment has been minimized.

Copy link
@flokli

flokli Jun 14, 2019

Author Contributor

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 :-)

This comment has been minimized.

Copy link
@flokli

flokli Jun 14, 2019

Author Contributor

done. :-)

flokli added some commits Jun 12, 2019

@flokli flokli force-pushed the flokli:gcloud-path branch from 81be359 to 6a166b2 Jun 14, 2019

@ofborg ofborg bot requested a review from zimbatm Jun 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.