feat(pkg): Add secrets SDK and `keyring` module #1759

traeok · 2023-07-25T17:33:35Z

What It Does

This adds keytar-rs into the CLI as a new package: @zowe/secrets-for-zowe-sdk. It also removes references to the old node-keytar library as it is no longer used.

Added the secrets SDK as a new package to the zowe-cli monorepo
Added keyring module to said secrets SDK
Added test and publish stages for secrets SDK to prepare for publishing

How to Test

Note: The test stages will fail - 2 integration tests are failing on the daemon because Imperative is not using the Secrets SDK yet. Once the Imperative branch is merged into the codebase, CLI will pass with the new version.

Reviewers should be able to clone the repo, cd into packages/secrets, and run npm install to build from source. In addition, reviewers should be able to run npm run test in this folder to verify that all tests are passing for the keyring module.

Review Checklist
I certify that I have:

tested my changes
added/updated automated tests
updated the changelog
followed the contribution guidelines

Additional Comments

Sayonara, node-keytar 👋

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

@t1m0thyj

kudos to @t1m0thyj for converting to matrix :) Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

codecov · 2023-07-25T17:33:40Z

Codecov Report

Patch and project coverage have no change.

Comparison is base (f5fff9e) 92.79% compared to head (13de50c) 92.79%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1759   +/-   ##
=======================================
  Coverage   92.79%   92.79%           
=======================================
  Files         422      422           
  Lines        7165     7165           
  Branches     1309     1309           
=======================================
  Hits         6649     6649           
  Misses        515      515           
  Partials        1        1

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

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

.github/workflows/secrets-sdk.yml

packages/secrets/scripts/prebuildify.sh

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

t1m0thyj · 2023-07-25T18:46:36Z

@traeok Is it right that we expect 2 CLI tests to be failing for now, until we can update Imperative to use the secrets SDK?

traeok · 2023-07-25T19:50:26Z

@traeok Is it right that we expect 2 CLI tests to be failing for now, until we can update Imperative to use the secrets SDK?

@t1m0thyj Yes - once imperative has the new secrets SDK, those integration tests should pass.

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

packages/secrets/package.json

packages/secrets/src/keyring/.gitignore

packages/secrets/src/keyring/.npmignore

packages/secrets/src/keyring/README.md

packages/secrets/src/keyring/napi.json

packages/secrets/src/keyring/src/build.rs

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

ATorrise

Hi - compiles! when building on windows, I get some unrelated output though

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

ATorrise

Those warnings went away, looks good :)

.github/workflows/secrets-sdk.yml

t1m0thyj

Will approve once the UTF-8 encoding issue on Windows has been addressed 😋

.github/workflows/secrets-sdk-publish.yml

packages/secrets/src/keyring/README.md

packages/secrets/package.json

t1m0thyj · 2023-07-28T12:14:36Z

packages/secrets/src/keyring/index.js

+
+const requireFn = typeof __webpack_require__ === "function" ? __non_webpack_require__ : require;
+const binaryPath = requireFn.resolve(`./keyring.${getTargetName()}.node`, {
+    paths: [__dirname, join(__dirname, "..", "..", "prebuilds")],


It may be weird for Zowe extenders who try to webpack the Secrets SDK that the prebuilds path is hardcoded to be 2 levels up. Since this works fine for ZE, I'm ok with leaving as-is until someone requests an enhancement 😋

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

sonarcloud · 2023-07-28T20:06:12Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
8 Security Hotspots
0 Code Smells

0.0% Coverage
6.1% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

traeok and others added 21 commits June 26, 2023 15:27

feat: replace keytar w/ keytar-rs

59865eb

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

Update keytar-rs sha in shrinkwrap

7d460b3

Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>

Merge branch 'master' into feat/keytar-rs

80c9c47

feat: new Secrets SDK with keyring module

7fcca77

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(pkg): Update scripts for secrets SDK package

b8e196c

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(pkg): adjust package.json and build logic for monorepo

7c0ef67

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix: case in index.js switch statement for platform resolution

5057491

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(pkg): shorten switch statement for index.js

af620a5

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

ci: add secrets SDK workflow, update install script

07e935c

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix: use yarn v3 instead of npm

10ddf4c

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix: adjust module exports for index.js

396b473

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

test(ci): ls before yarn test on 32bit windows

21bb5a3

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix: remove dir test and use npm instead of yarn

b4626b3

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

ci: use test matrix instead of individual test stages

3486cfb

kudos to @t1m0thyj for converting to matrix :) Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(ci): separate setup stage for musl

90fd778

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

feat(ci): add publish workflow for secrets SDK

bc383ea

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(ci): update linux-test script

575c056

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(ci): Update path for downloading artifacts

1c598d5

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(ci): use --ignore-scripts when installing pkg

3424431

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(ci): remove working-directory for install dep. stages

69cc33b

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(ci): adjust linux-test.sh

89c7c86

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix shrinkwrap

90e23d3

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

t1m0thyj reviewed Jul 25, 2023

View reviewed changes

.github/workflows/secrets-sdk.yml Outdated Show resolved Hide resolved

.github/workflows/secrets-sdk.yml Outdated Show resolved Hide resolved

.github/workflows/secrets-sdk.yml Outdated Show resolved Hide resolved

packages/secrets/scripts/prebuildify.sh Outdated Show resolved Hide resolved

traeok added 2 commits July 25, 2023 14:02

fix(ci): address Timothy's comments

11a1c21

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(pkg): address prebuilds comment

68b2518

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

feat(pkg): Build secrets SDK w/ typescript

df3f6fc

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

traeok requested a review from t1m0thyj July 27, 2023 12:37

traeok added 2 commits July 27, 2023 09:39

chore: adjust secrets changelog and versioning

32d35f3

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

fix(cov): remove secrets/index.ts from code coverage

b0a08a0

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

t1m0thyj reviewed Jul 27, 2023

View reviewed changes

fix: Address Timothy's review

af6e5b2

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

ATorrise reviewed Jul 27, 2023

View reviewed changes

fix(keyring): Address Amber's comment w/ unused enum variants

cb2cd23

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

ATorrise self-requested a review July 27, 2023 19:59

ATorrise approved these changes Jul 27, 2023

View reviewed changes

awharn approved these changes Jul 28, 2023

View reviewed changes

.github/workflows/secrets-sdk.yml Outdated Show resolved Hide resolved

t1m0thyj requested changes Jul 28, 2023

View reviewed changes

traeok requested a review from zFernand0 July 28, 2023 14:20

fix(keyring): CredFree after owning String for Windows getPassword impl.

5f244df

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

traeok force-pushed the feat/keytar-rs branch from 612de17 to 5f244df Compare July 28, 2023 15:33

traeok requested a review from t1m0thyj July 28, 2023 15:34

traeok force-pushed the feat/keytar-rs branch from 8cb6bb2 to e0acfcf Compare July 28, 2023 16:12

fix(keyring): cleanup Windows logic; address some of Timothy's comments

56209f8

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

traeok force-pushed the feat/keytar-rs branch from e0acfcf to 56209f8 Compare July 28, 2023 17:36

traeok added 2 commits July 28, 2023 13:45

chore: add armv7l to compat. matrix

de26781

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

ci: remove quotes where possible in workflow

e8ee4f9

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

traeok force-pushed the feat/keytar-rs branch from 27020d7 to e8ee4f9 Compare July 28, 2023 18:11

ci: relative path to .npmrc for npm-cli-login

44e14a0

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

t1m0thyj approved these changes Jul 28, 2023

View reviewed changes

chore: use original CLI package.json until imperative is merged

f150862

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

traeok force-pushed the feat/keytar-rs branch from c79f5f4 to f150862 Compare July 28, 2023 18:33

doc: Add typedoc to the Rust keyring code

13de50c

Signed-off-by: Trae Yelovich <trae.yelovich@broadcom.com>

traeok force-pushed the feat/keytar-rs branch from bdd000b to 13de50c Compare July 28, 2023 19:40

traeok merged commit a9f2e9b into master Jul 28, 2023
46 of 47 checks passed

traeok deleted the feat/keytar-rs branch July 28, 2023 20:09

traeok mentioned this pull request Sep 14, 2023

Keytar Replacement #1622

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pkg): Add secrets SDK and `keyring` module #1759

feat(pkg): Add secrets SDK and `keyring` module #1759

traeok commented Jul 25, 2023 •

edited

Loading

codecov bot commented Jul 25, 2023 •

edited

Loading

t1m0thyj commented Jul 25, 2023 •

edited

Loading

traeok commented Jul 25, 2023 •

edited

Loading

ATorrise left a comment

ATorrise left a comment

t1m0thyj left a comment

t1m0thyj Jul 28, 2023

sonarcloud bot commented Jul 28, 2023

feat(pkg): Add secrets SDK and keyring module #1759

feat(pkg): Add secrets SDK and keyring module #1759

Conversation

traeok commented Jul 25, 2023 • edited Loading

codecov bot commented Jul 25, 2023 • edited Loading

Codecov Report

t1m0thyj commented Jul 25, 2023 • edited Loading

traeok commented Jul 25, 2023 • edited Loading

ATorrise left a comment

Choose a reason for hiding this comment

ATorrise left a comment

Choose a reason for hiding this comment

t1m0thyj left a comment

Choose a reason for hiding this comment

t1m0thyj Jul 28, 2023

Choose a reason for hiding this comment

sonarcloud bot commented Jul 28, 2023

feat(pkg): Add secrets SDK and `keyring` module #1759

feat(pkg): Add secrets SDK and `keyring` module #1759

traeok commented Jul 25, 2023 •

edited

Loading

codecov bot commented Jul 25, 2023 •

edited

Loading

t1m0thyj commented Jul 25, 2023 •

edited

Loading

traeok commented Jul 25, 2023 •

edited

Loading