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

examples: create examples structure and add first example #317

Closed
wants to merge 2 commits into from

Conversation

whooo
Copy link
Contributor

@whooo whooo commented Jan 12, 2022

This is a starting point for example usage of the different APIs, starting with a basic example of using ESAPI.

My plan is to add more examples over time which somewhat should cover real world use cases while being readable.

@whooo
Copy link
Contributor Author

whooo commented Jan 12, 2022

@niooss-ledger, could you have a look and see if you think this can close #298

@williamcroberts
Copy link
Member

I wonder if we should mention in the ESYS_TR class documentation that to go from a TPM2_HANDLE to an ESYS_TR you should use tr_from_public.

@codecov
Copy link

codecov bot commented Jan 12, 2022

Codecov Report

Merging #317 (6bec769) into master (235c439) will decrease coverage by 0.01%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
- Coverage   93.54%   93.53%   -0.02%     
==========================================
  Files          15       15              
  Lines        4662     4699      +37     
  Branches      590      600      +10     
==========================================
+ Hits         4361     4395      +34     
- Misses        218      224       +6     
+ Partials       83       80       -3     
Impacted Files Coverage Δ
tpm2_pytss/internal/crypto.py 92.36% <0.00%> (-1.77%) ⬇️
tpm2_pytss/types.py 95.43% <25.00%> (-0.10%) ⬇️
tpm2_pytss/ESAPI.py 98.10% <100.00%> (+0.16%) ⬆️
tpm2_pytss/constants.py 99.27% <100.00%> (+0.02%) ⬆️
tpm2_pytss/tsskey.py 98.34% <100.00%> (ø)
tpm2_pytss/TCTILdr.py 93.75% <0.00%> (+6.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7338869...6bec769. Read the comment docs.

@whooo
Copy link
Contributor Author

whooo commented Jan 12, 2022

I wonder if we should mention in the ESYS_TR class documentation that to go from a TPM2_HANDLE to an ESYS_TR you should use tr_from_public.

Wouldn't hurt, I can create another PR for that.

Copy link
Contributor

@niooss-ledger niooss-ledger left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! I agree this would be a nice way to close #298 :)

I added some comments. Also, the example does not work as-is on a TPM I am using, due to an issue that I reported in #318, but when I work around this issue, I managed to run it. And the code of the example is easy to understand :)

examples/tpm2_filternv/README.md Outdated Show resolved Hide resolved
examples/tpm2_filternv/tpm2_filternv Show resolved Hide resolved
@williamcroberts
Copy link
Member

I have mixed feelings about example code as well, it's often times left to rot, plus how do we know if something we do breaks an example. We need to figure out a way to get this into our test suite.

@whooo whooo mentioned this pull request Jan 13, 2022
@whooo whooo force-pushed the examples branch 2 times, most recently from 6af8863 to 261f8af Compare January 13, 2022 14:57
@whooo
Copy link
Contributor Author

whooo commented Jan 13, 2022

I have mixed feelings about example code as well, it's often times left to rot, plus how do we know if something we do breaks an example. We need to figure out a way to get this into our test suite.

I worry about the rot as well and I think having good tests might be hard as some of the examples I plan on writing will require extra dependencies.
I'm open to having the examples in another git repository (for example directly under my user on github) with a big warning about testing/compatibility

@williamcroberts
Copy link
Member

I have mixed feelings about example code as well, it's often times left to rot, plus how do we know if something we do breaks an example. We need to figure out a way to get this into our test suite.

I worry about the rot as well and I think having good tests might be hard as some of the examples I plan on writing will require extra dependencies. I'm open to having the examples in another git repository (for example directly under my user on github) with a big warning about testing/compatibility

Wouldn't they just be dev dependencies? As long as you can pip install and add them to the dev section, I don't really care.
We've also talked about having more tutorials and stuff under our github io page, so that's an option too, or we could embed them in the docs as a tutorials section for just pytss. I'm pretty open to ideas here, and not really invested in any one way.

@whooo whooo force-pushed the examples branch 7 times, most recently from b8754d8 to 5581417 Compare January 17, 2022 12:34
@whooo
Copy link
Contributor Author

whooo commented Jan 17, 2022

I worry about the rot as well and I think having good tests might be hard as some of the examples I plan on writing will require extra dependencies. I'm open to having the examples in another git repository (for example directly under my user on github) with a big warning about testing/compatibility

Wouldn't they just be dev dependencies? As long as you can pip install and add them to the dev section, I don't really care. We've also talked about having more tutorials and stuff under our github io page, so that's an option too, or we could embed them in the docs as a tutorials section for just pytss. I'm pretty open to ideas here, and not really invested in any one way.

Putting the dependencies under dev should be enough for now.
I added a test for the first example, I didn't add full coverage as I don't think it's needed but I might be wrong

This examples covers tpm2-tss-engine / tpm2-openssl key usage and
signing using ESAPI.

Signed-off-by: Erik Larsson <[email protected]>
@iolivergithub
Copy link

FYI, I've started putting examples up at: https://github.com/tpm2dev/tpm.dev.tutorials/tree/master/PythonExamples
Feel free to use

@williamcroberts
Copy link
Member

I think if we have examples, to ensure they are complete they should run with the test harness, i'm sure we could plumb them in somehow.

@whooo
Copy link
Contributor Author

whooo commented Jun 25, 2022

I'll close this for now, might revisit it in the future

@whooo whooo closed this Jun 25, 2022
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.

4 participants