feat(config): add --merge and --dry-run flags to zowe config import#2712
feat(config): add --merge and --dry-run flags to zowe config import#2712chipset wants to merge 2 commits into
Conversation
- --merge adds missing properties from the imported config into an existing zowe.config.json without overwriting any existing values. Existing values win on all conflicts across profiles, defaults, plugins, and autoStore. - --dry-run previews what the resulting config would look like without writing anything to disk. Works standalone or combined with --merge. - Updated skip message to advertise --merge and --dry-run alongside the existing --overwrite hint. - Added CHANGES.md documenting the new behaviour and merge semantics. Closes #<issue-number> Made-with: Cursor
…e and --dry-run - Unit tests: 10 new cases covering --merge (existing values win, new profiles/defaults/plugins added, autoStore not overwritten, no duplicate plugins, first-time import, dry-run preview), --dry-run (no writes, no schema download), mutual exclusion of --merge and --overwrite, and updated skip message text. - Integration tests: 5 new cases covering --dry-run (no file created, no schema downloaded, preview without write), --merge (field-level correctness on disk, first-time import as-is), and help output asserting --merge and --dry-run flags are listed. - New fixture: test.config.for.merge.json with overlapping and new profiles/defaults used to verify merge behaviour end-to-end. Made-with: Cursor
|
I tested this on my own system based upon a large customer's need to manage multiple configuration files for their developers. Reach out to me if you need the names. Test fixtures are included. |
gejohnston
left a comment
There was a problem hiding this comment.
Thanks for this effort. It looks like a viable, useful enhancement. With a pull request from a forked branch, the effort to confirm or fix the full set of unit, integration, and system tests, and to reach code coverage test objectives falls on the Zowe team. Our product management team will prioritize such an effort. Completion of this pull request will be undertaken based on that prioritization.
In the meantime, I posted a number of comments/questions that can be considered when this work is undertaken.
| : config.api.layers.get(); | ||
|
|
||
| // profiles: deep-merge with existing winning — lodash.mergeWith(existing, incoming) | ||
| target.properties.profiles = lodash.mergeWith( |
There was a problem hiding this comment.
It appears that this logic will work for nexted configs. Has it been tested with nested configs?
Like having both of these profiles in a config:
target.properties.profiles.QA.profiles.zosmf
target.properties.profiles.DEV.profiles.zosmf
There was a problem hiding this comment.
I tested it with nested configs on my box.
| target.properties.profiles, // existing (higher priority, applied second — wins) | ||
| (existingVal: any, _importedVal: any) => { | ||
| // For arrays, keep the existing array as-is | ||
| if (lodash.isArray(existingVal)) { return existingVal; } |
There was a problem hiding this comment.
If an imported config adds a new secure property (like port), that secure port property will not added. Further, if port was previously a plain text property, the plain text port property would not be removed. These choices are understandable and consistent, but:
- Would a site think that this behavior is a bug?
- Can we explain our behavior somewhere?
| ...target.properties.defaults // existing overrides (wins on conflict) | ||
| }; | ||
|
|
||
| // plugins: add any new plugins from the import not already present |
There was a problem hiding this comment.
We add plugin names from the imported config to the target config.
My guess is that the function which we call later:
config.api.layers.set(target.properties)
stores the plugin names into plugins.json. Would that action have any real effect if we do not actually install the new plugins?
Further, since the value of the imported configJson.plugins property probably comes from the local plugins.json file (not from anything from the imported config file itself), does copying and storing the plugins really do anything at all? If not, we might choose to remove the entire plugin paragraph.
| // Print a preview of the would-be result without touching disk | ||
| params.response.console.log( | ||
| TextUtils.chalk.yellow("[Dry Run]") + | ||
| ` The following config would be written to ${layer.path}:\n` |
There was a problem hiding this comment.
We attempt to update plugins at line 100, but dryRun displays no indication that we will update the set of plugins. We should add a statement that we will update plugins. Of course, if we remove our attempt to update plugins (see earlier comment), No change is needed here.
Summary
--merge(-mg): Merges properties from an imported config into the existingzowe.config.jsoninstead of overwriting it. Existing values win on all conflicts — profiles are deep-merged, defaults use existing keys, plugins are appended without duplication, andautoStoreis only set if not already defined.--dry-run(-dr): Previews the resulting config without writing anything to disk. Works standalone (preview a full overwrite) or combined with--merge(preview a safe merge). Schema download is skipped during dry runs.--mergeand--dry-runalongside the existing--overwritehint.CHANGES.mddocumenting the new behaviour, merge semantics, and a behaviour matrix.Motivation
zowe config importpreviously offered only two modes: skip (if the target file already existed) or overwrite (--overwrite). Users with an existingzowe.config.jsonhad no safe way to adopt new defaults or profiles from a shared/team configuration without losing their own settings. This PR closes that gap.Behaviour matrix
--overwrite--merge--merge--dry-run--merge --dry-run--merge --overwriteMerge semantics (field-by-field)
profileslodash.mergeWith; existing property values and arrays are preserveddefaultspluginsautoStoreTest plan
import.handler.unit.test.tscovering all flag combinations, mutual exclusion, skip message text, dry-run output, and field-level merge correctness--dry-run,--merge --dry-run,--mergeon existing file, and--mergefirst-time importtest.config.for.merge.jsonwith overlapping and new profiles/defaults used by integration testszowe config import <url> --merge --dry-runagainst a live profileFiles changed
src/config/cmd/import/import.definition.ts--mergeand--dry-runoptions and three new usage examplessrc/config/cmd/import/import.handler.tssrc/config/cmd/import/CHANGES.md__tests__/config/cmd/import/import.handler.unit.test.ts__tests__/__integration__/.../cli/config/import/cli.imperative-test-cli.config.import.integration.subtest.ts__tests__/__integration__/.../cli/config/import/__resources__/test.config.for.merge.jsonMade with Cursor