aws-iam-authenticator support added (2) by slyoldfox · Pull Request #111 · databricks/click

slyoldfox · 2019-06-05T13:30:45Z

Synced the PR from #78 with the latest master branch and added some caching for the token.

At the moment haven't reused much from AuthProvider since it does seem a bit different to me.
If you have any remarks (my rust experience is 0), please let me know @nicklan

philipbjorge · 2019-06-13T18:48:27Z

Thanks for adding this @slyoldfox!
@nicklan -- I'd love to see this merged but in the meantime can try a custom build :). Thanks!

@slyoldfox -- I made a release build (mv target/release/click /usr/local/bin/click) and am hitting the following error while running...

⟩ env RUST_BACKTRACE=1 click
[staging-k8s.x.io] [none] [none] > pods
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/libcore/option.rs:355:21
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::_print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: std::panicking::rust_panic_with_hook
   5: std::panicking::continue_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::panicking::panic
   9: click::config::kubefile::Exec::ensure_token
  10: click::kube::Kluster::add_auth_header
  11: click::kube::Kluster::send_req
  12: click::kube::Kluster::get
  13: click::Env::run_on_kluster
  14: <click::cmd::Pods as click::cmd::Cmd>::exec
  15: click::main
  16: std::rt::lang_start::{{closure}}
  17: std::panicking::try::do_call
  18: __rust_maybe_catch_panic
  19: std::rt::lang_start_internal
  20: main

slyoldfox · 2019-06-14T09:09:00Z

@philipbjorge I had the same issues running with the aws-iam-authenticator binary, but since awscli now generates it with aws eks get-token exec I haven't bothered much in figuring out what the issue was.

Try updating to the latest awscli which supports the get-token construct and use them like this in your .kube/config:

users:
- name: eks-acceptance
  user:
    exec:
      apiVersion: client.authentication.k8s.io/v1alpha1
      args:
      - --region
      - eu-west-1
      - eks
      - get-token
      - --cluster-name
      - eks-acceptance
      command: aws
      env:
      - name: AWS_PROFILE
        value: default

And of course make sure you your profile name matches with the one from your aws-adfs login, check with aws sts get-caller-identity --profile default

Drop me your .kube/config files if it doesn't work and I'll try debugging it.

kkolk · 2019-07-16T15:25:41Z

I pulled this down today because I wanted to give click a try with AWS EKS and that wasn't working with the current release. Had to merge databricks/master into it since rust threw a few errors compiling dependencies (rustyline).

But, once that was cleared up this worked immediately for me with my existing kubeconfig.

My build error was the same one that caused Travis to mark this as failed, so pulling master into this resolves that failing check.

slyoldfox · 2019-07-16T19:27:18Z

@kkolk thanks for pinging me on this. I've updated the PR by merging master onto it and that seems to have fixed Travis indeed!

I hope @nicklan has some time to review the PR.

nicklan

Cool! Thanks for sticking with this though the delays. I'm okay with having this as a separate thing from AuthProvider for now, although I would at some point like to merge them more.

I've left a few small things I'd like changed, but then I think it can be merged.

src/config/kube.rs

nicklan · 2019-07-24T14:31:22Z

src/config/kubefile.rs

 }

+#[derive(Debug, Deserialize, Clone)]
+#[allow(non_snake_case)]


are you using this so that the deserialize finds the fields? if so, rather do something like this:

#[derive(Debug, Deserialize, Clone)] pub struct Exec { #[serde(rename = "apiVersion")] api_version: String, pub args: Option<Vec<String>>, pub command: Option<String>, pub env: Option<Vec<Env>>, pub token: RefCell<Option<String>>, pub expiry: RefCell<Option<DateTime<Utc>>>, }

nicklan · 2019-07-24T14:31:41Z

src/config/kubefile.rs

+}
+
+#[derive(Serialize, Deserialize)]
+#[allow(non_snake_case)]


ditto to above about snake case thing

nicklan · 2019-07-24T14:34:00Z

src/config/kubefile.rs

+    apiVersion: String,
+    pub args: Option<Vec<String>>,
+    pub command: Option<String>,
+    pub env: Option<Vec<Env>>,


Can you not just make this an Option<HashMap<String, String>> directly? I think serde should just "do the right thing" in that case.

I tried this and the suggestions below. It compiles fine, but when I start click it gives the following error which I don't understand:

$ cargo build && ./target/debug/click Compiling click v0.4.2 (/Users/user/Code/click/click) Finished dev [unoptimized + debuginfo] target(s) in 8.77s Could not load kubernetes config. Cannot continue. Error was: Couldn't read yaml in '/Users/user/.kube/config': invalid type: sequence, expected a map

I'm not understanding why, as the generate_token function is not even called when starting, just after the first request to the API.

Any ways I can debug this faster?

@nicklan
Apparently serde doesn't like parsing the following when it's a

Option<HashMap<String, String>>

users: - name: eks-acceptance user: exec: apiVersion: client.authentication.k8s.io/v1alpha1 args: - --region - eu-west-1 - eks - get-token - --cluster-name - eks-acceptance command: aws env: - name: AWS_PROFILE value: default

I suppose it's something you could test too, it happens just after starting the application. For some reason it doesn't like the env. Not sure why.

nicklan · 2019-07-24T14:37:37Z

src/config/kubefile.rs

+    }
+
+    fn generate_token(&self) -> String {
+        let mut filtered_env: HashMap<String, String> = HashMap::new();


If we make the above change to have self.env be a HashMap, this shouldn't be needed. Below you can just do:

let output = Command::new(self.command.clone().unwrap()) .args(self.args.clone().unwrap()) .envs(&self.env) .output() .expect("failed to execute process");

Without testing myself I'm not sure you won't have to clone it, since envs want's an IntoIter, so you may need to do that.

philipbjorge · 2019-07-30T01:38:41Z

I took another look at #111 (comment)

The crash is on this line and occurs because user.exec.env was null in my config.

- name: xxx
  user:
    exec:
      apiVersion: client.authentication.k8s.io/v1alpha1
      args:
      - token
      - -i
      - xxx
      - -r
      - arn:aws:iam::xxx:role/KubernetesUser
      command: aws-iam-authenticator
      env: null # also crashes if this key is omitted

When setting the value to [] it does not crash, and I can successfully access my cluster.

nicklan · 2020-02-03T22:54:42Z

I believe this is provided now by what I merged in #129. If anyone who is interested in EKS auth could try that out, that would be great. I tested by adding the cluster to my config via aws eks update-kubeconfig --name cluster-name and then just running click and it worked.

slyoldfox added 2 commits June 5, 2019 15:14

databricks#89 updated PR - added caching

c2aa2ee

formatting

14e9816

Merge remote-tracking branch 'upstream/master'

0b7ee48

nicklan requested changes Jul 24, 2019

View reviewed changes

slyoldfox added 2 commits July 25, 2019 20:20

_exec -> exec

82e66c1

apiVersion -> api_version

72b8a87

Conversation

slyoldfox commented Jun 5, 2019

Uh oh!

philipbjorge commented Jun 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slyoldfox commented Jun 14, 2019

Uh oh!

kkolk commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slyoldfox commented Jul 16, 2019

Uh oh!

nicklan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicklan Jul 24, 2019

Choose a reason for hiding this comment

Uh oh!

nicklan Jul 24, 2019

Choose a reason for hiding this comment

Uh oh!

slyoldfox Jul 25, 2019

Choose a reason for hiding this comment

Uh oh!

nicklan Jul 24, 2019

Choose a reason for hiding this comment

Uh oh!

slyoldfox Jul 25, 2019

Choose a reason for hiding this comment

Uh oh!

slyoldfox Jul 25, 2019

Choose a reason for hiding this comment

Uh oh!

nicklan Jul 24, 2019

Choose a reason for hiding this comment

Uh oh!

philipbjorge commented Jul 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicklan commented Feb 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

philipbjorge commented Jun 13, 2019 •

edited

Loading

kkolk commented Jul 16, 2019 •

edited

Loading

philipbjorge commented Jul 30, 2019 •

edited

Loading