feat: add support for the user_data variable #916
Conversation
|
/run pipeline |
|
/run pipeline |
variables.tf
Outdated
| @@ -289,6 +289,20 @@ variable "ssh_keys" { | |||
| } | |||
| } | |||
|
|
|||
| variable "user_data" { | |||
There was a problem hiding this comment.
We already have a user_data field in the vsi variable below, so I do not think that we should have a separate variable here.
Is it possible to assign the correct user_data to the correct vsi object in the pattern instead? That way the override still works and we do not have duplicate variables.
| @@ -40,6 +40,14 @@ variable "resource_tags" { | |||
| default = [] | |||
| } | |||
|
|
|||
| variable "user_data" { | |||
There was a problem hiding this comment.
This will not work well in quickstart, as we are hard-coding (in the override) the VSI configurations. If we still want to support user_data in quickstart, we should redesign this pattern a little bit so that user supplied user_data can be inserted into the override json. This does mean a bit of a redesign of this pattern where we would move the override from an input variable with default, and instead have it in the main.tf with variable placeholders. See the roks-quickstart/main.tf for an example.
| @@ -146,6 +146,14 @@ variable "vsi_per_subnet" { | |||
| default = 1 | |||
| } | |||
|
|
|||
| variable "user_data" { | |||
There was a problem hiding this comment.
The complex validation that you added at the root level really belongs here at the solution level. If you are going to validate that the map keys here match entries in the vpcs list, then you want to do it here at the entry point, and maybe also at the module level of this solution.
Also the description needs to be more detailed explaining to the user what this map is supposed to be (with the key being a VPC name from the vpcs variable, and the data being the yaml).
There is something else to mention here: the user_data is usually in yaml format. We need to test that yaml inside an object still works correctly from all deployement types (terraform CLI, schematics, projects) and to make sure the yaml is valid by time it is applied to the VSI.
patterns/vsi/module/main.tf
Outdated
| @@ -34,6 +34,7 @@ module "landing_zone" { | |||
| teleport_config_data = local.env.teleport_config | |||
| teleport_vsi = local.env.teleport_vsi | |||
| vpc_placement_groups = local.env.vpc_placement_groups | |||
| user_data = var.user_data | |||
There was a problem hiding this comment.
As stated in the root level variables file, I don't think this should be its own separate input variable, as this property already exists within the VSI input map.
Instead, your new input variable for the pattern should be used to set the existing user_data property for the individual VSI entries in that map, somewhere in the config.tf: https://github.com/terraform-ibm-modules/terraform-ibm-landing-zone/blob/main/patterns/vsi/module/config.tf#L85
dynamic_values.tf
Outdated
| @@ -33,6 +33,7 @@ module "dynamic_values" { | |||
| skip_kms_kube_s2s_auth_policy = var.skip_kms_kube_s2s_auth_policy | |||
| skip_all_s2s_auth_policies = var.skip_all_s2s_auth_policies | |||
| atracker_cos_bucket = var.atracker.add_route == true ? var.atracker.collector_bucket_name : null | |||
| user_data = var.user_data | |||
There was a problem hiding this comment.
These changes will not be needed if you assign the user_data to the VSI object out in the pattern level as suggested.
|
/run pipeline |
Description
Issue: #863
Release required?
x.x.X)x.X.x)X.x.x)Release notes content
Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
Checklist for reviewers
For mergers