-
Notifications
You must be signed in to change notification settings - Fork 301
[Storage] SAS proof-of-concept #2798
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
base: main
Are you sure you want to change the base?
Conversation
sdk/storage/azure_storage_blob/src/generated/clients/blob_client.rs
Outdated
Show resolved
Hide resolved
sdk/storage/azure_storage_blob/src/generated/clients/blob_client.rs
Outdated
Show resolved
Hide resolved
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
I thought you folks were going to consider a builder pattern for SAS tokens? |
impl GeneratedBlobClient { | ||
fn from_url_with_credential( | ||
blob_url: &str, | ||
credential: Arc<dyn TokenCredential>, | ||
options: Option<BlobClientOptions>, | ||
) -> Result<Self> { | ||
let options = options.unwrap_or_default(); | ||
let blob_url = Url::parse(blob_url)?; | ||
if !blob_url.scheme().starts_with("http") { | ||
return Err(azure_core::Error::message( | ||
azure_core::error::ErrorKind::Other, | ||
format!("{blob_url} must use http(s)"), | ||
)); | ||
} | ||
|
||
let mut segments = blob_url | ||
.path_segments() | ||
.expect("Failed to get path segments"); | ||
let container_name = segments | ||
.next() | ||
.expect("Failed to parse container_name") | ||
.to_string(); | ||
let blob_name = segments.collect::<Vec<_>>().join("/"); | ||
|
||
let auth_policy: Arc<dyn Policy> = Arc::new(BearerTokenCredentialPolicy::new( | ||
credential, | ||
vec!["https://storage.azure.com/.default"], | ||
)); | ||
|
||
Ok(Self { | ||
blob_name, | ||
container_name, | ||
blob_url, | ||
version: options.version, | ||
pipeline: Pipeline::new( | ||
option_env!("CARGO_PKG_NAME"), | ||
option_env!("CARGO_PKG_VERSION"), | ||
options.client_options, | ||
Vec::default(), | ||
vec![auth_policy], | ||
), | ||
}) | ||
} | ||
|
||
fn from_url_with_no_credential( | ||
blob_url: &str, | ||
options: Option<BlobClientOptions>, | ||
) -> Result<Self> { | ||
let options = options.unwrap_or_default(); | ||
let blob_url = Url::parse(blob_url)?; | ||
if !blob_url.scheme().starts_with("http") { | ||
return Err(azure_core::Error::message( | ||
azure_core::error::ErrorKind::Other, | ||
format!("{blob_url} must use http(s)"), | ||
)); | ||
} | ||
|
||
let mut segments = blob_url | ||
.path_segments() | ||
.expect("Failed to get path segments"); | ||
let container_name = segments | ||
.next() | ||
.expect("Failed to parse container_name") | ||
.to_string(); | ||
let blob_name = segments.collect::<Vec<_>>().join("/"); | ||
|
||
Ok(Self { | ||
blob_name, | ||
container_name, | ||
blob_url, | ||
version: options.version, | ||
pipeline: Pipeline::new( | ||
option_env!("CARGO_PKG_NAME"), | ||
option_env!("CARGO_PKG_VERSION"), | ||
options.client_options, | ||
Vec::default(), | ||
Vec::default(), | ||
), | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it turns out we can just implement out custom ctors like this. So regardless of what comes out of the emitter, we can simply do what we need to in our constructor and then use struct initialization.
let client = | ||
GeneratedBlobClient::from_url_with_credential(url.as_str(), credential, Some(options))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this instance, the regular convenience new()
will use from_url_with_credential
underlying
pub fn from_blob_url(blob_url: &str, options: Option<BlobClientOptions>) -> Result<Self> { | ||
let mut options = options.unwrap_or_default(); | ||
|
||
let storage_headers_policy = Arc::new(StorageHeadersPolicy); | ||
options | ||
.client_options | ||
.per_call_policies | ||
.push(storage_headers_policy); | ||
|
||
let url = Url::parse(blob_url)?; | ||
let client = GeneratedBlobClient::from_url_with_no_credential(url.as_str(), Some(options))?; | ||
|
||
Ok(Self { client }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example of exposed, handwritten from_blob_url
. We could then add credential support (either completely discrete API, or Option single API matching)
pub struct BlobClient { | ||
pub(crate) blob_name: String, | ||
pub(crate) container_name: String, | ||
pub(crate) endpoint: Url, | ||
pub(crate) blob_url: Url, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generated code change #1: endpoint
-> blob_url
. We can maintain all other fields because we return these struct fields directly from our convenience layer for the getters
pub async fn copy_from_url( | ||
pub async fn get_properties( | ||
&self, | ||
copy_source: String, | ||
options: Option<BlobClientCopyFromUrlOptions<'_>>, | ||
) -> Result<Response<BlobClientCopyFromUrlResult, NoFormat>> { | ||
options: Option<BlobClientGetPropertiesOptions<'_>>, | ||
) -> Result<Response<BlobClientGetPropertiesResult, NoFormat>> { | ||
let options = options.unwrap_or_default(); | ||
let ctx = Context::with_context(&options.method_options.context); | ||
let mut url = self.endpoint.clone(); | ||
let mut path = String::from("{containerName}/{blobName}"); | ||
path = path.replace("{blobName}", &self.blob_name); | ||
path = path.replace("{containerName}", &self.container_name); | ||
url = url.join(&path)?; | ||
url.query_pairs_mut() | ||
.append_pair("comp", "copy") | ||
.append_key_only("sync"); | ||
let mut url = self.blob_url.clone(); | ||
if let Some(snapshot) = options.snapshot { | ||
url.query_pairs_mut().append_pair("snapshot", &snapshot); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only currently working API
Generated code change #2:
On a per-API basis, we no longer need to get path parameters in there (i.e. container, blob) since it will now build off blob_url
and not a naked endpoint
So in general it seems like as long as the generated code changes to using We can look for emitter support for some semblance of a useful constructor, otherwise we can always fall back to just plainly implementing them right there in our handwritten layer 😄 |
This PR serves as a proof of concept of how to support the SAS authentication scheme.
Required Emitter Changes to Generated Code
blob_url
instead ofendpoint
, which will now contain the fully qualified URI (including path parameters etc, query parameters, etc.)After the required emitter changes are done, the handwritten layer needs the following refactor:
new()
convenience constructors should not use the underlyingnew()
generated ctorfrom_blob_url
)