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

regex_replace_chars is not inherited by context (follow-up on #55) #57

Closed
rfvermut opened this issue Apr 8, 2019 · 5 comments
Closed
Assignees

Comments

@rfvermut
Copy link

rfvermut commented Apr 8, 2019

I believe it didn't fix the problem. You forgot regex_replace_chars_or_context and as result this line is always setting default regex_chars.

@aknysh
Copy link
Member

aknysh commented Apr 8, 2019

@rfvermut

I think it's ok.

regex_replace_chars                    = "${var.regex_replace_chars != "" ? var.regex_replace_chars : local.regex_replace_chars_context_or_default}"

will take it from the var if provided, or from context_or_default (which will take it from the var, even if it's empty, if not provided in the context).

we don't need four local vars for this (as we do for name, namespace, etc.) since we are not replacing the chars in regex_replace_chars itself

@aknysh aknysh self-assigned this Apr 8, 2019
@rfvermut
Copy link
Author

rfvermut commented Apr 8, 2019 via email

@rfvermut
Copy link
Author

rfvermut commented Apr 9, 2019

See example:

module "base_label" {
  source              = "git::https://github.com/cloudposse/terraform-null-label.git?ref=tags/0.7.0"
  namespace           = "com.dotlover"
  environment         = "staging"
  regex_replace_chars = "/[^a-zA-Z0-9-.]/"
  delimiter           = "."
}

module "dns_label" {
  source  = "git::https://github.com/cloudposse/terraform-null-label.git?ref=tags/0.7.0"
  context = "${module.base_label.context}"
  name    = "dns"
}

output "base" {
  value = "${module.base_label.name}"
}

output "dns" {
  value = "${module.dns_label.name}"
}

It will output com.dotlover.staging and comdotlover.staging.dns (notice the dot missing). Is this how you meant it to work?

@aknysh
Copy link
Member

aknysh commented Apr 9, 2019

@rfvermut thanks, you are correct.
regex_replace_chars is different from all other attributes (namespace, stage, name) because it always has the default value. This needs to be fixed. There are two solutions here:

  1. When using context, the user needs to set regex_replace_chars = "" for the it to work (not a pretty solution for the user)

  2. Since it's not possible to detect whether the regex_replace_chars value was used from default or was provided, we can introduce an internal local var regex_replace_chars_default and set it to the current default. Then in the module check if the provided value in the variable is different from the internal default; if different, use the provided value from the input; if the same, check and use the context

@Nuru
Copy link
Contributor

Nuru commented Aug 27, 2020

I am closing this as I think it has been fixed by #99 if not before. In any case, it does not reflect the current design. Please open a new issue if you still have this problem.

@Nuru Nuru closed this as completed Aug 27, 2020
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

No branches or pull requests

3 participants