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 Secrets SDK with Rust keyring bindings #222

Merged
merged 46 commits into from
Nov 14, 2023
Merged

Conversation

t1m0thyj
Copy link
Member

@t1m0thyj t1m0thyj commented Oct 31, 2023

What It Does
Resolves #208 by adding Python to Rust bindings that use the same keyring backend as the Node SDK.
Replaced all references to the keyring package with our own keyring subpackage in the Secrets SDK.
Added Secrets SDK as an optional dependency of the core SDK that can be installed with the secrets feature.

How to Test
Reinstall dependencies after pulling this branch. The install script for Secrets SDK should build the keyring binary.

Manually test the methods of the keyring package with this script:

from zowe.core_for_zowe_sdk import CredentialManager
from zowe.secrets_for_zowe_sdk import keyring
# Store short password using keyring module directly
password = "abc123"
keyring.set_password("Test", "ShortPassword", password)
assert keyring.get_password("Test", "ShortPassword") == password
assert keyring.delete_password("Test", "ShortPassword")
assert keyring.get_password("Test", "ShortPassword") is None
# Store long password using CredentialManager wrapper
password *= 512
CredentialManager._set_credential("Test", "LongPassword", password)
assert CredentialManager._get_credential("Test", "LongPassword") == password
CredentialManager._delete_credential("Test", "LongPassword")
assert CredentialManager._get_credential("Test", "LongPassword") is None

Review Checklist
I certify that I have:

  • tested my changes
    • Windows
    • Linux
    • MacOS
  • added/updated automated tests
  • updated the changelog
  • followed the contribution guidelines

Additional Comments
Coauthored with @traeok 🙂

t1m0thyj and others added 3 commits October 30, 2023 15:30
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (05aa573) 82.88% compared to head (f21170e) 83.16%.

Files Patch % Lines
.../core/zowe/core_for_zowe_sdk/credential_manager.py 93.33% 2 Missing ⚠️
tests/unit/test_zowe_core.py 97.72% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #222      +/-   ##
==========================================
+ Coverage   82.88%   83.16%   +0.27%     
==========================================
  Files          33       34       +1     
  Lines        2115     2067      -48     
==========================================
- Hits         1753     1719      -34     
+ Misses        362      348      -14     
Flag Coverage Δ
unittests 83.16% <96.05%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@t1m0thyj t1m0thyj force-pushed the feat/add-secrets-sdk branch from 135cfc3 to 52a465e Compare October 31, 2023 17:43
@t1m0thyj t1m0thyj force-pushed the feat/add-secrets-sdk branch from 8d47a35 to 8a92ece Compare November 1, 2023 12:43
@t1m0thyj t1m0thyj force-pushed the feat/add-secrets-sdk branch from 689fb17 to 505de34 Compare November 1, 2023 15:10
@t1m0thyj t1m0thyj force-pushed the feat/add-secrets-sdk branch 4 times, most recently from 7d7852b to 42c222d Compare November 3, 2023 17:58
@t1m0thyj t1m0thyj force-pushed the feat/add-secrets-sdk branch from 42c222d to 3114108 Compare November 3, 2023 18:04
Copy link
Member

@traeok traeok left a comment

Choose a reason for hiding this comment

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

I feel awkward approving this, as I had some commits on this PR and did some related work to prepare the secrets_core crate 😅

But, everything LGTM though - thanks @t1m0thyj

src/secrets/Cargo.toml Outdated Show resolved Hide resolved
@t1m0thyj t1m0thyj force-pushed the feat/add-secrets-sdk branch from 8817566 to 917ebd7 Compare November 7, 2023 17:05
Enable release workflows to publish prerelease to PyPI
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

LGTM! 😋

Seems to work just fine with more than just ascii 😋

@zFernand0 zFernand0 merged commit 95e819f into main Nov 14, 2023
48 checks passed
@zFernand0 zFernand0 deleted the feat/add-secrets-sdk branch November 14, 2023 15:19
@t1m0thyj t1m0thyj added release-dev Add to PR to publish a new dev release when merged and removed release-dev Add to PR to publish a new dev release when merged labels Nov 14, 2023
@t1m0thyj t1m0thyj linked an issue Dec 3, 2023 that may be closed by this pull request
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.

Port the Zowe Secrets SDK to Python [core] Don't change log level of global logger
4 participants