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

Fix certPath in assetTransfer.go #1169

Closed
wants to merge 1 commit into from
Closed

Conversation

thejjw
Copy link

@thejjw thejjw commented Jan 15, 2024

Following the latest tutorial, the current version of network.sh does not generate cert.pem as filename under msp/signcerts but instead created [email protected]. I have edited the application file to reflect such difference.

current version of network.sh does not generate cert.pem as filename

Signed-off-by: thejjw <[email protected]>
@thejjw thejjw requested a review from a team as a code owner January 15, 2024 01:33
@denyeart
Copy link
Contributor

The problem is that test-network with crytpgen puts the cert in /signcerts/[email protected] while test-network with fabric-ca puts the cert in /signcerts/cert.pem. This is unfortunate. Most of the tests use test-network with fabric-ca so I don't think we'll want to make the change as initially proposed here.

I think the solution is to make the applications work regardless of the cert file name. The apps do this for the keystore directory but not the signcerts or tls ca directories.

See the example at:
https://github.com/hyperledger/fabric-samples/blob/main/asset-transfer-basic/application-gateway-go/assetTransfer.go#L31-L33

The apps use the first file found in the keystore directory. I think it should do this for signcerts and tls ca directories as well, so that the approach is consistent, and will work regardless of the cert file names.

If you want to contribute this change that would be great. Otherwise we can open an issue for the backlog to do this across all the app samples.

But let's make sure the author @bestbeforetoday agrees with the approach.

@bestbeforetoday
Copy link
Member

I'm not clear what the conditions are whereby the certificate has a different filename from the one currently used by the code. The repository tests seem to be passing with the current code, and this PR build is failing because the modified filename does not exist.

If there are cases where the certificate filename can be different and the certificate directory will only hold one certificate file (as with the private key filename), picking the first file in the directory (again, as we do with the private key file, and as suggested by @denyeart above) sounds like a good approach to me.

@denyeart
Copy link
Contributor

@bestbeforetoday See my first paragraph above. Basically it comes down to whether the certificate was created by cryptogen versus fabric-ca.

Regardless, we are in agreement that the solution is to change the samples to read any certificate file in the signcerts directory, rather than reading a hardcoded file name.

@bestbeforetoday
Copy link
Member

bestbeforetoday commented Feb 29, 2024

Oops, my sloppy reading. Your previous explanation was perfect!

It sounds like whichever is the first file in that directory should be usable. It would be good to break out the logic to read the first file from a directory to a separate function that can be shared by both the signer and identity creation functions. The same change will need to be made to all three language variants of the application-gateway-* application. Likely other samples too.

@bestbeforetoday
Copy link
Member

I raised PR #1183 to address this issue for all the relevant sample applications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants