[Release-2.2 BP FAB-2643]

[kopaygorodsky]

This is a backport of pull request #2643

This backport required upgrading viper to v1.1.1 as in the main branch, cherry-pick some related fixes to this upgrade.

#1523 (only 7dee9f7 "Use UnmarshalKey to retrieve peer BCCSP config" to fix pkcs11 suite)
#1511 (fully)

nothing new was added, just exact backport of #2643 with a bit of #1523 and full #1511

[kopaygorodsky]

PR is ready to be reviewed.

@yacovm @denyeart This failing unit test passes on my local machine + it passed one time in the pipeline. Could you check it pls? https://dev.azure.com/Hyperledger/Fabric/_build/results?buildId=41478&view=logs&j=6b58850f-3858-5a05-33e2-5e41cbf03c4e&t=bddec1cf-ba37-5883-9c3e-fd1e8608f9a1

[kopaygorodsky]

Unit tests were failing for some unknown reason to me. Locally everything passed, in the pipeline just 1 out of 10 times.
The second commit fixes the problem somehow, I partially cherry-picked it from 694bbca

[yacovm]

Thanks a lot for your effort, but, I'm honestly not 100% sure that we should cherry pick this.

Even though the documentation says that is possible, it seems to me more like a corner case of a feature.

Is there a specific reason we should cherry pick this to 2.2, and not let users just move to 2.3 if they really need it?

@kopaygorodsky do you have a specific reason you're backporting this? Do you need it in your environment or something?

@denyeart I saw you initiated the auto-backport but I'm not sure...

[kopaygorodsky]

@yacovm originally I made this fix because it was so annoying mounting core.yaml as a file. I needed to keep it up to date with the other values after patches. It looked like a bug to me which should be fixed in the LTS version.
Also, yes, @denyeart initiated this backport :)

[denyeart]

My thinking was if the backport is simple and focused, why not... but if it has to touch 125 files then it is no longer a simple and focused fix, and the risk of destabilizing the LTS release seems to outweigh the benefit. My preference therefore would be not to merge it, apologies for not being more clear up front!

[kopaygorodsky]

@denyeart what I can do is slightly different implementation with manual parsing of a YAML string, rather than using decode hook. Does it make sense? The only problem is that this test won't pass because of a bug in viper. Currently, it's not working as well.

[denyeart]

@kopaygorodsky I was willing to simply close it, but if this issue is impacting you and you'd like to invest the time, then go ahead with a smaller and more focused fix.

[mastersingh24]

This is way too many files to touch for a backport. Maybe you can try to make a more focused change?

[kopaygorodsky]

@mastersingh24 yes, I've already found a way to avoid the upgrade of viper, but I'm stuck with a test case that does not pass on the old viper.

fabric/core/chaincode/config_test.go

Line 50 in 2f9c167

Expect(config.SCCAllowlist).To(Equal(map[string]bool{"somecc": true}))

I added this test case as part of #2643 and it fails on 2.2. I'll try to find some middle ground to fix it.

[ale-linux]

This PR Is a tad old and has received no activity in a while. Should we close it? @kopaygorodsky what's the status? Thanks

[kopaygorodsky]

Closing this PR in favor of #3534