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

add new resource random_password #52

Merged
merged 6 commits into from
Jul 29, 2019

Conversation

jcarrothers-sap
Copy link
Contributor

As a final solution to #17/#47, add a new resource random_password which is identical
to random_string except that it treats the result as Sensitive data and avoids writing it
to the console. The new resource shares the vast majority of its code with random_string
and intends to simply be a mirror of that resource.

This change also restores the pre #18 (v1.x) behaviour of random_string since scrubbing
the id field is no longer necessary for that resource.

@jcarrothers-sap
Copy link
Contributor Author

@mildwonkey Hopefully this will serve as a final solution to this problem.

@mattyjones
Copy link

@appilon Is there anything I can do on this pull request to help it get merged. It helps to solve a big problem for us.

Thanks

@ipleten
Copy link

ipleten commented Apr 10, 2019

Is there anything to do with that PR? Could it be merged please?

@paulirwin
Copy link

Why not just add a sensitive attribute to random_string, and default it to false (for backwards compatibility)? Or is that technically infeasible? One benefit of this approach is that any existing state using random_string would not have to be modified, I would hope.

(Please note that I am not an expert at Terraform's internals or provider development, this is just the API design I would have imagined.)

@jcarrothers-sap
Copy link
Contributor Author

Why not just add a sensitive attribute to random_string, and default it to false (for backwards compatibility)? Or is that technically infeasible?

It is my understanding that the set of attributes that controls the "sensitive" behaviour is read in, and processed, prior to any .tf files even being read.

@tmc
Copy link

tmc commented May 22, 2019

ping @appilon

@StefanSchoof
Copy link

No that terraform 0.12 is there, is there any hope to see a merge sometime soon?

@StephenWithPH
Copy link

@appilon ... is there anything blocking this from being merged? If so, please reply and let us all know. That will help us address any concerns so we can ultimately get this very desirable feature.

@appilon
Copy link
Contributor

appilon commented Jul 24, 2019

Hello, apologies for taking a long time to address this. In response to demand I was going to actually supersede this PR with this #67. However there would be a drawback that terraform show would never be able to display the value, furthermore it felt awkward breaking an "expectation" to any users NOT using random_string as a secret.

It seems our docs indicate random_string CAN be used as a secret, which has lead to the fork in the road we are at. As such we will move forward with this PR, I will refactor it to share more code and will try and improve the documentation to make the distinctions more clear.

Longterm it's possible random_string becomes redundant given random_ids existence? We can cross that bridge later. Thank you all for your patience.

@ghost ghost added size/XL and removed size/M labels Jul 25, 2019
Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

Hey @appilon ! This LGTM, I left one little edit in a doc, otherwise this is good to merge.

@appilon appilon merged commit 55a717f into hashicorp:master Jul 29, 2019
@jcarlson
Copy link

Hey! Is there a migration path from random_string to random_password?

For example, I have a module that builds AWS RDS instances and includes a random_string to generate the password. But I've noted that I can't simply "move" the random_string resource to the random_password:

terraform state mv module.rds.random_string.password module.rds.random_password.password

Error: Invalid state move request

Cannot move module.rds.random_string.password to
module.rds.random_password.password:
resource types don't match.

I don't see an "import" function either on the random provider... so it looks like my only option would be to "change" the database's password, which might result in some downtime for the app.

@RyPeck
Copy link

RyPeck commented Jan 29, 2020

@jcarlson I just had the same question. I would say your comment should be a separate issue.

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.