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

Enable substr to handle shorter strings #15751

Closed
forevermatt opened this issue Aug 7, 2017 · 15 comments
Closed

Enable substr to handle shorter strings #15751

forevermatt opened this issue Aug 7, 2017 · 15 comments

Comments

@forevermatt
Copy link

Related to issue #1431, having a way to truncate a string to a given length is very useful, especially for names that cannot be more than x characters long (such as AWS Application Load Balancers) when the name will be assembled from multiple other variables (such as "alb-${var.app_name}-${var.app_env}"). However, the substr function currently chokes on strings that are already within the given constraint.

Terraform Version

0.9.11

Expected Behavior

A call to substr with a string shorter than the index+length specified should simply return the given string. Example substr("abc", 0, 5).

Actual Behavior

Error message:

'offset + length' cannot be larger than the length of the string

Steps to Reproduce

  1. Run terraform console.
  2. Run substr("abcdef", 0, 5) and see expected result of abcde.
  3. Run substr("abc", 0, 5) and see error message (rather than expected result of abc).

References

Are there any other GitHub issues (open or closed) or Pull Requests that should be linked here?

@forevermatt
Copy link
Author

I'm currently using essentially the following to limit the length of our AWS ALB Target Group names, through it seems a touch more complicated (= less readable) than substr would be:

"${replace("tg-${var.idp_name}-${var.app_name}-${var.app_env}", "/(.{0,32})(.*)/", "$1")}"

@apparentlymart
Copy link
Contributor

Thanks for this request, @forevermatt.

This seems reasonable to me. Generating an error when offset is greater than the length is desirable, I think, since it is likely indicative of a mistake. But requesting more characters than remain in the string is a perfectly reasonable thing to do for the reason you gave here.

@wandergeek
Copy link

+1

@nathanielks
Copy link
Contributor

See #5157 (comment)

@forevermatt
Copy link
Author

@nathanielks Thanks for your interest in this issue. Would you mind clarifying why you referenced that comment? It looks like that is merely pointing at the existence of the substr() function, whereas this issue is requesting that substr() be enhanced to better handle short input strings.

@nathanielks
Copy link
Contributor

forgive me, @forevermatt! It looks like I was a bit overzealous in mentioning that comment elsewhere. There was a duplicate ticket asking for a substr function, so I referenced the solution on that ticket. It would appear I didn't read this one closely enough! 😆

Ironically, I too would like the functionality you're asking for! My workaround:

substr("${local.prefix}", 0, min(19, length(local.prefix)))

@forevermatt
Copy link
Author

No problem 🙂

@dev-head
Copy link

dev-head commented Jan 27, 2018

+1 on this, i can understand the intention but in many practical use cases this should not break the build because there's not enough characters. Sometimes this is just to enforce the api restriction when dealing with dynamic variables that get passed through. I just ran into this same issue with the elastic cache cluster limit of only 20 characters and now have a unique variable to define the cluster, which follows no convention.

thanks, hope to see this improved.

edit: improved words

@CumpsD
Copy link

CumpsD commented Jun 26, 2018

I ran into this as well today, would love to see this addition to terraform

@dnk8n
Copy link

dnk8n commented Jun 27, 2018

I also ran into this today. Keen to learn some Go (Terraform is implemented in Go right?) so may try and work out how to contribute. Don't know what I am up against yet but will see if I can work it out.

@nathanielks
Copy link
Contributor

nathanielks commented Jun 27, 2018

@dnk8n I didn't know any Go before contributing to Terraform, but it's (Terraform) designed well that it's semi easy to pick it up/interact with Terraform. Give it a... Go :trollface:

@apparentlymart
Copy link
Contributor

Hi all!

While this sort of change would normally be a pretty nice introduction to Terraform development, unfortunately right now we're working in a feature branch that refactors how the interpolation functions are implemented, and so the implementations in the master branch will be deleted once that branch is merged.

Fortunately, from a quick check I think that this issue has been addressed already (when we wrote the new implementation of substr we wrote it without the length check mentioned here), so this should be addressed in the next major release. Since the development for that has not landed on master yet, I'll leave this open for now as a reminder to verify the behavior as part of the preparation for the release once the branch has been merged.

@nathanielks
Copy link
Contributor

Good to hear, thanks @apparentlymart!

@apparentlymart apparentlymart added this to the v0.12.0 milestone Nov 29, 2018
@apparentlymart
Copy link
Contributor

Hi all!

I've just verified this in the v0.12.0-alpha4 prerelease build, using the following configuration:

locals {
  str = "abc"
}

output "shorter" {
  value = substr(local.str, 0, 2)
}

output "equal" {
  value = substr(local.str, 0, 3)
}

output "longer" {
  value = substr(local.str, 0, 4)
}

This successfully produced the following output values, as expected:

equal = abc
longer = abc
shorter = ab

This functionality is now in the master branch ready to be included in the forthcoming v0.12.0 release, so I'm going to close this out. Thanks for requesting this, and sorry for the delay in getting it implemented!

@ghost
Copy link

ghost commented Mar 30, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants