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 Config API bugs that affect extenders converting V1 profiles #2313

Merged
merged 12 commits into from
Nov 11, 2024

Conversation

t1m0thyj
Copy link
Member

@t1m0thyj t1m0thyj commented Oct 18, 2024

What It Does

Fixes #2284, #2311, #2312

  • Refactored ProfileInfo.profileManagerWillLoad so that it works without requiring readFromConfigDisk to be called first.
  • Fixed ProfileInfo.onlyV1ProfilesExist so that it works outside of Zowe CLI when ImperativeConfig.instance.config is undefined. This required changing it from a static method to an instance method.
  • Fixed ConvertV1Profiles API detecting V1 profiles in process.env.ZOWE_CLI_HOME, but not creating V2 config there.

How to Test

  1. Example of testing both an invalid and a good credential manager:
const { ProfileCredentials, ProfileInfo } = require("@zowe/imperative");
(async () => {
    const profInfoBad = new ProfileInfo("zowe", {
        credMgrOverride: ProfileCredentials.defaultCredMgrWithKeytar(() => { throw new Error("bad cred mgr"); }),
    });
    console.log(await profInfoBad.profileManagerWillLoad());
    const profInfoGood = new ProfileInfo("zowe", {
        credMgrOverride: ProfileCredentials.defaultCredMgrWithKeytar(() => require("@zowe/secrets-for-zowe-sdk").keyring),
    });
    console.log(await profInfoGood.profileManagerWillLoad());
})();
  1. (a) Should print false if both V1 profiles and V2 config are present, (b) Should ignore presence of meta files:
const { ProfileInfo } = require("@zowe/imperative");
(async () => {
    const profInfo = new ProfileInfo("zowe");
    await profInfo.readProfilesFromDisk();
    console.log(profInfo.onlyV1ProfilesExist);
})();
  1. Convert V1 profiles with a custom ZOWE_CLI_HOME dir:
export ZOWE_CLI_HOME=/tmp/.zowe
zowe config convert-profiles

Review Checklist
I certify that I have:

Additional Comments

@t1m0thyj t1m0thyj linked an issue Oct 18, 2024 that may be closed by this pull request
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 6 lines in your changes missing coverage. Please review.

Project coverage is 91.27%. Comparing base (c493ad7) to head (fa66054).
Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
...es/imperative/src/config/src/ProfileCredentials.ts 50.00% 4 Missing ⚠️
packages/imperative/src/config/src/ProfileInfo.ts 81.81% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2313      +/-   ##
==========================================
+ Coverage   91.18%   91.27%   +0.09%     
==========================================
  Files         636      636              
  Lines       18057    18066       +9     
  Branches     3781     3784       +3     
==========================================
+ Hits        16465    16490      +25     
+ Misses       1591     1575      -16     
  Partials        1        1              

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

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.

This looks great, thanks for the fixes Timothy! I have a small request about updating typedoc for clarity, other than that the changes LGTM

packages/imperative/src/config/src/ProfileInfo.ts Outdated Show resolved Hide resolved
fix: Ignore events within the app that triggered them
t1m0thyj and others added 2 commits November 11, 2024 10:41
@traeok traeok self-requested a review November 11, 2024 16:08
Copy link

@t1m0thyj t1m0thyj merged commit 83dc71e into master Nov 11, 2024
19 checks passed
@t1m0thyj t1m0thyj deleted the fix/convert-v1-profiles branch November 11, 2024 19:49
@t1m0thyj t1m0thyj added the release-patch Indicates a patch to existing code has been applied label Nov 11, 2024
Copy link

Release succeeded for the master branch. 🎉

The following packages have been published:

Powered by Octorelease 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-patch Indicates a patch to existing code has been applied released
Projects
Status: Closed
5 participants