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

fix: GCE's credential load should be blocking API #281

Merged
merged 1 commit into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 14 additions & 16 deletions src/google/credential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ impl CredentialLoader {
}

/// Load credential from pre-configured methods.
pub async fn load(&self) -> Result<Option<Credential>> {
pub fn load(&self) -> Result<Option<Credential>> {
// Return cached credential if it has been loaded at least once.
if let Some(cred) = self.credential.lock().expect("lock poisoned").clone() {
return Ok(Some(cred));
}

let cred = if let Some(cred) = self.load_inner().await? {
let cred = if let Some(cred) = self.load_inner()? {
cred
} else {
return Ok(None);
Expand All @@ -75,34 +75,34 @@ impl CredentialLoader {
Ok(Some(cred))
}

async fn load_inner(&self) -> Result<Option<Credential>> {
fn load_inner(&self) -> Result<Option<Credential>> {
if let Some(cred) = self.load_via_content()? {
return Ok(Some(cred));
}

if let Some(cred) = self.load_via_path().await? {
if let Some(cred) = self.load_via_path()? {
return Ok(Some(cred));
}

if let Some(cred) = self.load_via_env().await? {
if let Some(cred) = self.load_via_env()? {
return Ok(Some(cred));
}

if let Some(cred) = self.load_via_well_known_location().await? {
if let Some(cred) = self.load_via_well_known_location()? {
return Ok(Some(cred));
}

Ok(None)
}

async fn load_via_path(&self) -> Result<Option<Credential>> {
fn load_via_path(&self) -> Result<Option<Credential>> {
let path = if let Some(path) = &self.path {
path
} else {
return Ok(None);
};

Ok(Some(Self::load_file(path).await?))
Ok(Some(Self::load_file(path)?))
}

/// Build credential loader from given base64 content.
Expand All @@ -121,13 +121,13 @@ impl CredentialLoader {
}

/// Load from env GOOGLE_APPLICATION_CREDENTIALS.
async fn load_via_env(&self) -> Result<Option<Credential>> {
fn load_via_env(&self) -> Result<Option<Credential>> {
if self.disable_env {
return Ok(None);
}

if let Ok(cred_path) = env::var(GOOGLE_APPLICATION_CREDENTIALS) {
let cred = Self::load_file(&cred_path).await?;
let cred = Self::load_file(&cred_path)?;
Ok(Some(cred))
} else {
Ok(None)
Expand All @@ -138,7 +138,7 @@ impl CredentialLoader {
///
/// - `$HOME/.config/gcloud/application_default_credentials.json`
/// - `%APPDATA%\gcloud\application_default_credentials.json`
async fn load_via_well_known_location(&self) -> Result<Option<Credential>> {
fn load_via_well_known_location(&self) -> Result<Option<Credential>> {
if self.disable_well_known_location {
return Ok(None);
}
Expand All @@ -156,14 +156,13 @@ impl CredentialLoader {

let cred = Self::load_file(&format!(
"{config_dir}/gcloud/application_default_credentials.json"
))
.await?;
))?;
Ok(Some(cred))
}

/// Build credential loader from given path.
async fn load_file(path: &str) -> Result<Credential> {
let content = tokio::fs::read(path).await.map_err(|err| {
fn load_file(path: &str) -> Result<Credential> {
let content = std::fs::read(path).map_err(|err| {
debug!("load credential failed at reading file: {err:?}");
err
})?;
Expand Down Expand Up @@ -209,7 +208,6 @@ mod tests {

let cred = cred_loader
.load()
.await
.expect("credentail must be exist")
.unwrap();

Expand Down
4 changes: 2 additions & 2 deletions src/google/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ mod tests {
);

let loader = CredentialLoader::default().with_path(&credential_path);
let cred = loader.load().await?.unwrap();
let cred = loader.load()?.unwrap();

let signer = Signer::new("storage");

Expand Down Expand Up @@ -385,7 +385,7 @@ mod tests {
);

let loader = CredentialLoader::default().with_path(&credential_path);
let cred = loader.load().await?.unwrap();
let cred = loader.load()?.unwrap();

let mut req = http::Request::new("");
*req.method_mut() = http::Method::GET;
Expand Down
4 changes: 2 additions & 2 deletions tests/google/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ async fn init_signer() -> Option<(GoogleCredentialLoader, GoogleTokenLoader, Goo
.expect("env REQSIGN_GOOGLE_CLOUD_STORAGE_SCOPE must set"),
Client::new(),
)
.with_credentials(cred_loader.load().await.unwrap().unwrap());
.with_credentials(cred_loader.load().unwrap().unwrap());

let signer = GoogleSigner::new("storage");

Expand Down Expand Up @@ -127,7 +127,7 @@ async fn test_get_object_with_query() -> Result<()> {
));
let mut req = builder.body("")?;

let cred = cred_loader.load().await?.unwrap();
let cred = cred_loader.load()?.unwrap();
signer
.sign_query(&mut req, Duration::from_secs(3600), &cred)
.expect("sign request must success");
Expand Down