Scrub ID for random_string resource

[Ginja]

Fixes #17

Sets the ID for random_string resources to "none".

$ terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

The Terraform execution plan has been generated and is shown below.
Resources are shown in alphabetical order for quick scanning. Green resources
will be created (or destroyed and then created if an existing resource
exists), yellow resources are being changed in-place, and red resources
will be destroyed. Cyan entries are data sources to be read.

Note: You didn't specify an "-out" parameter to save this plan, so when
"apply" is called, Terraform can't guarantee this is what will execute.

  + random_string.password
      length:  "16"
      lower:   "true"
      number:  "true"
      result:  "<computed>"
      special: "true"
      upper:   "true"


Plan: 1 to add, 0 to change, 0 to destroy.


$ terraform apply
random_string.password: Creating...
  length:  "" => "16"
  lower:   "" => "true"
  number:  "" => "true"
  result:  "" => "<computed>"
  special: "" => "true"
  upper:   "" => "true"
random_string.password: Creation complete (ID: none)

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.


$ terraform plan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

random_string.password: Refreshing state... (ID: none)
No changes. Infrastructure is up-to-date.

This means that Terraform did not detect any differences between your
configuration and real physical resources that exist. As a result, Terraform
doesn't need to do anything.


$ terraform apply
random_string.password: Refreshing state... (ID: none)

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

[Ginja]

@apparentlymart thoughts?

[apparentlymart]

Hi @Ginja! Thanks for working on this, and sorry for the late response.

Looking at this resource again, I realize that it doesn't actually use the saved id value at all, and so it really doesn't matter what is stored in here. I assume your goal here was to just avoid showing the generated random string in the log output, in which case maybe it'd be simpler to just set the id to a hard-coded value (like d.SetId("none")) and then we'd avoid the complexity of including bcrypt and make the output simpler:

random_string.password: Refreshing state... (ID: none)

What do you think?

I know it's been a while since you submitted this so if you no longer have time to work on this please let me know and I can work on a simplified version myself when I get some spare moments.

[Ginja]

No problem! I didn't realize it could've been that simple. I thought all IDs needed to be unique. I'll submit a patch to this PR soon. Thanks for the tip.

[Ginja]

Done :)

I also went ahead and just squashed the two commits to avoid confusion.

[apparentlymart]

Hi @Ginja! Thanks for the updated version.

Although we do document that result is the way to get the random result from this resource, it is possible that someone would currently be using random_string.foo.id which would then be broken by this.

Because of that, we'll need to make a major release in order to incorporate this. This is not a big problem, but we have some other changes that will warrant a major version bump coming up as part of our configuration language improvement project, so if it's okay with you I'd like to hold this for a little while so we can release these things together. The timeline here would be at some point in the next few months, depending on where some other work lands in Terraform Core.

Thanks again for working on this!

[Ginja]

No problem! Sounds good to me.

[blackjid]

can't wait

[dpmerron-ltd]

This is a problem for us too. Do we have any ETA for this being merged?

[tinder-jlee]

I think setting the ID to "none" is not really a good approach. My concern is that it may hurt the "consistency" with the way how resources are managed. For example, archive_file resource generates a unique ID, and this ID is what addresses that particular resource.
So, I'd rather leave the id to address the particular random_string resource itself while setting result to be the actual random string generated.

[dpmerron-ltd]

See my PR above to address the issue of not setting the random_string id to ‘none’.

The way I see it working the random_password resource should never really require an ID so really does address the issue.

[mildwonkey]

Hi @Ginja!
It turns out that the configuration changes we were waiting for are going to take longer than expected, and we'd like to get your change merged (along with some other PRs) and release terraform-provider-random sooner than that. Can you please clean up the merge conflict? It appears to be very simple. Thanks!

[Ginja]

@mildwonkey Done 😃

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.