Skip to content
This repository has been archived by the owner on Dec 21, 2022. It is now read-only.

support K8s service account for auth #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

support K8s service account for auth #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/env.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ pub(crate) fn fetch(opts: FetchOpts) -> Result<HashMap<String, String>, Error> {
let user = prompt_default("Vault username: ", env::var("USER").ok())?;
let password = prompt_password("Vault password: ")?;
vault.ldap_auth(&user, &password)?;
} else if opts.kube && std::path::Path::new("/var/run/secrets/kubernetes.io/serviceaccount/token").exists() {
debug!("Authenticating with Vault via K8s ServiceAccount");
let role: String = service.to_string();
vault.kubernetes_auth(role)?;
Comment on lines +120 to +121
Copy link
Contributor

Choose a reason for hiding this comment

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

service is already a String, what you want here is to either give kubernetes_auth a copy of service, like vault.kubernetes_auth(service.clone())?;, or lend it a &str reference to service like vault.kubernetes_auth(&service)?;.

The second option requires a small change to kubernetes_auth (explained in later comments), but makes more sense in Rust, and avoids a needless copy.

In Rust when you give a function a value it is moved into that function, and you can't get it back, but using & lets you lend a value to a function in a way that you do get it back.

} else if let (Some(app_id), Some(app_user)) = (&opts.app_id, &opts.app_user) {
debug!("Authenticating with Vault via App ID");
vault.app_id_auth(app_id, app_user)?;
Expand Down
5 changes: 4 additions & 1 deletion src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ pub(crate) struct FetchOpts {
/// authenticate with vault
#[structopt(long = "dev")]
dev: bool,
/// use Kubernetes ServiceAccount to get token
#[structopt(long = "kube")]
kube: bool,
/// add an environment variable
#[structopt(
short = "a",
Expand All @@ -143,7 +146,7 @@ pub(crate) struct FetchOpts {
long = "vault-token",
value_name = "TOKEN",
env = "VAULT_TOKEN",
required_unless_one = &["dev", "app_user", "app_id"]
required_unless_one = &["dev", "kube", "app_user", "app_id"]
)]
token: Option<vault::Secret>,
/// authenticate with vault app-user
Expand Down
23 changes: 23 additions & 0 deletions src/vault.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use std::{fmt, str::FromStr};

use std::fs::File;
use std::io::Read;
Comment on lines 1 to +4
Copy link
Contributor

Choose a reason for hiding this comment

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

You can combine these into a single statement like use std::{io::Read, fmt, fs::File, str::FromStr};


use log::{debug, trace, warn};
use reqwest::Url;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -64,6 +67,12 @@ struct AppIdAuthRequest<'a> {
user_id: &'a str,
}

#[derive(Serialize)]
pub struct AuthKubernetesRequest {
pub jwt: String,
pub role: String
}
Comment on lines +71 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be able to just take string references like:

#[derive(Serialize)]
pub struct AuthKubernetesRequest<'a> {
    pub jwt: &'a str,
    pub role: &'a str,
}

You'll with that change probably need to create an instance like:

AuthKubernetesRequest { jwt: &jwt, role }

as you'll have an owned String, but it wants a &str reference (role will already be a &str reference if you follow my other comments).


#[derive(Deserialize)]
struct AuthResponse {
client_token: String,
Expand Down Expand Up @@ -135,6 +144,20 @@ impl Client {
Ok(())
}

pub fn kubernetes_auth(&mut self, role: String) -> Result<(), Error> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't need to take an owned copy of the role argument, it would work fine with just a reference, like pub fn kubernetes_auth(&mut self, role:&str) -> Result<(), Error> {. That will require a tweak to AuthKubernetesRequest, see the comment on AuthKubernetesRequest.

let mut file = File::open("/var/run/secrets/kubernetes.io/serviceaccount/token").expect("Unable to open");
let mut jwt = String::new();
file.read_to_string(&mut jwt);
Comment on lines +148 to +150
Copy link
Contributor

Choose a reason for hiding this comment

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

Both File::open and File::read_to_string can return a std::io::Error, to be able to use the ? operator you need to be able to convert a std::io::Error to the Error type this function returns (defined up on line 87).

To be able to do that conversion you need to implement the std::convert::From trait. Currently the way Error is defined it holds a ClientError, but you've got a std::io::Error, so you probably want it to also be able to hold a ClientError or a std::io::Error. You would do this by changing it to an enum, like:

pub enum Error {
    ClientError(ClientError),
    IoError(std::io::Error),
}

You'd then need to update the various trait implementations for ClientError to now be able to cope with it being an enum, and finally implement the From trait. Check client_error.rs for examples.

Ignoring the error from File::read_to_string will mean that if it fails you'll end up with an empty jwt, and things will probably break later down the line in confusing ways.

Using expect(...) on the error from File::open will mean the program will immediately crash with an ugly error, instead of passing the error back up to any error handing that may exist in the caller.

// .is_err() {
// return Err(Error::);
// // return Err(String::from("Kubernetes authentication failed"));
// };
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this error handling not needed?

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't sure how to add it properly..
read_to_string? is not possible (perhaps, after Rust language changes)
and I could not find a usage Error class.. I guess, new one needs to be created

let request = AuthKubernetesRequest { jwt, role };
let response: AuthResponseWrapper = self.post(&format!("auth/kubernetes/login"), &request)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to use format here, as you're not adding anything to the string, just self.post("auth/kubernetes/login", &request)?; should do.

self.token = Some(Secret(response.auth.client_token));
Ok(())
}

fn resolve_leader(&mut self) -> Result<(), Error> {
trace!("Resolving Vault leader");
let info = match self.get_internal::<LeaderResponse>("/sys/leader")? {
Expand Down