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

v2: Port profiles and import fixes #2377

Merged
merged 6 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module.exports = {
],
"rules": {
"@typescript-eslint/no-magic-numbers": "off",
"@typescript-eslint/no-restricted-imports": "off",
"jest/expect-expect": ["warn", {
"assertFunctionNames": ["expect*", "**.*expect*"]
}],
Expand Down Expand Up @@ -63,6 +64,11 @@ module.exports = {
"ignoreEnums": true,
"ignoreReadonlyClassProperties": true
}],
"@typescript-eslint/no-restricted-imports": ["error", {
"patterns": [{
"group": ["**/../lib", "**/../src"]
}]
}],
Comment on lines +67 to +71
Copy link
Member

@traeok traeok Nov 26, 2024

Choose a reason for hiding this comment

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

This lint rule seems really useful - would be nice to use this in Zowe Explorer & ZE API

"@typescript-eslint/no-unused-vars": "off",
"@typescript-eslint/no-var-requires": "off",
"@typescript-eslint/semi": "warn",
Expand Down
5 changes: 5 additions & 0 deletions packages/imperative/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

All notable changes to the Imperative package will be documented in this file.

## Recent Changes

- BugFix: Fixed an issue where the `ProfileInfo.profileManagerWillLoad` method failed if profiles were not yet read from disk. [#2284](https://github.com/zowe/zowe-cli/issues/2284)
- BugFix: Fixed an issue where the `ProfileInfo.updateKnownProperty` method could rewrite team config file to disk without any changes when storing secure value. [#2324](https://github.com/zowe/zowe-cli/issues/2324)

## `5.27.4`

- BugFix: Updated the `cross-spawn` dependency for technical currency. [#2374](https://github.com/zowe/zowe-cli/pull/2374)
Expand Down
2 changes: 1 addition & 1 deletion packages/imperative/src/cmd/src/doc/ICommandDefinition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { IChainedHandlerEntry } from "./handler/IChainedHandlerEntry";
import { ICommandOptionDefinition } from "./option/ICommandOptionDefinition";
import { ICommandPositionalDefinition } from "./option/ICommandPositionalDefinition";
import { ICommandDefinitionPassOn } from "./ICommandDefinitionPassOn";
import { ICommandProfile } from "../../src/doc/profiles/definition/ICommandProfile";
import { ICommandProfile } from "../doc/profiles/definition/ICommandProfile";
/**
* Command Segment type - either "group" or "command".
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*
*/

import { ICommandResponse } from "../../../src/doc/response/response/ICommandResponse";
import { ICommandResponse } from "../../doc/response/response/ICommandResponse";
import { CommandResponse } from "../../response/CommandResponse";

export interface ICommandHandlerResponseValidator {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import { ICommandDefinition } from "../ICommandDefinition";
import { IHelpGenerator } from "../../help/doc/IHelpGenerator";
import { IProfileManagerFactory } from "../../../../profiles";
import { ICommandProfileTypeConfiguration } from "../../../src/doc/profiles/definition/ICommandProfileTypeConfiguration";
import { ICommandProfileTypeConfiguration } from "../../doc/profiles/definition/ICommandProfileTypeConfiguration";
import { Config } from "../../../../config";
import { IDaemonContext } from "../../../../imperative/src/doc/IDaemonContext";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*
*/

import { ICommandOutputFormat } from "../../../../../src/doc/response/response/ICommandOutputFormat";
import { ICommandOutputFormat } from "../../../../doc/response/response/ICommandOutputFormat";

export interface IHandlerFormatOutputApi {
output: (format: ICommandOutputFormat) => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
*
*/

import { CommandProfiles } from "../../../../src/profiles/CommandProfiles";
import { ICommandArguments } from "../../../../src/doc/args/ICommandArguments";
import { CommandProfiles } from "../../../profiles/CommandProfiles";
import { ICommandArguments } from "../../../doc/args/ICommandArguments";
/**
* Command Processor prepare response.
* @export
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { CommandUtils } from "../utils/CommandUtils";
import { ImperativeError } from "../../../error";
import { IHelpGeneratorParms } from "./doc/IHelpGeneratorParms";
import { IHelpGeneratorFactoryParms } from "./doc/IHelpGeneratorFactoryParms";
import { compareCommands, ICommandDefinition } from "../../src/doc/ICommandDefinition";
import { compareCommands, ICommandDefinition } from "../doc/ICommandDefinition";
import stripAnsi = require("strip-ansi");

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { ImperativeError } from "../../../error";
import { Arguments } from "yargs";
import { CommandResponse } from "../response/CommandResponse";
import { ICommandHandlerRequire } from "../doc/handler/ICommandHandlerRequire";
import { ICommandHandler } from "../../src/doc/handler/ICommandHandler";
import { ICommandHandler } from "../doc/handler/ICommandHandler";
import { ICommandProfileTypeConfiguration } from "../doc/profiles/definition/ICommandProfileTypeConfiguration";
import { CommandProfiles } from "./CommandProfiles";
import { ICommandProfileProperty } from "../doc/profiles/definition/ICommandProfileProperty";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { ImperativeExpect } from "../../../expect";
import { IHandlerFormatOutputApi } from "../doc/response/api/handler/IHandlerFormatOutputApi";
import { ICommandOutputFormat, OUTPUT_FORMAT } from "../doc/response/response/ICommandOutputFormat";
import { Arguments } from "yargs";
import { ICommandDefinition } from "../../src/doc/ICommandDefinition";
import { ICommandDefinition } from "../doc/ICommandDefinition";
import { OptionConstants } from "../constants/OptionConstants";
import { inspect } from "util";
import * as DeepMerge from "deepmerge";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { IProfileManagerFactory } from "../../../profiles";
import { ICommandProfileTypeConfiguration } from "../doc/profiles/definition/ICommandProfileTypeConfiguration";
import { IHelpGeneratorFactory } from "../help/doc/IHelpGeneratorFactory";
import { CommandResponse } from "../response/CommandResponse";
import { ICommandResponse } from "../../src/doc/response/response/ICommandResponse";
import { ICommandResponse } from "../doc/response/response/ICommandResponse";
import { ICommandExampleDefinition } from "../..";
import { ImperativeConfig } from "../../../utilities/src/ImperativeConfig";

Expand Down
6 changes: 3 additions & 3 deletions packages/imperative/src/cmd/src/yargs/CommandYargs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import { inspect } from "util";
import { Constants } from "../../../constants";
import { IYargsResponse } from "./doc/IYargsResponse";
import { AbstractCommandYargs, YargsCommandCompleted } from "./AbstractCommandYargs";
import { ICommandOptionDefinition } from "../../src/doc/option/ICommandOptionDefinition";
import { ICommandOptionDefinition } from "../doc/option/ICommandOptionDefinition";
import { ICommandDefinition } from "../doc/ICommandDefinition";
import { CommandProcessor } from "../CommandProcessor";
import { ICommandResponse } from "../../src/doc/response/response/ICommandResponse";
import { CommandResponse } from "../../src/response/CommandResponse";
import { ICommandResponse } from "../doc/response/response/ICommandResponse";
import { CommandResponse } from "../response/CommandResponse";
import { ImperativeConfig } from "../../../utilities";

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*
*/

import { ICommandResponse } from "../../../src/doc/response/response/ICommandResponse";
import { ICommandResponse } from "../../doc/response/response/ICommandResponse";
/**
* Indicates the action performed.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ describe("TeamConfig ProfileInfo tests", () => {
describe("profileManagerWillLoad", () => {
it("should return false if secure credentials fail to load", async () => {
const profInfo = createNewProfInfo(teamProjDir);
jest.spyOn((profInfo as any).mCredentials, "isSecured", "get").mockReturnValueOnce(true);
jest.spyOn((profInfo as any).mCredentials, "loadManager").mockImplementationOnce(async () => {
jest.spyOn((profInfo as any).mCredentials, "isCredentialManagerInAppSettings").mockReturnValueOnce(true);
jest.spyOn((profInfo as any).mCredentials, "activateCredMgrOverride").mockImplementationOnce(async () => {
throw new Error("bad credential manager");
});

Expand All @@ -319,10 +319,10 @@ describe("TeamConfig ProfileInfo tests", () => {
expect(response).toEqual(true);
});

it("should return true if credentials are not secure", async () => {
it("should return true if there is no credential manager", async () => {
// ensure that we are not in the team project directory
const profInfo = createNewProfInfo(origDir);
(profInfo as any).mCredentials = {isSecured: false};
jest.spyOn((profInfo as any).mCredentials, "isCredentialManagerInAppSettings").mockReturnValueOnce(false);
const response = await profInfo.profileManagerWillLoad();
expect(response).toEqual(true);
});
Expand Down Expand Up @@ -1232,7 +1232,7 @@ describe("TeamConfig ProfileInfo tests", () => {
});

describe("updateKnownProperty", () => {
it("should throw and error if the property location type is invalid", async () => {
it("should throw an error if the property location type is invalid", async () => {
const profInfo = createNewProfInfo(teamProjDir);
await profInfo.readProfilesFromDisk();
let caughtError;
Expand Down Expand Up @@ -1271,6 +1271,7 @@ describe("TeamConfig ProfileInfo tests", () => {
const profInfo = createNewProfInfo(teamProjDir);
await profInfo.readProfilesFromDisk();
const jsonPathMatchesSpy = jest.spyOn(ConfigUtils, "jsonPathMatches");
const configSaveSpy = jest.spyOn(profInfo.getTeamConfig(), "save");

const prof = profInfo.mergeArgsForProfile(profInfo.getAllProfiles("dummy")[0]);
const ret = await profInfo.updateKnownProperty({ mergedArgs: prof, property: "host", value: "example.com" });
Expand All @@ -1279,6 +1280,26 @@ describe("TeamConfig ProfileInfo tests", () => {
expect(newHost).toEqual("example.com");
expect(ret).toBe(true);
expect(jsonPathMatchesSpy).toHaveBeenCalled(); // Verify that profile names are matched correctly
expect(configSaveSpy).toHaveBeenCalled();
});

it("should update the given property in the vault and return true", async () => {
const profInfo = createNewProfInfo(teamProjDir);
await profInfo.readProfilesFromDisk();
const jsonPathMatchesSpy = jest.spyOn(ConfigUtils, "jsonPathMatches");
jest.spyOn(profInfo.getTeamConfig().api.secure, "secureFields").mockReturnValue(["profiles.LPAR4.properties.host"]);
const configSaveSpy = jest.spyOn(profInfo.getTeamConfig(), "save");
const configSecureSaveSpy = jest.spyOn(profInfo.getTeamConfig().api.secure, "save");

const prof = profInfo.mergeArgsForProfile(profInfo.getAllProfiles("dummy")[0]);
const ret = await profInfo.updateKnownProperty({ mergedArgs: prof, property: "host", value: "example.com", setSecure: true });
const newHost = profInfo.getTeamConfig().api.layers.get().properties.profiles.LPAR4.properties.host;

expect(newHost).toEqual("example.com");
expect(ret).toBe(true);
expect(jsonPathMatchesSpy).toHaveBeenCalled(); // Verify that profile names are matched correctly
expect(configSaveSpy).not.toHaveBeenCalled();
expect(configSecureSaveSpy).toHaveBeenCalled();
});

it("should remove the given property if the value specified if undefined", async () => {
Expand Down
33 changes: 21 additions & 12 deletions packages/imperative/src/config/src/ProfileCredentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,25 @@
throw new ImperativeError({ msg: "Secure credential storage is not enabled" });
}

await this.activateCredMgrOverride();

if (this.mProfileInfo.usingTeamConfig) {
await this.mProfileInfo.getTeamConfig().api.secure.load({
load: (key: string): Promise<string> => {
return CredentialManagerFactory.manager.load(key, true);

Check warning on line 82 in packages/imperative/src/config/src/ProfileCredentials.ts

View check run for this annotation

Codecov / codecov/patch

packages/imperative/src/config/src/ProfileCredentials.ts#L81-L82

Added lines #L81 - L82 were not covered by tests
},
save: (key: string, value: any): Promise<void> => {
return CredentialManagerFactory.manager.save(key, value);

Check warning on line 85 in packages/imperative/src/config/src/ProfileCredentials.ts

View check run for this annotation

Codecov / codecov/patch

packages/imperative/src/config/src/ProfileCredentials.ts#L84-L85

Added lines #L84 - L85 were not covered by tests
}
});
}
}

/**
* Attempt to initialize `CredentialManagerFactory` with the specified override.
* @internal
*/
public async activateCredMgrOverride(): Promise<void> {
if (!CredentialManagerFactory.initialized) {
try {
// TODO? Make CredentialManagerFactory.initialize params optional
Expand All @@ -86,17 +105,6 @@
});
}
}

if (this.mProfileInfo.usingTeamConfig) {
await this.mProfileInfo.getTeamConfig().api.secure.load({
load: ((key: string): Promise<string> => {
return CredentialManagerFactory.manager.load(key, true);
}),
save: ((key: string, value: any): Promise<void> => {
return CredentialManagerFactory.manager.save(key, value);
})
});
}
}

/**
Expand All @@ -112,8 +120,9 @@
/**
* Check whether a custom CredentialManager is defined in the Imperative
* settings.json file.
* @internal
*/
private isCredentialManagerInAppSettings(): boolean {
public isCredentialManagerInAppSettings(): boolean {
try {
const fileName = path.join(ImperativeConfig.instance.cliHome, "settings", "imperative.json");
let settings: any;
Expand Down
17 changes: 12 additions & 5 deletions packages/imperative/src/config/src/ProfileInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,13 @@ export class ProfileInfo {
this.getTeamConfig().api.layers.activate(osLoc.user, osLoc.global);
}

const updateVaultOnly = options.setSecure && this.getTeamConfig().api.secure.secureFields().includes(toUpdate.argLoc.jsonLoc);
this.getTeamConfig().set(toUpdate.argLoc.jsonLoc, options.value, { secure: options.setSecure });
await this.getTeamConfig().save(false);
if (!updateVaultOnly) {
await this.getTeamConfig().save(false);
} else {
await this.getTeamConfig().api.secure.save(false);
}

if (oldLayer) {
this.getTeamConfig().api.layers.activate(oldLayer.user, oldLayer.global);
Expand Down Expand Up @@ -1005,13 +1010,15 @@ export class ProfileInfo {

//_________________________________________________________________________
/**
* Function to ensure the credential manager will load successfully
* Returns true if it will load, or the credentials are not secured. Returns false if it will not load.
* Checks whether the credential manager will load successfully.
* @returns
* True if it loaded successfully, or there is no credential manager
* configured in Imperative settings.json
*/
public async profileManagerWillLoad(): Promise<boolean> {
if (this.mCredentials.isSecured) {
if (this.mCredentials.isCredentialManagerInAppSettings()) {
try {
await this.mCredentials.loadManager();
await this.mCredentials.activateCredMgrOverride();
return true;
} catch (err) {
this.mImpLogger.warn("Failed to initialize secure credential manager: " + err.message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ function formatLogLevelMsg(logTypeName: string) {
export const probTests: IProbTest[] = [
{
itemId: ItemId.NODEJS_VER,
probExpr: "semver.satisfies('{val}', '<18.x || 19.x || >=21.x')",
probMsg: "Only Node.js versions 18 and 20 are supported."
probExpr: "semver.satisfies('{val}', '<18.x || 19.x || 21.x || >=23.x')",
probMsg: "Only Node.js versions 18, 20, and 22 are supported."
},
{
itemId: ItemId.NPM_VER,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
*
*/

import { IImperativeConfig } from "../../src/doc/IImperativeConfig";
import { UpdateImpConfig } from "../../src/UpdateImpConfig";
import { IImperativeConfig } from "../doc/IImperativeConfig";
import { UpdateImpConfig } from "../UpdateImpConfig";
import { isAbsolute, join } from "path";
import { ImperativeConfig, JsUtils } from "../../../utilities";
import { Logger } from "../../../logger";
Expand Down
2 changes: 1 addition & 1 deletion packages/zostso/src/doc/IIssueResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export interface IIssueResponse {
startResponse: IStartStopResponses;

/**
* Indicates if started TSO containes "READY " message
* Indicates if started TSO contains "READY" message
* @type {boolean}
* @memberof IIssueResponse
*/
Expand Down
Loading