-
Notifications
You must be signed in to change notification settings - Fork 0
Add pluggable auth support for external account creds #1
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
Conversation
@@ -0,0 +1,130 @@ | |||
|
|||
// | |||
// Copyright 2020 gRPC authors. |
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.
super nit: 2023
#include "src/core/lib/json/json_reader.h" | ||
#include "src/core/lib/json/json_writer.h" | ||
#include "src/core/lib/security/credentials/credentials.h" | ||
#include "src/core/lib/uri/uri_parser.h" |
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.
nit: Do we need all these includes? Wondering about the uri_parser in particular
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.
We need not worry about includes as the "include what you use" iwyu tool will show us the unused and missed includes during build.
I've modified the includes for now by running iwyu locally, which isn't that reliable.
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.
oh great, no worries then
if (executable_it != executable_json.object().end()) { | ||
if (!absl::SimpleAtoi(executable_it->second.string(), | ||
&executable_timeout_ms_)) { | ||
*error = GRPC_ERROR_CREATE("timeout_millis field must be a number"); |
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.
nit: add period at the end of the error message
This adds pre-built library for aarch64 linux, will help improve the install speed and avoid building environment issues at customer side. @apolcyn @jtattermusch Can you help build and push the new rake compiler image? Will update the tag and hash after the image is available Manually tested locally: ``` uname -a Linux u20 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux ``` ``` time gem install /work/ruby/grpc/pkg/grpc-1.56.0.dev-aarch64-linux.gem Successfully installed grpc-1.56.0.dev-aarch64-linux Parsing documentation for grpc-1.56.0.dev-aarch64-linux Installing ri documentation for grpc-1.56.0.dev-aarch64-linux Done installing documentation for grpc after 0 seconds 1 gem installed real 0m22.794s user 0m17.268s sys 0m5.156s ``` ``` ruby greeter_server.rb & [1] 319 ruby greeter_client.rb "Greeting: Hello world" ``` Fixes: grpc#31855 grpc#29489
This adds pre-built library for aarch64 linux, will help improve the install speed and avoid building environment issues at customer side. @apolcyn @jtattermusch Can you help build and push the new rake compiler image? Will update the tag and hash after the image is available Manually tested locally: ``` uname -a Linux u20 5.15.49-linuxkit #1 SMP PREEMPT Tue Sep 13 07:51:32 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux ``` ``` time gem install /work/ruby/grpc/pkg/grpc-1.56.0.dev-aarch64-linux.gem Successfully installed grpc-1.56.0.dev-aarch64-linux Parsing documentation for grpc-1.56.0.dev-aarch64-linux Installing ri documentation for grpc-1.56.0.dev-aarch64-linux Done installing documentation for grpc after 0 seconds 1 gem installed real 0m22.794s user 0m17.268s sys 0m5.156s ``` ``` ruby greeter_server.rb & [1] 319 ruby greeter_client.rb "Greeting: Hello world" ``` Fixes: grpc#31855 grpc#29489
815c8eb
to
803d60c
Compare
|
||
TEST( | ||
CredentialsTest, | ||
TestPluggableAuthExternalAccountCredentialsCreateFailureExecutableTimeoutLessThanMin) { |
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.
nit: less than 5 seconds?
* Add pluggable auth support for external account creds * Add tests for pluggable auth - ADC * Update test name - credentials_test.cc
* Add pluggable auth support for external account creds * Add tests for pluggable auth - ADC * Update test name - credentials_test.cc
* Add pluggable auth support for external account creds * Add tests for pluggable auth - ADC * Update test name - credentials_test.cc
* Add pluggable auth support for external account creds * Add tests for pluggable auth - ADC * Update test name - credentials_test.cc
* Add pluggable auth support for external account creds * Add tests for pluggable auth - ADC * Update test name - credentials_test.cc
* Add pluggable auth support for external account creds * Add tests for pluggable auth - ADC * Update test name - credentials_test.cc
* Add pluggable auth support for external account creds * Add tests for pluggable auth - ADC * Update test name - credentials_test.cc
No description provided.