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

Remote peering #81

Merged
merged 10 commits into from
Oct 11, 2024
Merged

Remote peering #81

merged 10 commits into from
Oct 11, 2024

Conversation

alexwood
Copy link
Contributor

@alexwood alexwood commented Oct 7, 2024

Support remote peering in kaleido platform terraform

Adds

  • updates to networks to enable init mode and data to be input and referenced from originator to other network resource
    • to achieve this it added a new data type for bootstrap data. which uses the initdata api on the network and and be referenced from another network. See the example tf module to see how that is used. This is to overcome the two phases of network ready vs initialised.
    • this required having the common data object support apiRequest function.
  • adds authenticators to terraform
  • updates services to support connectivity output and chaining into authenticators
  • adds examples for making a network split over multiple envs/accounts

Also had to add additional read in service Create so that the connectivity status got read again (after wait for ready completed) or else it would not be set.

There is an operator PR to go along with this https://github.com/kaleido-io/kap-operator/pull/1858

Copy link
Contributor

@onelapahead onelapahead left a comment

Choose a reason for hiding this comment

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

This looks fantastic, nicely done @alexwood. No real significant feedback.

Perhaps while the Authenticators API is still togglable/unstable, is there a way to annotate a resource in TF to indicate its experimental ? Just don't want this Terraform release to be our formal commitment to the API until we officially make it stable in future platform releases.

kaleido/platform/common.go Show resolved Hide resolved
kaleido/platform/service.go Outdated Show resolved Hide resolved
Comment on lines +157 to +160
file_sets = data.kaleido_platform_network_bootstrap_data.net_og_bootstrap.bootstrap_files != null ? {
init = data.kaleido_platform_network_bootstrap_data.net_og_bootstrap.bootstrap_files
} : {}
init_files = data.kaleido_platform_network_bootstrap_data.net_og_bootstrap.bootstrap_files != null ? "init" : null
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need the conditional if we have a proper dependency on the data object here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah good point. I will try removing it. I added this when I was trying to make it work with one resource before I added the datasource

} : {}
init_files = data.kaleido_platform_network_bootstrap_data.net_og_bootstrap.bootstrap_files != null ? "init" : null
config_json = jsonencode({})
depends_on = [kaleido_platform_network.net_og, kaleido_platform_service.bns_signer_net_og, data.kaleido_platform_network_bootstrap_data.net_og_bootstrap]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: depends_on is needed to make explicit dependencies btwn objects when a clear dependency doesn't exist. So I think only the service.bns_signer... is really needed here bc the data is directly referenced in the file sets / init files, and the data depends on the network (and the service).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah also true. I will remove and retest it

Comment on lines +216 to +240
// Authenticators
resource "kaleido_platform_authenticator" "net_sec_authenticator" {
provider = kaleido.secondary
type = "Permitted"
name = "${var.secondary_name}_auth"
environment = kaleido_platform_environment.env_sec.id
network = kaleido_platform_network.net_sec.id
zone = var.secondary_peer_network_dz
conn = var.secondary_peer_network_connection
permitted_json = jsonencode({ peers = [ for peer in resource.kaleido_platform_service.bns_peer_net_og : jsondecode(peer.connectivity_json) ] })
depends_on = [kaleido_platform_network.net_sec, kaleido_platform_service.bns_peer_net_og]
}


resource "kaleido_platform_authenticator" "net_og_authenticator" {
provider = kaleido.originator
type = "Permitted"
name = "${var.originator_name}_auth"
environment = kaleido_platform_environment.env_og.id
network = kaleido_platform_network.net_og.id
zone = var.originator_peer_network_dz
conn = var.originator_peer_network_connection
permitted_json = jsonencode({ peers = [ for peer in resource.kaleido_platform_service.bns_peer_net_sec : jsondecode(peer.connectivity_json) ] })
depends_on = [kaleido_platform_network.net_og, kaleido_platform_service.bns_peer_net_sec]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

awesome!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah nice right.

@alexwood
Copy link
Contributor Author

alexwood commented Oct 8, 2024

This looks fantastic, nicely done @alexwood. No real significant feedback.

Perhaps while the Authenticators API is still togglable/unstable, is there a way to annotate a resource in TF to indicate its experimental ? Just don't want this Terraform release to be our formal commitment to the API until we officially make it stable in future platform releases.

yes this is a good point.

@calbritt calbritt merged commit 8844ec9 into v1.1 Oct 11, 2024
1 check passed
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.

3 participants