From dd43e62a89b6e68f5d5a717ff4df7075aab88d1c Mon Sep 17 00:00:00 2001 From: aadityasinha-dotcom Date: Mon, 31 Jul 2023 19:05:29 +0530 Subject: [PATCH 1/8] added functionality to merge layers than load profile props Signed-off-by: aadityasinha-dotcom --- src/core/zowe/core_for_zowe_sdk/profile_manager.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/core/zowe/core_for_zowe_sdk/profile_manager.py b/src/core/zowe/core_for_zowe_sdk/profile_manager.py index 1e6d49ca..a668bbaf 100644 --- a/src/core/zowe/core_for_zowe_sdk/profile_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/profile_manager.py @@ -13,6 +13,7 @@ import os.path import warnings from typing import Optional +from deepmerge import always_merger from .config_file import ConfigFile, Profile from .custom_warnings import ( @@ -212,10 +213,17 @@ def load( missing_secure_props = [] # track which secure props were not loaded + loaded_cfg: dict = {} + for i, (config_type, cfg) in enumerate(config_layers.items()): + loaded_cfg = always_merger.merge(loaded_cfg, cfg.profiles) + if loaded_cfg: + cfg.profiles = loaded_cfg + profile_loaded = self.get_profile( cfg, profile_name, profile_type, config_type ) + # How about we update by iterating each layer and at last we will get the merged layer # TODO Why don't user and password show up here for Project User Config? # Probably need to update load_profile_properties method in config_file.py if profile_loaded.name and not profile_name: From e5a754f57d483e976f898f6a29e531be45c8dea4 Mon Sep 17 00:00:00 2001 From: aadityasinha-dotcom Date: Mon, 31 Jul 2023 19:09:04 +0530 Subject: [PATCH 2/8] added changelog Signed-off-by: aadityasinha-dotcom --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c57aa5de..8589b6f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,3 +2,6 @@ All notable changes to the Zowe Client Python SDK will be documented in this file. +## Recent Changes + +- Bug: Fixed profile merge order to match Node.js SDK \ No newline at end of file From 86538dfc7751b297970935d79dc5f6555f06eaba Mon Sep 17 00:00:00 2001 From: aadityasinha-dotcom Date: Mon, 31 Jul 2023 19:14:03 +0530 Subject: [PATCH 3/8] requirements.txt updated Signed-off-by: aadityasinha-dotcom --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 0c52c06a..d0290d4e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,6 +3,7 @@ chardet==4.0.0 colorama==0.4.4 commentjson==0.9.0 coverage==5.4 +deepmerge==1.1.0 flake8==3.8.4 idna==2.10 importlib-metadata==3.6.0 From 45c368420e9b3a421cef8b777f299edc8fed367b Mon Sep 17 00:00:00 2001 From: aadityasinha-dotcom Date: Wed, 16 Aug 2023 11:52:23 +0530 Subject: [PATCH 4/8] refactored the merging logic for profile properties Signed-off-by: aadityasinha-dotcom --- .../zowe/core_for_zowe_sdk/profile_manager.py | 16 +++++++++++----- tests/unit/test_zowe_core.py | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/profile_manager.py b/src/core/zowe/core_for_zowe_sdk/profile_manager.py index dc496068..18a9b221 100644 --- a/src/core/zowe/core_for_zowe_sdk/profile_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/profile_manager.py @@ -263,9 +263,6 @@ def load( loaded_cfg: dict = {} for i, (config_type, cfg) in enumerate(config_layers.items()): - loaded_cfg = always_merger.merge(loaded_cfg, cfg.profiles) - if loaded_cfg: - cfg.profiles = loaded_cfg profile_loaded = self.get_profile( cfg, profile_name, profile_type, config_type @@ -281,12 +278,21 @@ def load( missing_secure_props.extend(profile_loaded.missing_secure_props) - if override_with_env: - env_var = {**self.get_env(cfg)} if i == 1 and profile_props: break # Skip loading from global config if profile was found in project config + usrProject = self.project_user_config.profiles + project = self.project_config.profiles + prjt = always_merger.merge(usrProject, project) + + usrGlobal = self.global_user_config.profiles + glbal = self.global_config.profiles + glbl = always_merger.merge(usrGlobal, glbal) + + if override_with_env: + env_var = {**self.get_env(cfg)} + if profile_type != BASE_PROFILE: profile_props = { **self.load(profile_type=BASE_PROFILE, check_missing_props=False), diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 518c51c5..d04fda1f 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -320,7 +320,7 @@ def test_profile_loading_with_user_overriden_properties(self, get_pass_func): self.assertEqual(prof_manager.config_filepath, cwd_up_file_path) expected_props = { - "host": "zowe.test.user.cloud", + "host": "zowe.test.cloud", "rejectUnauthorized": False, "user": "user", "password": "password", From 49221bdbcbdb8f1fe84447f3d0094179e6fa5d89 Mon Sep 17 00:00:00 2001 From: aadityasinha-dotcom Date: Thu, 21 Sep 2023 17:22:11 +0530 Subject: [PATCH 5/8] created copies using deepcopy before performing merge Signed-off-by: aadityasinha-dotcom --- src/core/zowe/core_for_zowe_sdk/profile_manager.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/core/zowe/core_for_zowe_sdk/profile_manager.py b/src/core/zowe/core_for_zowe_sdk/profile_manager.py index 18a9b221..aa49c828 100644 --- a/src/core/zowe/core_for_zowe_sdk/profile_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/profile_manager.py @@ -15,6 +15,7 @@ import warnings from typing import Optional from deepmerge import always_merger +from copy import deepcopy from .config_file import ConfigFile, Profile from .custom_warnings import ( @@ -284,10 +285,18 @@ def load( usrProject = self.project_user_config.profiles project = self.project_config.profiles + + # Creating copies of the usrProject and project objects + usrProjectCopy = deepcopy(usrProject) + projectCopy = deepcopy(project) prjt = always_merger.merge(usrProject, project) usrGlobal = self.global_user_config.profiles glbal = self.global_config.profiles + + # Creating copies of the usrGlobal and glbal objects + usrGlobalCopy = deepcopy(usrGlobal) + glbalCopy = deepcopy(glbal) glbl = always_merger.merge(usrGlobal, glbal) if override_with_env: From 1f4821ad4340ffc2b5efe054360a65d1db049782 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 28 Sep 2023 16:15:24 -0400 Subject: [PATCH 6/8] WIP Update profile merging to match Node SDK Signed-off-by: Timothy Johnson --- .../zowe/core_for_zowe_sdk/config_file.py | 25 ++++-- .../zowe/core_for_zowe_sdk/profile_manager.py | 81 ++++++++----------- 2 files changed, 50 insertions(+), 56 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index b9e01b3d..a2d47581 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -120,7 +120,14 @@ def init_from_file( setting filepath (or if not set, autodiscover the file) """ if self.filepath is None: - self.autodiscover_config_dir() + try: + self.autodiscover_config_dir() + except FileNotFoundError: + pass + + if self.filepath is None or not os.path.isfile(self.filepath): + warnings.warn(f"Config file does not exist at {self.filepath}") + return with open(self.filepath, encoding="UTF-8", mode="r") as fileobj: profile_jsonc = commentjson.load(fileobj) @@ -132,10 +139,9 @@ def init_from_file( if self.schema_property and validate_schema: self.validate_schema() - # loading secure props is done in load_profile_properties - # since we want to try loading secure properties only when - # we know that the profile has saved properties - # self.load_secure_props() + + CredentialManager.load_secure_props() + self.secure_props = CredentialManager.secure_props.get(self.filepath, {}) def validate_schema( self @@ -355,9 +361,7 @@ def load_profile_properties(self, profile_name: str) -> dict: # load secure props only if there are secure fields - if secure_fields: - CredentialManager.load_secure_props() - self.secure_props = CredentialManager.secure_props.get(self.filepath, {}) + if secure_fields and self.secure_props: # load properties with key as profile.{profile_name}.properties.{*} for (key, value) in self.secure_props.items(): if re.match( @@ -372,3 +376,8 @@ def load_profile_properties(self, profile_name: str) -> dict: # self._missing_secure_props.extend(secure_fields) return props + + def load_secure_properties(self, profile_name: str): + secure_props = {} + lst = profile_name.split(".") + diff --git a/src/core/zowe/core_for_zowe_sdk/profile_manager.py b/src/core/zowe/core_for_zowe_sdk/profile_manager.py index 2e8a4eec..23c6e695 100644 --- a/src/core/zowe/core_for_zowe_sdk/profile_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/profile_manager.py @@ -162,7 +162,6 @@ def get_profile( cfg: ConfigFile, profile_name: Optional[str], profile_type: Optional[str], - config_type: Optional[str], validate_schema: Optional[bool] = True, ) -> Profile: """ @@ -232,7 +231,7 @@ def get_profile( ) except Exception as exc: warnings.warn( - f"Could not load {config_type} '{cfg.filename}' at '{cfg.filepath}'" + f"Could not load '{cfg.filename}' at '{cfg.filepath}'" f"because {type(exc).__name__}'{exc}'.", ConfigNotFoundWarning, ) @@ -273,58 +272,44 @@ def load( if not self._show_warnings: warnings.simplefilter("ignore") - config_layers = { - "Project User Config": self.project_user_config, - "Project Config": self.project_config, - "Global User Config": self.global_user_config, - "Global Config": self.global_config, - } profile_props: dict = {} - schema_path = None env_var: dict = {} - missing_secure_props = [] # track which secure props were not loaded - loaded_cfg: dict = {} - - for i, (config_type, cfg) in enumerate(config_layers.items()): - - profile_loaded = self.get_profile( - cfg, profile_name, profile_type, config_type, validate_schema - ) - # How about we update by iterating each layer and at last we will get the merged layer - # TODO Why don't user and password show up here for Project User Config? - # Probably need to update load_profile_properties method in config_file.py - if profile_loaded.name and not profile_name: - profile_name = ( - profile_loaded.name - ) # Define profile name that will be merged from other layers - profile_props = {**profile_loaded.data, **profile_props} - + defaults_merged: dict = {} + profiles_merged: dict = {} + cfg_name = None + + for cfg_layer in (self.project_user_config, self.project_config, self.global_user_config, self.global_config): + if cfg_layer.profiles is None: + cfg_layer.init_from_file(validate_schema) + if cfg_layer.defaults: + for name, value in cfg_layer.defaults.items(): + defaults_merged[name] = defaults_merged.get(name, value) + if not cfg_name and cfg_layer.name: + cfg_name = cfg_layer.name + + usrProject = self.project_user_config.profiles or {} + project = self.project_config.profiles or {} + project_temp = always_merger.merge(deepcopy(usrProject), project) + + usrGlobal = self.global_user_config.profiles or {} + glbal = self.global_config.profiles or {} + global_temp = always_merger.merge(deepcopy(usrGlobal), glbal) + + profiles_merged = project_temp + for name, value in global_temp.items(): + if name not in profiles_merged: + profiles_merged[name] = value + + cfg = ConfigFile(type="Merged Config", name=cfg_name, profiles=profiles_merged, defaults=defaults_merged) + profile_loaded = self.get_profile(cfg, profile_name, profile_type, validate_schema) + if profile_loaded: + profile_props = profile_loaded.data missing_secure_props.extend(profile_loaded.missing_secure_props) - - if i == 1 and profile_props: - break # Skip loading from global config if profile was found in project config - - usrProject = self.project_user_config.profiles - project = self.project_config.profiles - - # Creating copies of the usrProject and project objects - usrProjectCopy = deepcopy(usrProject) - projectCopy = deepcopy(project) - prjt = always_merger.merge(usrProject, project) - - usrGlobal = self.global_user_config.profiles - glbal = self.global_config.profiles - - # Creating copies of the usrGlobal and glbal objects - usrGlobalCopy = deepcopy(usrGlobal) - glbalCopy = deepcopy(glbal) - glbl = always_merger.merge(usrGlobal, glbal) - if override_with_env: - env_var = {**self.get_env(cfg)} + env_var = {**self.get_env(cfg)} if profile_type != BASE_PROFILE: profile_props = { @@ -343,7 +328,7 @@ def load( warnings.resetwarnings() - for k, v in profile_props.items(): + for k in profile_props.keys(): if k in env_var: profile_props[k] = env_var[k] From ae023642b14f88b11722224e8e578dfb892e23f8 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Fri, 29 Sep 2023 14:47:20 -0400 Subject: [PATCH 7/8] Fix secure property loading and update unit tests Signed-off-by: Timothy Johnson --- .../zowe/core_for_zowe_sdk/config_file.py | 111 +++++++------ .../zowe/core_for_zowe_sdk/profile_manager.py | 53 +++---- tests/unit/test_zowe_core.py | 149 ++++++++++-------- 3 files changed, 156 insertions(+), 157 deletions(-) diff --git a/src/core/zowe/core_for_zowe_sdk/config_file.py b/src/core/zowe/core_for_zowe_sdk/config_file.py index 842d0bfc..f5127d48 100644 --- a/src/core/zowe/core_for_zowe_sdk/config_file.py +++ b/src/core/zowe/core_for_zowe_sdk/config_file.py @@ -15,6 +15,7 @@ import json import requests import warnings +from copy import deepcopy from dataclasses import dataclass, field from typing import Optional, NamedTuple @@ -36,9 +37,9 @@ HOME = os.path.expanduser("~") -GLOBAl_CONFIG_LOCATION = os.path.join(HOME, ".zowe") +GLOBAL_CONFIG_LOCATION = os.path.join(HOME, ".zowe") GLOBAL_CONFIG_PATH = os.path.join( - GLOBAl_CONFIG_LOCATION, f"{GLOBAL_CONFIG_NAME}.config.json" + GLOBAL_CONFIG_LOCATION, f"{GLOBAL_CONFIG_NAME}.config.json" ) CURRENT_DIR = os.getcwd() @@ -143,7 +144,7 @@ def init_from_file( self.validate_schema() CredentialManager.load_secure_props() - self.secure_props = CredentialManager.secure_props.get(self.filepath, {}) + self.__load_secure_properties() def validate_schema( self @@ -358,23 +359,39 @@ def load_profile_properties(self, profile_name: str) -> dict: ) lst.pop() - # load secure props only if there are secure fields - if secure_fields and self.secure_props: - # load properties with key as profile.{profile_name}.properties.{*} - for (key, value) in self.secure_props.items(): - if re.match( - "profiles\\." + profile_name + "\\.properties\\.[a-z]+", key - ): - property_name = key.split(".")[-1] - if property_name in secure_fields: - props[property_name] = value - secure_fields.remove(property_name) - - # if len(secure_fields) > 0: - # self._missing_secure_props.extend(secure_fields) - return props + def __load_secure_properties(self): + """ + Inject secure properties that have been loaded from the vault into the profiles object. + """ + secure_props = CredentialManager.secure_props.get(self.filepath, {}) + for key, value in secure_props.items(): + segments = [name for i, name in enumerate(key.split(".")) if i % 2 == 1] + profiles_obj = self.profiles + property_name = segments.pop() + for i, profile_name in enumerate(segments): + if profile_name in profiles_obj: + profiles_obj = profiles_obj[profile_name] + if i == len(segments) - 1: + profiles_obj.setdefault("properties", {}) + profiles_obj["properties"][property_name] = value + else: + break + + def __extract_secure_properties(self, profiles_obj, json_path="profiles"): + """ + Extract secure properties from the profiles object so they can be saved to the vault. + """ + secure_props = {} + for key, value in profiles_obj.items(): + for property_name in value.get("secure", []): + if property_name in value.get("properties", {}): + secure_props[f"{json_path}.{key}.properties.{property_name}"] = value["properties"].pop(property_name) + if value.get("profiles"): + secure_props.update(self.__extract_secure_properties(value["profiles"], f"{json_path}.{key}.profiles")) + return secure_props + def __set_or_create_nested_profile(self, profile_name, profile_data): """ Set or create a nested profile. @@ -426,20 +443,11 @@ def set_property(self, json_path, value, secure=None) -> None: current_profile = self.find_profile(profile_name, self.profiles) or {} current_properties = current_profile.setdefault("properties", {}) current_secure = current_profile.setdefault("secure", []) - if is_secure: - CredentialManager.load_secure_props() - if not is_property_secure: - current_secure.append(property_name) - - CredentialManager.secure_props[self.filepath] = { - **CredentialManager.secure_props.get(self.filepath, {}), json_path: value} - current_properties.pop(property_name, None) - - else: - if is_property_secure: - CredentialManager.secure_props[self.filepath].pop(json_path,None) - current_secure.remove(property_name) - current_properties[property_name] = value + current_properties[property_name] = value + if is_secure and not is_property_secure: + current_secure.append(property_name) + elif not is_secure and is_property_secure: + current_secure.remove(property_name) current_profile["properties"] = current_properties current_profile["secure"] = current_secure @@ -459,49 +467,40 @@ def set_profile(self, profile_path: str, profile_data: dict) -> None: if "secure" in profile_data: # Checking if the profile has a 'secure' field with values secure_fields = profile_data["secure"] - current_profile = self.find_profile(profile_name,self.profiles) or {} + current_profile = self.find_profile(profile_name,self.profiles) or {} existing_secure_fields = current_profile.get("secure", []) new_secure_fields = [field for field in secure_fields if field not in existing_secure_fields] - # JSON paths for new secure properties and store their values in CredentialManager.secure_props - CredentialManager.load_secure_props() - CredentialManager.secure_props[self.filepath] = {} - for field in new_secure_fields: - json_path = f"{profile_path}.properties.{field}" - profile_value = profile_data["properties"][field] - CredentialManager.secure_props[self.filepath][json_path] = profile_value # Updating the 'secure' field of the profile with the combined list of secure fields profile_data["secure"] = existing_secure_fields + new_secure_fields # If a field is provided in the 'secure' list and its value exists in 'profile_data', remove it profile_data["properties"] = { - field: value - for field, value in profile_data.get("properties", {}).items() - if field not in profile_data["secure"] + **current_profile.get("properties", {}), + **profile_data.get("properties", {}), } self.__set_or_create_nested_profile(profile_name, profile_data) - def save(self, secure_props=True): + def save(self, update_secure_props=True): """ Save the config file to disk. and secure props to vault parameters: - secure_props (bool): If True, the secure properties will be stored in the vault. Default is False. + secure_props (bool): If True, the secure properties will be stored in the vault. Default is True. Returns: None """ # Updating the config file with any changes - if self.profiles is None: - try: - self.init_from_file() - except FileNotFoundError: - pass + if not any(self.profiles.values()): + return - elif any(self.profiles.values()): - with open(self.filepath, 'w') as file: - self.jsonc["profiles"] = self.profiles - commentjson.dump(self.jsonc, file, indent=4) - if secure_props: - CredentialManager.save_secure_props() + profiles_temp = deepcopy(self.profiles) + secure_props = self.__extract_secure_properties(profiles_temp) + CredentialManager.secure_props[self.filepath] = secure_props + with open(self.filepath, 'w') as file: + self.jsonc["profiles"] = profiles_temp + commentjson.dump(self.jsonc, file, indent=4) + if update_secure_props: + CredentialManager.save_secure_props() def get_profile_name_from_path(self, path: str) -> str: diff --git a/src/core/zowe/core_for_zowe_sdk/profile_manager.py b/src/core/zowe/core_for_zowe_sdk/profile_manager.py index 4a2d37ce..d3da6882 100644 --- a/src/core/zowe/core_for_zowe_sdk/profile_manager.py +++ b/src/core/zowe/core_for_zowe_sdk/profile_manager.py @@ -214,24 +214,6 @@ def get_profile( f" instead.", ProfileNotFoundWarning, ) - except SecureProfileLoadFailed: - warnings.warn( - f"Config '{cfg.filename}' has no saved secure properties.", - SecurePropsNotFoundWarning, - ) - except SecurePropsNotFoundWarning: - if profile_name: - warnings.warn( - f"Secure properties of profile '{profile_name}' from file '{cfg.filename}' were not found " - f"hence profile not loaded.", - SecurePropsNotFoundWarning, - ) - else: - warnings.warn( - f"Secure properties of profile type '{profile_type}' from file '{cfg.filename}' were not found " - f"hence profile not loaded.", - SecurePropsNotFoundWarning, - ) except Exception as exc: warnings.warn( f"Could not load '{cfg.filename}' at '{cfg.filepath}'" @@ -282,30 +264,39 @@ def load( defaults_merged: dict = {} profiles_merged: dict = {} cfg_name = None + cfg_schema = None for cfg_layer in (self.project_user_config, self.project_config, self.global_user_config, self.global_config): if cfg_layer.profiles is None: - cfg_layer.init_from_file(validate_schema) + try: + cfg_layer.init_from_file(validate_schema) + except SecureProfileLoadFailed: + warnings.warn( + f"Could not load secure properties for {cfg_layer.filepath}", + SecurePropsNotFoundWarning, + ) if cfg_layer.defaults: for name, value in cfg_layer.defaults.items(): defaults_merged[name] = defaults_merged.get(name, value) if not cfg_name and cfg_layer.name: cfg_name = cfg_layer.name + if not cfg_schema and cfg_layer.schema_property: + cfg_schema = cfg_layer.schema_property usrProject = self.project_user_config.profiles or {} project = self.project_config.profiles or {} - project_temp = always_merger.merge(deepcopy(usrProject), project) + project_temp = always_merger.merge(deepcopy(project), usrProject) usrGlobal = self.global_user_config.profiles or {} - glbal = self.global_config.profiles or {} - global_temp = always_merger.merge(deepcopy(usrGlobal), glbal) + global_ = self.global_config.profiles or {} + global_temp = always_merger.merge(deepcopy(global_), usrGlobal) profiles_merged = project_temp for name, value in global_temp.items(): if name not in profiles_merged: profiles_merged[name] = value - cfg = ConfigFile(type="Merged Config", name=cfg_name, profiles=profiles_merged, defaults=defaults_merged) + cfg = ConfigFile(type="Merged Config", name=cfg_name, profiles=profiles_merged, defaults=defaults_merged, schema_property=cfg_schema) profile_loaded = self.get_profile(cfg, profile_name, profile_type, validate_schema) if profile_loaded: profile_props = profile_loaded.data @@ -331,7 +322,7 @@ def load( warnings.resetwarnings() - for k in profile_props.keys(): + for k in profile_props: if k in env_var: profile_props[k] = env_var[k] @@ -358,7 +349,7 @@ def get_highest_priority_layer(self, json_path: str) -> Optional[ConfigFile]: ] original_name = layers[0].get_profile_name_from_path(json_path) - + for layer in layers: try: layer.init_from_file() @@ -385,10 +376,10 @@ def get_highest_priority_layer(self, json_path: str) -> Optional[ConfigFile]: if highest_layer is None: raise FileNotFoundError(f"Could not find a valid layer for {json_path}") - + return highest_layer - - + + def set_property(self, json_path, value, secure=None) -> None: """ Set a property in the profile, storing it securely if necessary. @@ -401,7 +392,7 @@ def set_property(self, json_path, value, secure=None) -> None: # highest priority layer for the given profile name highest_priority_layer = self.get_highest_priority_layer(json_path) - + # Set the property in the highest priority layer highest_priority_layer.set_property(json_path, value, secure=secure) @@ -417,7 +408,7 @@ def set_profile(self, profile_path: str, profile_data: dict) -> None: highest_priority_layer = self.get_highest_priority_layer(profile_path) highest_priority_layer.set_profile(profile_path, profile_data) - + def save(self) -> None: """ Save the layers (configuration files) to disk. @@ -426,7 +417,7 @@ def save(self) -> None: self.project_config, self.global_user_config, self.global_config] - + for layer in layers: layer.save(False) CredentialManager.save_secure_props() diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index aa8dd634..5ef2adc1 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -13,7 +13,6 @@ from jsonschema import validate, ValidationError, SchemaError from pyfakefs.fake_filesystem_unittest import TestCase from unittest import mock -from unittest.mock import call, patch from zowe.core_for_zowe_sdk.validators import validate_config_json from zowe.core_for_zowe_sdk import ( @@ -242,7 +241,7 @@ def setUpCreds(self, file_path, secure_props): global SECURE_CONFIG_PROPS SECURE_CONFIG_PROPS = base64.b64encode((json.dumps(CRED_DICT)).encode()).decode() - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_autodiscovery_and_base_profile_loading(self, get_pass_func): """ Test loading of correct file by autodiscovering from current working directory @@ -274,7 +273,7 @@ def test_autodiscovery_and_base_profile_loading(self, get_pass_func): } self.assertEqual(props, expected_props) - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_custom_file_and_custom_profile_loading(self, get_pass_func): """ Test loading of correct file given a filename and directory, @@ -305,7 +304,7 @@ def test_custom_file_and_custom_profile_loading(self, get_pass_func): } self.assertEqual(props, expected_props) - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_custom_file_and_custom_profile_loading_with_nested_profile(self, get_pass_func): """ Test loading of correct file given a filename and directory, @@ -334,8 +333,8 @@ def test_custom_file_and_custom_profile_loading_with_nested_profile(self, get_pa } self.assertEqual(props, expected_props) - @patch("keyring.get_password", side_effect=keyring_get_password) - def test_profile_loading_with_user_overriden_properties(self, get_pass_func): + @mock.patch("keyring.get_password", side_effect=keyring_get_password) + def test_profile_loading_with_user_overridden_properties(self, get_pass_func): """ Test overriding of properties from user config, also load by profile_name correctly populating fields from base profile @@ -359,7 +358,7 @@ def test_profile_loading_with_user_overriden_properties(self, get_pass_func): self.assertEqual(prof_manager.config_filepath, cwd_up_file_path) expected_props = { - "host": "zowe.test.cloud", + "host": "zowe.test.user.cloud", "rejectUnauthorized": False, "user": "user", "password": "password", @@ -367,7 +366,7 @@ def test_profile_loading_with_user_overriden_properties(self, get_pass_func): } self.assertEqual(props, expected_props) - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_profile_loading_exception(self, get_pass_func): """ Test correct exceptions are being thrown when a profile is @@ -388,7 +387,7 @@ def test_profile_loading_exception(self, get_pass_func): config_file = ConfigFile(name=self.custom_appname, type="team_config") props: dict = config_file.get_profile(profile_name="non_existent_profile", validate_schema=False) - @patch("keyring.get_password", side_effect=keyring_get_password_exception) + @mock.patch("keyring.get_password", side_effect=keyring_get_password_exception) def test_secure_props_loading_warning(self, get_pass_func): """ Test correct warnings are being thrown when secure properties @@ -404,9 +403,9 @@ def test_secure_props_loading_warning(self, get_pass_func): # Test prof_manager = ProfileManager() prof_manager.config_dir = self.custom_dir - props: dict = prof_manager.load("base") + props: dict = prof_manager.load("base", validate_schema=False) - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_profile_not_found_warning(self, get_pass_func): """ Test correct warnings are being thrown when profile is not found @@ -424,8 +423,8 @@ def test_profile_not_found_warning(self, get_pass_func): prof_manager.config_dir = self.custom_dir props: dict = prof_manager.load("non_existent_profile", validate_schema=False) - @patch("sys.platform", "win32") - @patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") + @mock.patch("sys.platform", "win32") + @mock.patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") def test_load_secure_props(self, retrieve_cred_func): """ Test loading secure_props from keyring or storage. @@ -456,8 +455,8 @@ def test_load_secure_props(self, retrieve_cred_func): expected_secure_props = credential self.assertEqual(credential_manager.secure_props, expected_secure_props) - @patch("sys.platform", "win32") - @patch("keyring.delete_password") + @mock.patch("sys.platform", "win32") + @mock.patch("keyring.delete_password") def test_delete_credential(self, delete_pass_func): """ Test the delete_credential method for deleting credentials from keyring. @@ -483,8 +482,8 @@ def side_effect(*args, **kwargs): ] delete_pass_func.assert_has_calls(expected_calls) - @patch("sys.platform", "win32") - @patch("keyring.get_password", side_effect=["password", None, "part1", "part2\0", None]) + @mock.patch("sys.platform", "win32") + @mock.patch("keyring.get_password", side_effect=["password", None, "part1", "part2\0", None]) def test_retrieve_credential(self, get_pass_func): """ Test the _retrieve_credential method for retrieving credentials from keyring. @@ -508,8 +507,8 @@ def test_retrieve_credential(self, get_pass_func): get_pass_func.assert_any_call(f"{service_name}-1", f"{constants['ZoweAccountName']}-1") get_pass_func.assert_any_call(f"{service_name}-2", f"{constants['ZoweAccountName']}-2") - @patch("sys.platform", "win32") - @patch("keyring.get_password", side_effect=[None,None]) + @mock.patch("sys.platform", "win32") + @mock.patch("keyring.get_password", side_effect=[None,None]) def test_retrieve_credential_encoding_errors(self, get_pass_func): """ Test the _retrieve_credential method for handling encoding errors and None values. @@ -520,10 +519,10 @@ def test_retrieve_credential_encoding_errors(self, get_pass_func): get_pass_func.assert_called_with(f"{service_name}-1", f"{constants['ZoweAccountName']}-1") - @patch("sys.platform", "win32") - @patch("keyring.set_password") - @patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") - @patch("zowe.core_for_zowe_sdk.CredentialManager.delete_credential") + @mock.patch("sys.platform", "win32") + @mock.patch("keyring.set_password") + @mock.patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") + @mock.patch("zowe.core_for_zowe_sdk.CredentialManager.delete_credential") def test_save_secure_props_normal_credential(self, delete_pass_func, retrieve_cred_func, set_pass_func): """ Test the save_secure_props method for saving credentials to keyring. @@ -558,10 +557,10 @@ def test_save_secure_props_normal_credential(self, delete_pass_func, retrieve_cr encoded_credential ) - @patch("sys.platform", "win32") - @patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") - @patch("keyring.set_password") - @patch("zowe.core_for_zowe_sdk.CredentialManager.delete_credential") + @mock.patch("sys.platform", "win32") + @mock.patch("zowe.core_for_zowe_sdk.CredentialManager._retrieve_credential") + @mock.patch("keyring.set_password") + @mock.patch("zowe.core_for_zowe_sdk.CredentialManager.delete_credential") def test_save_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, retrieve_cred_func): # Set up mock values and expected results @@ -604,7 +603,7 @@ def test_save_secure_props_exceed_limit(self, delete_pass_func, set_pass_func, r )) set_pass_func.assert_has_calls(expected_calls) - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_profile_loading_with_valid_schema(self, get_pass_func): """ Test Validation, no error should be raised for valid schema @@ -625,7 +624,7 @@ def test_profile_loading_with_valid_schema(self, get_pass_func): prof_manager.config_dir = self.custom_dir props: dict = prof_manager.load(profile_name="zosmf") - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_profile_loading_with_invalid_schema(self, get_pass_func): """ Test Validation, no error should be raised for valid schema @@ -647,7 +646,7 @@ def test_profile_loading_with_invalid_schema(self, get_pass_func): prof_manager.config_dir = self.custom_dir props: dict = prof_manager.load(profile_name="zosmf", validate_schema=True) - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_profile_loading_with_invalid_schema_internet_URI(self, get_pass_func): """ Test Validation, no error should be raised for valid schema @@ -669,7 +668,7 @@ def test_profile_loading_with_invalid_schema_internet_URI(self, get_pass_func): prof_manager.config_dir = self.custom_dir props: dict = prof_manager.load(profile_name="zosmf", validate_schema=True) - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_profile_loading_with_env_variables(self, get_pass_func): """ Test loading of correct file given a filename and directory, @@ -708,18 +707,18 @@ def test_get_highest_priority_layer(self): project_user_config = mock.MagicMock(spec=ConfigFile) project_user_config.find_profile.return_value = mock.MagicMock() project_user_config.find_profile.return_value.data = {"profiles": "zosmf"} - + # Set up the ProfileManager profile_manager = ProfileManager() profile_manager.project_user_config = project_user_config project_user_config.get_profile_name_from_path.return_value = "zosmf" # Call the function being tested result_layer = profile_manager.get_highest_priority_layer("zosmf") - + # Assert the results self.assertEqual(result_layer, project_user_config) - @patch("zowe.core_for_zowe_sdk.ProfileManager.get_highest_priority_layer") + @mock.patch("zowe.core_for_zowe_sdk.ProfileManager.get_highest_priority_layer") def test_profile_manager_set_property(self, get_highest_priority_layer_mock): """ Test that set_property calls the set_property method of the highest priority layer. @@ -744,18 +743,18 @@ def test_profile_manager_set_property(self, get_highest_priority_layer_mock): highest_priority_layer_mock.set_property.assert_called_with(json_path, value, secure=secure) - @patch("zowe.core_for_zowe_sdk.ConfigFile.save") - @patch("zowe.core_for_zowe_sdk.CredentialManager.save_secure_props") + @mock.patch("zowe.core_for_zowe_sdk.ConfigFile.save") + @mock.patch("zowe.core_for_zowe_sdk.CredentialManager.save_secure_props") def test_profile_manager_save(self, mock_save_secure_props, mock_save): """ Test that save calls the save method of all layers. """ profile_manager = ProfileManager() profile_manager.save() - expected_calls = [call(False) for _ in range(4)] + expected_calls = [mock.call(False) for _ in range(4)] mock_save.assert_has_calls(expected_calls) - mock_save_secure_props.assert_called_once() - + mock_save_secure_props.assert_called_once() + @mock.patch("zowe.core_for_zowe_sdk.ProfileManager.get_highest_priority_layer") def test_profile_manager_set_profile(self, get_highest_priority_layer_mock): """ @@ -789,7 +788,7 @@ def test_set_or_create_nested_profile(self, mock_get_profile_path): "properties": { "user": "samadpls", "password": "password1" - } + } } config_file._ConfigFile__set_or_create_nested_profile("zosmf", profile_data) expected_profiles = { @@ -821,7 +820,7 @@ def test_is_secure(self, mock_find_profile): @mock.patch("zowe.core_for_zowe_sdk.ConfigFile.get_profile_name_from_path") @mock.patch("zowe.core_for_zowe_sdk.ConfigFile.find_profile") @mock.patch("zowe.core_for_zowe_sdk.ConfigFile._ConfigFile__is_secure") - @patch("keyring.get_password", side_effect=keyring_get_password) + @mock.patch("keyring.get_password", side_effect=keyring_get_password) def test_config_file_set_property(self, get_pass_func, mock_is_secure, mock_find_profile, mock_get_profile_name): """ Test that set_property calls the __is_secure, find_profile and get_profile_name_from_path methods. @@ -845,10 +844,10 @@ def test_config_file_set_property(self, get_pass_func, mock_is_secure, mock_find mock_get_profile_name.assert_called_with("profiles.zosmf.properties.user") self.assertEqual(config_file.profiles, { "zosmf": { - "properties": {"port": 1443}, + "properties": {"port": 1443, "user": "admin"}, "secure": ["user"] } - }) + }) def test_get_profile_name_from_path(self): """ @@ -856,8 +855,8 @@ def test_get_profile_name_from_path(self): """ config_file = ConfigFile(name="zowe_abcd", type="User Config") profile_name = config_file.get_profile_name_from_path("profiles.lpar1.profiles.zosmf.properties.user") - self.assertEqual(profile_name, "lpar1.zosmf") - + self.assertEqual(profile_name, "lpar1.zosmf") + def test_get_profile_path_from_name(self): """ Test that get_profile_path_from_name returns the profile path from the name. @@ -866,8 +865,8 @@ def test_get_profile_path_from_name(self): profile_path_1 = config_file.get_profile_path_from_name("lpar1.zosmf") self.assertEqual(profile_path_1, "profiles.lpar1.profiles.zosmf") - @patch("keyring.get_password", side_effect=keyring_get_password) - def test_config_file_set_profile(self,get_pass_func): + @mock.patch("keyring.get_password", side_effect=keyring_get_password) + def test_config_file_set_profile_and_save(self, get_pass_func): """ Test the set_profile method. """ @@ -880,12 +879,12 @@ def test_config_file_set_profile(self,get_pass_func): "profiles.zosmf.properties.password": "def" }) initial_profiles = { - "zosmf": { - "properties": { - "port": 1443 - }, - "secure": [] - } + "zosmf": { + "properties": { + "port": 1443 + }, + "secure": [] + } } config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path , profiles=initial_profiles) profile_data = { @@ -898,10 +897,21 @@ def test_config_file_set_profile(self,get_pass_func): "secure": ["user", "password"] } - with patch("zowe.core_for_zowe_sdk.ConfigFile.get_profile_name_from_path", return_value="zosmf"): - with patch("zowe.core_for_zowe_sdk.ConfigFile.find_profile", return_value=initial_profiles["zosmf"]): + with mock.patch("zowe.core_for_zowe_sdk.ConfigFile.get_profile_name_from_path", return_value="zosmf"): + with mock.patch("zowe.core_for_zowe_sdk.ConfigFile.find_profile", return_value=initial_profiles["zosmf"]): config_file.set_profile("profiles.zosmf", profile_data) + expected_profiles = { + "zosmf": profile_data + } + self.assertEqual(config_file.profiles, expected_profiles) + + config_file.jsonc = { + "profiles": expected_profiles + } + with mock.patch("builtins.open", mock.mock_open()): + config_file.save(False) + expected_secure_props = { cwd_up_file_path: { "profiles.zosmf.properties.user": "abc", @@ -909,18 +919,18 @@ def test_config_file_set_profile(self,get_pass_func): } } expected_profiles = { - "zosmf": { - 'type': 'zosmf', - "properties": { - "port": 443, - }, - "secure": ["user", "password"] - } + "zosmf": { + "type": "zosmf", + "properties": { + "port": 443, + }, + "secure": ["user", "password"] + } } self.assertEqual(CredentialManager.secure_props, expected_secure_props) - self.assertEqual(config_file.profiles, expected_profiles) - - @patch("zowe.core_for_zowe_sdk.CredentialManager.save_secure_props") + self.assertEqual(config_file.jsonc["profiles"], expected_profiles) + + @mock.patch("zowe.core_for_zowe_sdk.CredentialManager.save_secure_props") def test_config_file_save(self, mock_save_secure_props): """ Test saving a config file with secure properties. @@ -939,8 +949,8 @@ def test_config_file_save(self, mock_save_secure_props): "secure": ["user"] } } - } - with patch("builtins.open", mock.mock_open()) as mock_file: + } + with mock.patch("builtins.open", mock.mock_open()) as mock_file: config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path , profiles=config_data.copy()) config_file.jsonc = config_data config_file.save() @@ -948,7 +958,7 @@ def test_config_file_save(self, mock_save_secure_props): mock_save_secure_props.assert_called_once() mock_file.assert_called_once_with(cwd_up_file_path, 'w') mock_file.return_value.__enter__.return_value.write.asser_called() - + class TestValidateConfigJsonClass(unittest.TestCase): """Testing the validate_config_json function""" @@ -981,5 +991,4 @@ def test_validate_config_json_invalid(self): validate_config_json(path_to_invalid_config, path_to_invalid_schema, cwd = FIXTURES_PATH) self.assertEqual(str(actual_info.exception), str(expected_info.exception)) - - \ No newline at end of file + From 248f6d2e7144dd152bc9f6105bbf0152a54b6edf Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Fri, 29 Sep 2023 15:45:14 -0400 Subject: [PATCH 8/8] Test saving profile with nested secure property Signed-off-by: Timothy Johnson --- tests/unit/test_zowe_core.py | 38 +++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/tests/unit/test_zowe_core.py b/tests/unit/test_zowe_core.py index 5ef2adc1..54a91739 100644 --- a/tests/unit/test_zowe_core.py +++ b/tests/unit/test_zowe_core.py @@ -939,25 +939,35 @@ def test_config_file_save(self, mock_save_secure_props): cwd_up_file_path = os.path.join(cwd_up_dir_path, "zowe.config.json") os.chdir(CWD) shutil.copy(self.original_file_path, cwd_up_file_path) - config_data = { - "profiles": { - "zosmf": { - "properties": { - "user": "admin", - "port": 1443 - }, - "secure": ["user"] - } + profile_data = { + "lpar1": { + "profiles": { + "zosmf": { + "properties": { + "port": 1443, + "password": "secret" + }, + "secure": ["password"] + } + }, + "properties": { + "host": "example.com", + "user": "admin" + }, + "secure": ["user"] } } with mock.patch("builtins.open", mock.mock_open()) as mock_file: - config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path , profiles=config_data.copy()) - config_file.jsonc = config_data + config_file = ConfigFile("User Config", "zowe.config.json", cwd_up_dir_path, profiles=profile_data) + config_file.jsonc = {"profiles": profile_data} config_file.save() - mock_save_secure_props.assert_called_once() - mock_file.assert_called_once_with(cwd_up_file_path, 'w') - mock_file.return_value.__enter__.return_value.write.asser_called() + mock_save_secure_props.assert_called_once() + mock_file.assert_called_once_with(cwd_up_file_path, 'w') + mock_file.return_value.write.assert_called() + self.assertIn("user", profile_data["lpar1"]["properties"]) + self.assertNotIn("user", config_file.jsonc["profiles"]["lpar1"]["properties"]) + self.assertEqual(["port"], list(config_file.jsonc["profiles"]["lpar1"]["profiles"]["zosmf"]["properties"].keys())) class TestValidateConfigJsonClass(unittest.TestCase):