-
Notifications
You must be signed in to change notification settings - Fork 78
add terraform support for Teleport servers #1019
Conversation
terraform/example/server.tf.example
Outdated
version = "v2" | ||
sub_kind = "openssh" | ||
metadata = { | ||
name = "test" |
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.
Did you test this?
I was testing for the openssh-ec2-ice
nodes and it doesn't work if we give it a non-uuid string.
An example that works:
resource "random_uuid" "ec2-random-name" {
}
resource "teleport_server" "my-dev-server" {
version = "v2"
sub_kind = "openssh-ec2-ice"
metadata = {
name = "${random_uuid.ec2-random-name.result}"
}
spec = {
addr = "<ec2 private ip>:22"
hostname = "my-dev-server"
cloud_metadata = {
aws = {
account_id = "<account-id>"
instance_id = "<instance-id>"
region = "<region>"
vpc_id = "<vpc-id>"
integration = "<awsoidc-integration-used-for-credentials>"
subnet_id = "<subnet-id>"
}
}
}
}
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.
Did you test this?
I was testing for the openssh-ec2-ice nodes and it doesn't work if we give it a non-uuid string.
Do you mean against a real AWS account? No, I only checked that Teleport was accepting the resource in the operator. I don't understand why the eice server name must be a uuid 🤔
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.
It's a bit buried, but it is required to be an uuid because of this
https://github.com/gravitational/teleport/blob/a39c21716b40518f0433b619bb9828d127bd6b77/api/utils/route.go#L41
When I implemented EC2 ICE mode, I closely followed the OpenSSH/Agentless mode.
And I ended up making the same assumptions.
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.
I guess you shouldn't set the name. Instead, you should set the hostid.
Look at the condition here:
_, err := uuid.Parse(host)
dialByID := err == nil || aws.IsEC2NodeID(host)
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.
I updated the TF example to not set the name, and added tests for both named and nameless openssh servers.
terraform/gen/plural_resource.go.tpl
Outdated
|
||
{{- if .ForceSetKind }} | ||
{{.VarName}}Resource.Kind = {{.ForceSetKind}} | ||
{{- end}} | ||
|
||
{{if .HasCheckAndSetDefaults -}} | ||
err = {{.VarName}}Resource.CheckAndSetDefaults() | ||
if err != nil { | ||
resp.Diagnostics.Append(diagFromWrappedErr("Error setting {{.Name}} defaults", trace.Wrap(err), "{{.Kind}}")) | ||
return | ||
} | ||
{{- end}} | ||
|
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.
CheckAndSetDefaults
and kind setting were moved before
id := {{.VarName}}Resource.Metadata.Name
because the name itself can be changed during this step (e.g. nameless openssh servers).
Co-authored-by: Marco André Dinis <[email protected]>
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.
Just left a comment
Co-authored-by: Marco André Dinis <[email protected]>
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.
Bot.
This PR implements: gravitational/teleport#37624