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

Create a custom_data string with Terraform Ouputs #667

Merged
merged 3 commits into from
Sep 15, 2021

Conversation

abdulrabbani00
Copy link
Contributor

@abdulrabbani00 abdulrabbani00 commented Sep 1, 2021

Overview

Currently, users have the ability to add custom_data to a Linux VM in two ways:

  1. Passing a local path to a script.
  2. Directly inserting code in the tfvars

This allows users to manually enter custom_data for these machines. But it does not allow users to dynamically build a string for the custom_data from Terraform outputs.

Our Use Case

In our use case, we need to set the following string as the custom_data, assuming a storage account, file share, and directory exist.:

storage-account=stname, access-key=some-secret, file-share=myfileshare, share-directory=testdirectory

Implementation

The implementation I propose is to use a locals block that contains a map called dynamic_custom_data. Here, any developer would be able to add their condition. The key would be used to reference their condition, and the value would contain the dynamic string created from Terraform outputs for each server.

Our Example

The example (and use case) of this PR is the following:
*

  dynamic_custom_data = {
    palo_alto_connection_string = {
      for item in var.settings.virtual_machine_settings: 
        item.name => try(base64encode("storage-account=${var.storage_accounts[var.client_config.landingzone_key][item.palo_alto_connection_string.storage_account].name}, access-key=${var.storage_accounts[var.client_config.landingzone_key][item.palo_alto_connection_string.storage_account].primary_access_key}, file-share=${var.storage_accounts[var.client_config.landingzone_key][item.palo_alto_connection_string.storage_account].file_share[item.palo_alto_connection_string.file_share].name}, share-directory=${var.storage_accounts[var.client_config.landingzone_key][item.palo_alto_connection_string.storage_account].file_share[item.palo_alto_connection_string.file_share].file_share_directories[item.palo_alto_connection_string.file_share_directory].name}"), null)
    }
  }
  • custom_data defined by the user in .tfvars: custom_data: palo_alto_connection_string
  • A map called palo_alto_connection_string:
        palo_alto_connection_string = {
          storage_account = "sa1"
          file_share = "share1"
          file_share_directory = "dir1"        

The following occurs:

  1. The user sets the custom_data to a key found in dynamic_custom_data -- palo_alto_connection_string.
  2. The user sets all the attributes that will be needed to construct the custom_data object within dynamic_custom_data.
    a. palo_alto_connection_string = {...}
  3. TF loops through each VM's settings and checks to see if custom_data exists.
  4. If it exists, it checks to see if the custom_data value is a key in local.dynamic_custom_data. If it is, it will construct the value of custom_data using the map that has a key with the same value as custom_data. -- var.palo_alto_connection_string
  5. If the key does not exist in dynamic_custom_data. It will attempt to search for a file by that name, or add the string as-is.

Future Use Cases

In the future, if anyone wanted to add a dynamic string, they have to do the following:

  1. Add the condition to local.dynamic_custom_data
  2. Reference the key in custom_data
  3. Add any attributes needed in var.dynamic_custom_data[custom_data]

Questions

I understand this might be a lot to follow. Please let me know if you guys want to jump on a call to review.

Abdul Rabbani added 3 commits September 1, 2021 17:01
# Overview
Currently, users have the ability to add `custom_data` to a linux VM in two ways:

1. Passing a local path to a script.
2. Directly inserting code in the `tfvars`

This allows users to manually enter `custom_data` for these machines. But it does __not__ allow users to build a a string for the `custom_data` from the outputs of other Terraform outputs.

# Our Use Case
In our use case, we need to set the following string as the `custom_data`:
```
storage-account=stname, access-key=some-secret, file-share=myfileshare, share-directory=testdirectory
```

# Implementation
The implementation isnt exactly the cleanest. It currently requires a nested `if` statement to mimic an `elif` block. In the future is others want to add other dynamically created string for `custom_data`, they will have to add other another loop.
@abdulrabbani00 abdulrabbani00 marked this pull request as ready for review September 2, 2021 00:25
@abdulrabbani00 abdulrabbani00 changed the title Create a custom_data string with Terraform Ouputs Create a custom_data string with Terraform Ouputs Sep 2, 2021
@hriaz
Copy link
Contributor

hriaz commented Sep 4, 2021

@arnaudlh @LaurentLesle Can you provide your feedback on this? What are your thoughts?

@arnaudlh arnaudlh deleted the branch aztfmod:patch.5.4.4 September 6, 2021 14:33
@arnaudlh arnaudlh closed this Sep 6, 2021
@arnaudlh arnaudlh reopened this Sep 6, 2021
@arnaudlh arnaudlh changed the base branch from patch.5.4.3 to patch.5.4.4 September 6, 2021 14:48
@@ -1,3 +1,12 @@
locals {
dynamic_custom_data = {
palo_alto_connection_string = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a way to abstract the name here.
var.settings.custom_data will give you the key of your dynamic attribute you want to process.
have you tried naming the key in line 3 like
(var.settings.custom_data) = { .... }

Copy link
Contributor

Choose a reason for hiding this comment

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

or if I understand your objective here is to add additional items in the map like citrix_connection_string, avd_domain_join .... ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The latter is correct, the goal is to allow others to add new items to the map like citrix_connection_string, avd_domain_join ....

@@ -23,6 +23,11 @@ output "primary_blob_endpoint" {
value = azurerm_storage_account.stg.primary_blob_endpoint
}

output "primary_access_key" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Our original design objective was not to put keys as much as possible in the output as they can rotate.
can't you use a data source in the vm to retrieve the key at runtime?

access-key=${var.storage_accounts[var.client_config.landingzone_key][item.palo_alto_connection_string.storage_account].primary_access_key}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LaurentLesle - The question I would have is this:

  • If they access key rotates, does it get updated in Terraform per rotation?
    • If it does get updated in Terraform, then wouldn't the output be updated as well?
  • If it doesn't get updated, then would we be forced to set the key manually?
    • If so, wouldn't the key be insecure as plain text in the .tfvars file?

@LaurentLesle LaurentLesle merged commit 8aecb35 into aztfmod:patch.5.4.4 Sep 15, 2021
@LaurentLesle LaurentLesle added the enhancement New feature or request label Sep 15, 2021
@LaurentLesle LaurentLesle added this to In progress in Milestone 2109 via automation Sep 15, 2021
@LaurentLesle LaurentLesle added this to the 5.4.4 milestone Sep 15, 2021
@abdulrabbani00 abdulrabbani00 deleted the patch.5.4.3 branch September 15, 2021 13:35
@abdulrabbani00
Copy link
Contributor Author

@LaurentLesle - I created the following PR in relation to this: #684

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Milestone 2109
In progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants