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

Assign a Network Address to a Target #2613

Merged
merged 10 commits into from
Jan 9, 2023
Merged

Assign a Network Address to a Target #2613

merged 10 commits into from
Jan 9, 2023

Conversation

hugoghx
Copy link
Collaborator

@hugoghx hugoghx commented Nov 14, 2022

Summary:

This branch contains a set of commits that enable a Boundary Target to be directly configured with an address to create a session against, instead of having to create Host Catalogs, Host Sources and Hosts.

How to use (CLI):

Creating a tcp target with an address:

boundary targets create tcp -scope-id p_1234567890 -name test -default-port 22 -address "localhost"

Updating a tcp target with an address:

boundary targets update tcp -id ttcp_1234567890 -address "localhost"

Removing an address from a target:

boundary targets update tcp -id ttcp_1234567890 -address "null"

Notes:

  • The address field can be a dns name or an ipv4 endpoint
  • When deleting an address from a target, all active sessions using the target will be canceled. Changing the value of the address on a given target will not cancel any sessions.
  • When adding or setting a host source to a target that has a network address directly configured to it, an error should be returned
  • When setting a network address directly to a target that has a host source association, an error should be returned

Example of the error:

Error from controller when performing add-host-sources on target

Error information:
  Kind:                FailedPrecondition
  Message:             target.(Repository).AddTargetHostSources: error creating sets: db.DoTx:
  target.(Repository).AddTargetHostSources: unable to add host sources because a network address is directly assigned to the
  given target: integrity violation: error #409
  Status:              400
  context:             Error from controller when performing add-host-sources on target

Note: grpc-gateway maps Failed Precondition errors into http bad request errors because this deliberately doesn't translate to the similarly named '412 Precondition Failed' HTTP response status.

Concerns:

  • database migration test 59_01 was deleted because of the breaking changes implemented into the domain code. which includes interacting with the target_address table. This test invoked functions in the domain code, such as LookupTarget & UpdateTarget, which now interacts with the target_address table. Because the target_address table does not exist until migration id 70, a postgress error is returned notifying the user that the table does not exist.

Important: We also have Terraform changes to implement client support for a Target using an address. Merging and releasing a provider with those changes is blocked until a Boundary version with the changes in this PR is released. After that, we can update the module imports on the PR and create a new Terraform provider release there.

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

Should this be targeting the release branch? I assume not.

@hugoghx
Copy link
Collaborator Author

hugoghx commented Nov 14, 2022

Should this be targeting the release branch? I assume not.

From what I know, that's how we're doing things now. We're not targeting main anymore, instead targeting release branches and then cherry-picking the commits into main when the PR is merged

@johanbrandhorst
Copy link
Collaborator

Should this be targeting the release branch? I assume not.

From what I know, that's how we're doing things now. We're not targeting main anymore, instead targeting release branches and then cherry-picking the commits into main when the PR is merged

Uh, I thought that was the case until we were doing a release, but I suppose you could be onto something, good point!

@hugoghx hugoghx added this to the 0.12.x milestone Nov 14, 2022
@johanbrandhorst
Copy link
Collaborator

Lets just make sure we don't merge this until 0.11.1 is out the door 😁

@ddebko ddebko requested a review from tmessi January 3, 2023 14:10
@hugoghx hugoghx marked this pull request as ready for review January 3, 2023 15:02
Copy link
Collaborator

@ddebko ddebko left a comment

Choose a reason for hiding this comment

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

Need to update the changelog

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

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

Looks like the CHANGELOG needs an update too 😄

internal/errors/is.go Outdated Show resolved Hide resolved
internal/daemon/controller/handlers/errors.go Show resolved Hide resolved
internal/tests/api/targets/target_test.go Show resolved Hide resolved
internal/tests/cli/boundary/target.bats Show resolved Hide resolved
@hugoghx

This comment was marked as resolved.

internal/tests/api/targets/target_test.go Outdated Show resolved Hide resolved
internal/tests/api/targets/target_test.go Show resolved Hide resolved
@hugoghx
Copy link
Collaborator Author

hugoghx commented Jan 5, 2023

Rebasing off of main and squashing everything back into the original 10 commits now

Hugo Vieira and others added 10 commits January 9, 2023 15:39
This commit adds CLI support for addresses to be attached directly to a
Target, for all current types of Target (ssh and tcp).

It also clarifies the relevant documentation regarding the autogen of
API option functionality.
Targets are not guaranteed to have Host Ids anymore, so we now use
Target Id when a Host Id is not present.
This test uses the target domain repository code to test the ingress
& egress worker filter. With our changes we have updated the target
domain repository logic to fetch the target address. When this migration
test is invoked, the target_address table does not exist and the domain
code will always return an error making this test invalid.
Creates a target using an address for boundary dev and when first
init-ing boundary (boundary database init).

It also makes this newly created target the default one
(ttcp_1234567890). The target using host sources is now a secondary
target (ttcp_0987654321).
Co-authored-by: Hugo <hugoamvieira@users.noreply.github.com>
@hashicorp hashicorp deleted a comment from vercel bot Jan 9, 2023
@hugoghx hugoghx merged commit 1fb9605 into main Jan 9, 2023
@hugoghx hugoghx deleted the llb-target-address branch January 9, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants