-
Notifications
You must be signed in to change notification settings - Fork 251
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
add customer encryption key for upload and download blob request #1304
Conversation
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
if let Some(cpk_info) = &this.encryption_key { | ||
headers.add_ref(cpk_info); | ||
} |
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.
Can you discuss this choice, instead of using
if let Some(cpk_info) = &this.encryption_key { | |
headers.add_ref(cpk_info); | |
} | |
headers.add(this.encryption_key); |
This is made possible by the following:
azure-sdk-for-rust/sdk/core/src/headers/mod.rs
Lines 25 to 27 in be63d1e
impl<T> AsHeaders for Option<T> | |
where | |
T: Header, |
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.
Thank you for your review.
The struct CPKInfo
contains 3 headers: EncryptionKey
, EncryptionKeySha256
and EncryptionAlgorithm
. That's because once the EncryptionKey
is specified, the other 2 headers must be provided. (from azure-sdk-for-go)
The this.encryption_key
is wrapped by Option
, for which I find there has been an implement:
// ./sdk/core/src/headers/mod.rs:L25
impl<T> AsHeaders for Option<T>
where
T: Header,
{
type Iter = std::option::IntoIter<(HeaderName, HeaderValue)>;
fn as_headers(&self) -> Self::Iter {
match self {
Some(h) => h.as_headers(),
None => None.into_iter(),
}
}
}
The T has a trait bound Header
, which has the function name
and value
. But the trait Header
can not be implemented by CPKInfo
. So I cannot directly use headers.add(this.encryption_key);
.
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.
@demoray PTAL
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.
Maybe we can just specify the AsHeaders::Iter
to expand the scope. See the code as followes:
impl<T> AsHeaders for Option<T>
where
T: AsHeaders<Iter = std::option::IntoIter<(HeaderName, HeaderValue)>>,
{
type Iter = T::Iter;
fn as_headers(&self) -> Self::Iter {
match self {
Some(h) => h.as_headers(),
None => None.into_iter(),
}
}
}
Signed-off-by: Leavrth <jianjun.liao@outlook.com>
In the commit c18fbdd, I modify the |
support
x-ms-encryption-algorithm
,x-ms-encryption-key
andx-ms-encryption-key-sha256
for upload/download blobs in headers.