Skip to content
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

grpc: Add AWS IAM grpc credentials extension #7532

Merged
merged 15 commits into from
Aug 9, 2019
Merged

grpc: Add AWS IAM grpc credentials extension #7532

merged 15 commits into from
Aug 9, 2019

Conversation

lavignes
Copy link
Member

This is the PR that ties together all the pieces that me and my colleagues contributed to add an AWS IAM gRPC authentication extension (#5215).

Note that I put mattklein123 as a CODEOWNER. This was just a default to get the PR open. I'm not sure who else should be listed. My other team-member who helped repackage my code into previous commits for this feature is no longer working with App Mesh, so obviously can't commit to helping maintain the extension as well. :-)

Risk Level: Medium
Testing: Added integration tests. +Manually tested against the AWS App Mesh service.
Docs Changes: Added in protobuf.
Release Notes: Added line about added extension.
Fixes: #5215

Signed-off-by: LaVigne, Scott lavignes@amazon.com

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
@alyssawilk
Copy link
Contributor

@mattklein123 @envoyproxy/maintainers is anyone up for co-owning this one?

@lavignes
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #7532 (comment) was created by @lavignes.

see: more, trace.

@alyssawilk
Copy link
Contributor

At some point can you do a master merge, and move your release note to 1.12.0? Thanks!

@lavignes
Copy link
Member Author

@alyssawilk Yup! I just realized 1.11 was cut.

* origin/master:
  bazel: adapt cc_wraper.py to python3 (#7519)
  Remove exclusive tag from aws_metadata_fetcher_integration_test (#7545)
  Common: Consume, proxy and insert metadata from downstream to upstream (#5656)
  quiche: add EnvoyQuicPacketWriter with sendmsg to set source address (#7416)
  config dump, listeners are added to warming list if workers are not started yet (#7511)
  Fix crash in request hedging handling. (#7530)
  build: simplify cc_configure and static link for libc++ (#7329)
  tools: update our release scripts (#7493)
  test: hopefully deflaking lua test (#7542)
  release: kicking off 1.12 (#7544)
  release: bump to 1.11.0 (#7538)
  Bump c-ARES version to include important fixes. (#7539)
  bazel: Update to 0.28.0 (#7533)
  tests: remove redundant std::move() (#7535)
  docs: cleanup for 1.11 release (#7522)
  build: remove -lc++fs from linkopts (#7534)

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
@lavignes
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #7532 (comment) was created by @lavignes.

see: more, trace.

@mattklein123 mattklein123 self-assigned this Jul 13, 2019
@mattklein123
Copy link
Member

@lavignes i'm happy to review this one, but let's chat offline because if AWS is going to continue to submit extensions to the OSS repo we would appreciate one of you working to become a maintainer so that you can both help the project in a general sense and also help with self reviews. Thank you!

@mattklein123 mattklein123 added the no stalebot Disables stalebot from closing an issue label Jul 18, 2019
@mattklein123
Copy link
Member

Sorry am traveling will get to this next week. (Note also needs a master merge.)

…m_auth_plugin

* refs/remotes/origin/master: (41 commits)
  clang-tidy: modernize-deprecated-headers (#7626)
  ci: use local_memory_estimate (#7558)
  listener: keep ListenerFactoryContext small (#7528)
  build: remove ranlib workaround (#7620)
  server: skip lifecycle notifications on early envoy shutdown (#7585)
  load balancer: fix?: panic mode is disabled only when healthy_panic_threshold is 0% (#7478)
  ci: speed up release and mac run (#7612)
  stats: rename StatDataAllocator and friends, paying down some tech-debt. (#7591)
  test: remove unused function in lua_integration_test.cc (#7590)
  clang-tidy: bugprone-unused-raii (#7605)
  admin: Add socket options (#7562)
  fix coverage publish script (#7608)
  Set zipkin_service_name to LocalInfo::clusterName(). (#7354)
  test: separate AdsIntegrationTest to a library  (#7581)
  access log: Add AccessLoggers::Common::ImplBase base class to handle AccessLog::Filter evaluation (#7359)
  build: remove cc_configure and cc_wrapper (#7555)
  dispatcher: fix comment. (#7597)
  coverage: use LLVM and bazel native coverage (#7552)
  test: adding tests for set-cookie being handled correctly (#7541)
  Revert "ci: Enable bazel limited download flags (#7563)" (#7593)
  ...

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from a quick skim. A few small comments. Thank you!

/wait

docs/root/intro/version_history.rst Outdated Show resolved Hide resolved
source/extensions/grpc_credentials/aws_iam/config.cc Outdated Show resolved Hide resolved
source/extensions/grpc_credentials/aws_iam/config.h Outdated Show resolved Hide resolved
source/extensions/grpc_credentials/aws_iam/config.h Outdated Show resolved Hide resolved
@mattklein123 mattklein123 removed the no stalebot Disables stalebot from closing an issue label Jul 22, 2019
@stale
Copy link

stale bot commented Jul 30, 2019

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Jul 30, 2019
@lavignes
Copy link
Member Author

lavignes commented Aug 5, 2019

been on vacation for the last week or so. Will address comments tomorrow.

@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Aug 5, 2019
@dio
Copy link
Member

dio commented Aug 5, 2019

Thanks @lavignes 🙏

* origin/master: (112 commits)
  docs: fix incorrect comment (#7827)
  docs: link to GetEnvoy.io for pre-built binaries (#7814)
  ci: compile_time_options in Azure RBE (#7817)
  docs: fix minor style issue in tracing.rst (#7821)
  clang-tidy: modernize-loop-convert (#7790)
  Fix comment (#7816)
  config: do not finish initialization on stream disconnection (#7427)
  ci: roll forward build image (#7809)
  security: add @yanavlasov to Envoy security team. (#7811)
  zookeeper: parse server responses (#7574)
  clang-tidy modernize-use-nullptr (#7791)
  fuzz: remove header prefix from Bootstrap (#7803)
  bump abseil to latest version (#7802)
  docs: fix custom header example (#7806)
  ci: partially revert #7786 to fix post submit (#7807)
  Add flag when open file (#7445)
  ci: use small image and use remote_jdk (#7786)
  docs: RBE, Docker sandbox and bazel fixes (#7657)
  [xds] refactor config provider framework (#7704)
  Dependency: update PGV, opencensus-proto, nghttp2, msgpack-c, bazel-gazelle & cleanup (#7787)
  ...

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
* Update proto docs to mention AWS_REGION environment variable
* Add aws_iam.proto to extension docs
* fix nitpicks

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7532 was synchronize by lavignes.

see: more, trace.

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7532 was synchronize by lavignes.

see: more, trace.

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7532 was synchronize by lavignes.

see: more, trace.

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7532 was synchronize by lavignes.

see: more, trace.

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
* origin/master:
  codec: add metadata_not_supported_error to HTTP/1 codec stats (#7801)
  accesslog: Add buffering and flushing to gRPC access log (#7755)
  tracing: Update OpenCensus. (#7797)

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7532 was synchronize by lavignes.

see: more, trace.

Signed-off-by: LaVigne, Scott <lavignes@amazon.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7532 was synchronize by lavignes.

see: more, trace.

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7532 was synchronize by lavignes.

see: more, trace.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one small doc request. Thank you!

/wait

docs/root/intro/version_history.rst Outdated Show resolved Hide resolved
Signed-off-by: Scott LaVigne <lavignes@amazon.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #7532 was synchronize by lavignes.

see: more, trace.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

xDS gRPC credentials for AWS IAM
5 participants