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

Secrets endpoints #91

Merged
merged 4 commits into from
Feb 5, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
37 changes: 37 additions & 0 deletions cloudflare/src/endpoints/workers/create_secret.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use super::WorkersSecret;

use crate::framework::endpoint::{Endpoint, Method};

/// Create Secret
/// https://api.cloudflare.com/#worker-create-secret
pub struct CreateSecret<'a> {
/// account id of owner of the script
pub account_identifier: &'a str,
/// the name of the script to attach the secret to
pub script_name: &'a str,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm starting to think that these should be owned strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean by these? @ashleymichal how should we apply this suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically turn &'a str into a String

Copy link
Contributor

Choose a reason for hiding this comment

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

we don't do this anywhere else :-\

Copy link
Contributor

@ashleymichal ashleymichal Feb 5, 2020

Choose a reason for hiding this comment

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

@sssilver i actually started turning all of these &'a str's into &'static str's elsewhere in wrangler. saves the requirement for lifetimes on all your structs and their methods. will illustrate in another pr.

/// secert's contents
Copy link
Contributor

Choose a reason for hiding this comment

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

s/secert/secret

Also all our comments elsewhere start with a capital, could you please change yours to be consistent? :)

pub params: CreateSecretParams,
}

impl<'a> Endpoint<WorkersSecret, (), CreateSecretParams> for CreateSecret<'a> {
fn method(&self) -> Method {
Method::Put
}
fn path(&self) -> String {
format!(
"accounts/{}/workers/scripts/{}/secrets",
self.account_identifier, self.script_name
)
}
fn body(&self) -> Option<CreateSecretParams> {
Some(self.params.clone())
}
}

#[derive(Serialize, Clone, Debug)]
pub struct CreateSecretParams {
/// the variable name of the secret that will be bound to the script
pub name: String,
/// the string value of the secret
pub value: String,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are secret values strings or bytes? It feels like the latter would be more proper, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

yah bytes is probably better

Copy link
Contributor

Choose a reason for hiding this comment

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

except you can't precisely send a byte string in a json body...

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 serialize them as a string, however within the Rust type system it should still be bytes

Copy link
Contributor

Choose a reason for hiding this comment

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

That would force you to think about how that "string" should be deserialized. There isn't a single way to serialize bytes into a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was giving me a hard time. Since it's not a blocker I filed an issue to prioritize later #98

}
15 changes: 15 additions & 0 deletions cloudflare/src/endpoints/workers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use crate::framework::response::ApiResult;
use serde::Deserialize;

mod create_route;
pub mod create_secret;
mod delete_route;
pub mod delete_secret;
exvuma marked this conversation as resolved.
Show resolved Hide resolved
mod list_routes;

pub use create_route::{CreateRoute, CreateRouteParams};
Expand Down Expand Up @@ -37,3 +39,16 @@ pub struct WorkersRouteIdOnly {
}

impl ApiResult for WorkersRouteIdOnly {}

/// Secrets attach to a single script to be readable in only the script
/// https://api.cloudflare.com/#worker-secrets-properties
#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
pub struct WorkersSecret {
/// TODO these fields depend on the API and may be wrong
pub name: String,
pub modified_on: String,
exvuma marked this conversation as resolved.
Show resolved Hide resolved
pub script_id: String,
exvuma marked this conversation as resolved.
Show resolved Hide resolved
}

impl ApiResult for WorkersSecret {}
impl ApiResult for Vec<WorkersSecret> {} // to parse arrays too