microsoft / CCF

Confidential Consortium Framework
https://microsoft.github.io/CCF/
Apache License 2.0
766 stars 207 forks source link

Add JWT support to Perf Client #2096

Closed sidmore closed 3 years ago

sidmore commented 3 years ago

rpc_tls_client.h changes:

std::vector<uint8_t> gen_http_request_internal(
    const std::string& method,
    const CBuffer params,
    const std::string& content_type,
    llhttp_method verb,
    const char* jwt = nullptr)

perf_client.h changes:

app.add_option("--jwt", jwt)
        ->required(false)
eddyashton commented 3 years ago

I don't see where the std::string jwt in perf_client.h is converted to a char const* (.c_str()) and passed to any calls to rpc_tls_client.h - what am I missing?

sidmore commented 3 years ago

I don't see where the std::string jwt in perf_client.h is converted to a char const* (.c_str()) and passed to any calls to rpc_tls_client.h - what am I missing?

actually there are couple of methods....with gen_request being called...was not sure which all need to be updated. I will update the code accordingly.

sidmore commented 3 years ago

I don't see where the std::string jwt in perf_client.h is converted to a char const* (.c_str()) and passed to any calls to rpc_tls_client.h - what am I missing?

actually there are couple of methods....with gen_request being called...was not sure which all need to be updated. I will update the code accordingly.

I have implemented the change to Pass JWT from argument to the Perfclient.

sidmore commented 3 years ago

I don't see where the std::string jwt in perf_client.h is converted to a char const* (.c_str()) and passed to any calls to rpc_tls_client.h - what am I missing?

actually there are couple of methods....with gen_request being called...was not sure which all need to be updated. I will update the code accordingly.

I have implemented the change to Pass JWT from argument to the Perfclient.

achamayou commented 3 years ago

@sidmore thank you for making this change, it's an excellent idea to add a test for this! Do you think we could add an instance of the perf_test running this in the CI, so it could be tracked along with other configs by cimetrics? What authority/token combination do you use for testing at the moment?

achamayou commented 3 years ago

/azp run

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 2 pipeline(s).
ccf-bot commented 3 years ago

sidmore-patch-1@17810 aka 20210121.18 vs master ewma over 50 builds from 17155 to 17795 images

achamayou commented 3 years ago

Could you also run scripts/ci-checks.sh -f? That should automatically reformat your changes so they comply with the repo guidelines (you will need to commit the result). To make sure it's worked, you can run scripts/ci-checks.sh, there may be some rare cases where the auto formatting fails, but it's generally quite good.

sidmore commented 3 years ago

Could you also run scripts/ci-checks.sh -f? That should automatically reformat your changes so they comply with the repo guidelines (you will need to commit the result). To make sure it's worked, you can run scripts/ci-checks.sh, there may be some rare cases where the auto formatting fails, but it's generally quite good.

yup @achamayou will do. Thanks for the pointers.

achamayou commented 3 years ago

/azp run

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 2 pipeline(s).