Repository navigation
feat(plugin) Implement AWS Dynamic Host Catalog Cross-Account Discovery - #95
Merged
Merged
Conversation
matt-romanelli
marked this pull request as ready for review
September 25, 2026 17:28
delainerogers
approved these changes
Sep 25, 2026
…t dynamic host catalogs (#84)
…et handlers (ICU-19979) (#87) * temporary commit * feat(host): replace dryRunValidation with checkHosts in catalog and set handlers * refactor(host): clean up comments and fix typos in dry-run validation blocks * refactor(host): address PR feedback on dry-run validation blocks * refactor(host): remove dead dryRunValidation function and its tests * refactor(host): extract dry run error message as a constant * refactor(host): propagate BadRequestStatusf from buildFilters through buildDescribeInstancesInput --------- Co-authored-by: ryanflynn9299 <ryan.flynn001@gmail.com>
…ic (#91) * feat(cli) Update error handling for checkHosts, Target EC2 client logic * feat(cli) Add UnauthorizedOperation handling to errors.go
* Fix role arn fallthrough bug * add unit test * bump awsutil version * fix tests
…89) * feat(tests) Add Terraform testing to support Cross-account discovery **Problem** * To support cross-account DHC discovery, we need to add terraform configurations and tests to test against live AWS data. * However, there was nothing in place to support any AssumeRole testing, irrespective of cross-account. **Solution** * Added a new terraform file, iam_assume_role.tf, to contain the new terraform configurations for single-account AssumeRole and cross-role * Added a second terraform file, iam_cross_account.tf, to contain the new terraform configurations for cross-account * Refactored how the tests are organized. The previous static test cases used a system where a separate function was created for each test. This was moved to a dedicated file, e2e_host_static_test.go. * For AssumeRole tests (single account, cross role, and cross account), this system wasn't ideal since there are so many test cases and they are all extremely similar. A table driven system was better that passed in a set of variables to shared test helpers. A dedicated file, e2e_host_assumerole_test.go, was created to contain the testing infrastructure for AssumeRole tests. * e2e_host_test.go remains the main test runner that then calls e2e_host_assumerole_test.go and e2e_host_static_test.go * The terraform tests work by setting environment variables for the AWS account that you want to configure the terraform for. The existing setup worked for single-account and cross-role. However, to support cross-account, support had to be added for a second set of credentials. The second set of environment variables can now be set with the TARGET_ prefix. Some configuration in testing.go had to be added to support creating/destroying multiple terraform setups. **Testing** * The single account tests pass as expected. However, a bug was discovered during this process that will be addressed later. * The cross-role and cross-account tests failed as expected because this featured has not yet been delivered. * It was noticed that some of the existing static tests were flaky. The problem and fix were identified and will be addressed later.
* bug(go) gRPC status codes were getting overwritten by the handlers **Problem** * If the gRPC handlers received an error with a gRPC status code from checkHosts, it got overwritten by a plain error value. **Solution** * Added a check to each checkHosts call that passes on the error received (with its gRPC status) **Testing** * Added new unit tests for when the handlers receive an error from checkHosts with a status code. * Existing unit tests and terraform tests passed
matt-romanelli
force-pushed
the
llb-icu-19968
branch
from
September 28, 2026 17:05
6c21b5c to
bed5cf8
Compare
ryanflynn9299
approved these changes
Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RFC: https://github.com/hashicorp/boundary-enterprise/blob/main/docs/rfcs/icu-2018-cross-account-dhc-discovery.md
Testing
PCI review checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.