Replace md5 with sha256 algorithm #9021
Conversation
Signed-off-by: IvanChalukov <ichalukov@gmail.com>
There was a problem hiding this comment.
I have tested something really similar, due to my closed issue #9007. I tried with FIPS and non FIPS compliant databases, both worked like a charm locally and on my bosh environment. When having multiple webs in rolling update, I can see some temporary trouble, due to the first web migrating the data and the second still working with md5 columns, but that is expected with such migrations. The other way would be to not rename the columns at all, since they are used for UID from what I can gather. But that wouldn’t be okay imho. Looks good to me, good job!
|
The validations are currently failing due to check comment. Once the issue is fixed in upstream, or the module is reverted, we will recheck. |
|
@taylorsilva since the PR resource uses the broken base at the moment still and I don't have permissions to recreate the version, what is the procedure, so we can trigger the PR validations with the correct base? Thanks! Edit: |
|
@Kump3r it's fixed now, so you should be unblocked |
|
Yes, just the failing PR's with this error need to clear the cache of the resource, since otherwise it will not fetch the new base. Btw I am not sure if bug is the correct label for such a PR, feel free to adjust as necessary 😅 |
|
Haven't had a chance to review this. Happy to see all the test passed though! I think this should go into a minor release version since it's a big change. Will definitely take this in once I have time to review it. |
|
Just making a note: one concern I have is how long will the migration take on a large Concourse database? I'm reminded of #8499 which takes 30mins on a large database. I sadly don't have access to a large database. I plan to create one and do some manual acceptance testing. |
|
From my side I haven’t tested with a heavily loaded database, but if I manage to allocate some time, I can try it out and let you know as well. Thanks for the remark, it is a valid concern. |
|
I unfortunately haven't had the opportunity to test with a heavily loaded database either, but I'll make sure to give it a try as soon as I have the chance. Thanks for pointing that out! |
|
Would be a big help if either of you can do that! |
Changes proposed by this PR
closes #9020
Switching
md5algorithm tosha256across the code base and add migration for database naming convention.Notes to reviewer
Tested on FIPS compliant database and on already existing database.
Release Note
md5algorithm tosha256across the code basemd5tosha256suffix