equinix / terraform-equinix-metal-openstack

OpenStack Cloud on Equinix Metal
https://registry.terraform.io/modules/equinix/openstack/metal/latest
Apache License 2.0
13 stars 12 forks source link

Supporting pull request for submitting this module to the Terraform R… #44

Closed jmarhee closed 3 years ago

jmarhee commented 4 years ago

…egistry: Per best practices, moves scripts to assets/, renames vars.tf to variables.tf, 0.13upgrade's this repo

displague commented 4 years ago

Do we have access to the variables we would need to make this module configure the openstack provider? These should be made available through the module's root outputs.tf.

https://registry.terraform.io/providers/terraform-provider-openstack/openstack/latest/docs

provider "openstack" {
  user_name   = "admin"
  tenant_name = "admin"
  password    = "pwd"
  auth_url    = "http://myauthurl:5000/v2.0"
  region      = "RegionOne"
}
jmarhee commented 4 years ago

Good call-- I was going to do this as part of the PR to draft examples/, but it makes sense to do it here; I believe all of these are available for outputs, but not all are implemented as outputs that would be available to the root module (and I'd like to change that the password is hardcoded as ADMIN_PASS for that reason), but should be trivial to setup as expected.

displague commented 3 years ago

I didn't know if the OpenStack password was generated upon creation, or if the user needed to supply one when creating the service.

The password could be taken from an input variable. But if you want to avoid asking the user to provide one, you could generate one with https://registry.terraform.io/providers/hashicorp/random/latest/docs/resources/password (maybe in one of the examples or as part of the CI testing if GH Actions don't provide a facility for generated passwords)

jmarhee commented 3 years ago

The main change in the above commit is to:

Additionally, I have opened #45 to note that we will need to rename this repo in order to submit to the registry as @displague noted in the original ticket-- this is just a reminder for us to do that before we tag a release at the end of this.

displague commented 3 years ago

This is just a tip, take it or leave it, but I've found it confusing to navigate shell templates that Terraform will mangle because the template syntax matches normal shell syntax. I've found that it is helpful to use an extension like .tpl.sh (or similar) and to include a comment block at the beginning with alternate variables names to make the template variables more obviously differentiated from the other variables.

#!/usr/bin/env bash

# Terraform Template Variables
TF_ADMIN_PASS="${ADMIN_PASS}"

# now we shouldn't expect to see ${ADMIN_PASS} again
echo "$TF_ADMIN_PASS"