Comments (3)
Hey sorry for the delayed responses.
Can you provide a stack trace? You can enable that feature by setting the env variable RUST_BACKTRACE
to 1
.
I tried to reproduce it unfortunately I don't have a Google account, so I used a different OIDC Provider to test it.
from vaultrs.
Error("EOF while parsing a value", line: 1, column: 0),
This error means that's the HTTP response had an empty body and we were expecting one. This can happen for methods that can both create and update, like the one you have used.
The empty body case can happen when you are in updating mode but the parameters you provided don't have new information, so there is no real update. Sometimes even if there is an update the API could return with an empty body
In that case we are already ignoring the result (vaultrs::auth::oidc::role::set
returns unit ()
) changing the implementation of the function from api::exec_with_empty_result
to api::exec_with_empty
should do the work.
This does raise a bigger concern about how we should handle this create/update function in the library. I've come up with 4 solutions:
- catch the error in the calling function and return a clearer error (like i did in #77) . But can catch other errors as well.
Here a sample of the code
api::exec_with_result(client, endpoint)
.await
.map_err(|err| {
// In the case the response as an empty HTTP Body
if matches!(
err,
ClientError::RestClientError {
source: rustify::errors::ClientError::ResponseParseError { .. }
}
) {
return ClientError::InvalidUpdateParameter;
}
err
})
- systematically return
api::exec_with_empty
in that case. But sometimes the information returned is useful, so the user would have to issue both a create and then a read. This gets worse when the create method returns an id, where sometimes we have to do a list (to get the id) and then a read. - have an hybrid method
execute_with_result_or_empty
. I've also experimented this workaround in #77 . It adds an option to the return type, so we are complexifying the most common use cases, for something that is should be rare (update with wrong parameters or an update that return an empty body). As nothing is really specified for the return type of the API, that's maybe the good representation
Here was my working implementation
/// Executes an [Endpoint] and if it returns an empty HTTP response, it returns `None`, otherwise it returns the result.
///
/// This function is mostly useful for endpoints that can both *update* and *create*.
/// The *create* operation will return a result and the *update* one not.
///
/// If there is a result, a few operations are performed on it:
///
/// * Any potential API error responses from the execution are searched for and,
/// if found, converted to a [ClientError::APIError]
/// * All other errors are mapped from [rustify::errors::ClientError] to
/// [ClientError::RestClientError]
/// * An empty content body from the execution is rejected and a
/// [ClientError::ResponseEmptyError] is returned instead
/// * The enclosing [EndpointResult] is stripped off and any warnings found in
/// the result are logged
/// * An empty `data` field in the [EndpointResult] is rejected and a
/// [ClientError::ResponseDataEmptyError] is returned instead
/// * The value from the enclosed `data` field is returned along with any
/// propagated errors.
pub async fn exec_with_result_or_empty<E>(
client: &impl Client,
endpoint: E,
) -> Result<Option<E::Response>, ClientError>
where
E: Endpoint,
{
info!(
"Executing {} and expecting maybe a response",
endpoint.path()
);
endpoint
.with_middleware(client.middle())
.exec(client.http())
.await
.map(|res| {
if res.response.body().is_empty() {
Ok(None)
} else {
res.wrap::<EndpointResult<_>>()
.map_err(ClientError::from)
.map(strip)?
.map(Some)
.ok_or(ClientError::ResponseDataEmptyError)
}
})
.map_err(parse_err)?
}
- Separate the create and the update functionality in 2 functions, even if they talk to the same endpoint. The create one will always return data (
api::exec_with_result
) and we are could simply ignore the return of the update which is less useful in that case (api::exec_with_empty
). The disadvantage is we are not one one with the API but I think it's the more idiomatic
In all cases, we should definitely look how the go implementation (or other SDK) is dealing with such cases
from vaultrs.
@stormshield-gt thanks for the suggestions and the explanation of the error.
- I don't like this one because it blots the calling functions.
- I also don't like this option, because the reason you already mention.
- If we add a new
exec_with_result_or_empty
fiction to the API it can be hard to determine when to useexec_with_result
andexec_with_result_or_empty
. In the vault documentation they don't mention if they return an empty response or not. We should check if we can solve this in the currentapi::exec_with_result
- also cause a lot of bloated code.
The reason this happens in your PR is that when the API call to identity/entity
contains an id
vault always returns a response with the return code 204
which stands for No Content
. Even if you changed something with it.
You can see this in the following examples
curl -v -X POST -H "X-Vault-Request: true" -H "X-Vault-Token: $(vault print token)" -d '{"id":"f79073f1-35b9-a65e-d124-8efc5b0ed160","name":"test4"}' http://127.0.0.1:8200/v1/identity/entity
Note: Unnecessary use of -X or --request, POST is already inferred.
* Trying 127.0.0.1:8200...
* Connected to 127.0.0.1 (127.0.0.1) port 8200
> POST /v1/identity/entity HTTP/1.1
> Host: 127.0.0.1:8200
> User-Agent: curl/8.6.0
> Accept: */*
> X-Vault-Request: true
> X-Vault-Token: root
> Content-Length: 60
> Content-Type: application/x-www-form-urlencoded
>
< HTTP/1.1 204 No Content
< Cache-Control: no-store
< Content-Type: application/json
< Strict-Transport-Security: max-age=31536000; includeSubDomains
< Date: Sat, 09 Mar 2024 13:58:12 GMT
<
* Connection #0 to host 127.0.0.1 left intact
curl -v -X POST -H "X-Vault-Request: true" -H "X-Vault-Token: $(vault print token)" -d '{"id":"f79073f1-35b9-a65e-d124-8efc5b0ed160","name":"test5"}' http://127.0.0.1:8200/v1/identity/entity
Note: Unnecessary use of -X or --request, POST is already inferred.
* Trying 127.0.0.1:8200...
* Connected to 127.0.0.1 (127.0.0.1) port 8200
> POST /v1/identity/entity HTTP/1.1
> Host: 127.0.0.1:8200
> User-Agent: curl/8.6.0
> Accept: */*
> X-Vault-Request: true
> X-Vault-Token: root
> Content-Length: 60
> Content-Type: application/x-www-form-urlencoded
>
< HTTP/1.1 204 No Content
< Cache-Control: no-store
< Content-Type: application/json
< Strict-Transport-Security: max-age=31536000; includeSubDomains
< Date: Sat, 09 Mar 2024 13:58:18 GMT
<
* Connection #0 to host 127.0.0.1 left intact
But Let's move the discussion to about this to you're PR #77
from vaultrs.
Related Issues (20)
- RUSTSEC-2021-0139: ansi_term is Unmaintained
- Set api call fails when kv versioning is not enabled and a new key is created HOT 1
- Allow client certificates for mTLS connections
- New crate version with last modifications HOT 2
- URL-Encoding in Path is applied twice
- Unexpected behavior from login client/api interactions
- RUSTSEC-2023-0052: webpki: CPU denial of service in certificate path building
- Use JWT auth HOT 1
- X-Vault-Request should be a default header
- Add support for Vault issuers
- [BUG]: Can't create derived key
- [META] Looking for maintainer help HOT 5
- Considering migrating to testcontainers HOT 3
- v0.7.1 build fails for native-tls HOT 2
- There is room for improvement for the logs HOT 1
- Incompatible types on v0.6+ for Ubuntu 20.04 and 22.04 HOT 3
- Add support for asymmetric keys to the transit client HOT 2
- SSL cert issue with rustify >0.5.3 HOT 1
- Any plans to add Okta auth method? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from vaultrs.