From c7cf4cc138081cb44f9bc0508b5082b8fe4f2867 Mon Sep 17 00:00:00 2001 From: angrybayblade Date: Wed, 25 Oct 2023 18:19:04 +0530 Subject: [PATCH 1/8] feat: use docker-compose' environement variable parser for private key password and env var for kubernetes --- autonomy/deploy/base.py | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/autonomy/deploy/base.py b/autonomy/deploy/base.py index dcf74da2ea..1a5b8ba804 100644 --- a/autonomy/deploy/base.py +++ b/autonomy/deploy/base.py @@ -110,7 +110,6 @@ def __init__( # pylint: disable=too-many-arguments self, service: Service, keys: Optional[List[Dict[str, str]]] = None, - private_keys_password: Optional[str] = None, agent_instances: Optional[List[str]] = None, apply_environment_variables: bool = False, ) -> None: @@ -128,7 +127,6 @@ def __init__( # pylint: disable=too-many-arguments self._service_name_clean = self.service.name.replace("_", "") self._keys = keys or [] self._agent_instances = agent_instances - self._private_keys_password = private_keys_password self._all_participants = self.try_get_all_participants() def get_abci_container_name(self, index: int) -> str: @@ -169,18 +167,6 @@ def try_get_all_participants(self) -> Optional[List[str]]: return None - @property - def private_keys_password( - self, - ) -> Optional[str]: - """Service password for agent keys.""" - - password = self._private_keys_password - if password is None: - password = os.environ.get("AUTONOLAS_SERVICE_PASSWORD") - - return password - @property def agent_instances( self, @@ -214,7 +200,6 @@ def from_dir( # pylint: disable=too-many-arguments path: Path, keys_file: Optional[Path] = None, number_of_agents: Optional[int] = None, - private_keys_password: Optional[str] = None, agent_instances: Optional[List[str]] = None, apply_environment_variables: bool = False, ) -> "ServiceBuilder": @@ -228,7 +213,6 @@ def from_dir( # pylint: disable=too-many-arguments service_builder = cls( service=service, apply_environment_variables=apply_environment_variables, - private_keys_password=private_keys_password, ) if keys_file is not None: @@ -626,10 +610,12 @@ def generate_common_vars(self, agent_n: int) -> Dict: ENV_VAR_AEA_AGENT: self.service.agent, ENV_VAR_LOG_LEVEL: self.log_level, } - - if self.private_keys_password is not None: - agent_vars[ENV_VAR_AEA_PASSWORD] = self.private_keys_password - + if self.deplopyment_type == DOCKER_COMPOSE_DEPLOYMENT: + agent_vars[ENV_VAR_AEA_PASSWORD] = "$OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD" + else: + agent_vars[ENV_VAR_AEA_PASSWORD] = os.environ.get( + "OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD", "" + ) return agent_vars def generate_agent( From 48fda084fe2b6fab92939b1ba34ff36a2edcdc8d Mon Sep 17 00:00:00 2001 From: angrybayblade Date: Wed, 25 Oct 2023 18:27:19 +0530 Subject: [PATCH 2/8] feat: add deprecation warning for `--password` flag --- autonomy/cli/deploy.py | 13 ++++++++++--- autonomy/cli/helpers/deployment.py | 4 ---- autonomy/deploy/build.py | 2 -- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/autonomy/cli/deploy.py b/autonomy/cli/deploy.py index 0dfb0bd3fb..e6e4cfc1bb 100644 --- a/autonomy/cli/deploy.py +++ b/autonomy/cli/deploy.py @@ -187,6 +187,11 @@ def build_deployment_command( # pylint: disable=too-many-arguments, too-many-lo image_author: Optional[str] = None, ) -> None: """Build deployment setup for n agents.""" + if password is not None: + click.echo( + "WARNING: `--password` flag has been deprecated, " + "use `OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD` to export the password value" + ) keys_file = Path(keys_file or DEFAULT_KEYS_FILE).absolute() if not keys_file.exists(): @@ -222,7 +227,6 @@ def build_deployment_command( # pylint: disable=too-many-arguments, too-many-lo deployment_type=deployment_type, dev_mode=dev_mode, number_of_agents=number_of_agents, - password=password, packages_dir=packages_dir, open_aea_dir=open_aea_dir, open_autonomy_dir=open_autonomy_dir, @@ -343,11 +347,15 @@ def run_deployment_from_token( # pylint: disable=too-many-arguments, too-many-l password: Optional[str] = None, ) -> None: """Run service deployment.""" + if password is not None: + click.echo( + "WARNING: `--password` flag has been deprecated, " + "use `OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD` to export the password value" + ) ctx = cast(Context, click_context.obj) ctx.registry_type = REGISTRY_REMOTE keys_file = Path(keys_file or DEFAULT_KEYS_FILE).absolute() - with reraise_as_click_exception( NotValidKeysFile, FileNotFoundError, FileExistsError ): @@ -359,7 +367,6 @@ def run_deployment_from_token( # pylint: disable=too-many-arguments, too-many-l n=n, deployment_type=deployment_type, aev=aev, - password=password, no_deploy=no_deploy, detach=detach, ) diff --git a/autonomy/cli/helpers/deployment.py b/autonomy/cli/helpers/deployment.py index 71a7767c39..3566ce3a2c 100644 --- a/autonomy/cli/helpers/deployment.py +++ b/autonomy/cli/helpers/deployment.py @@ -180,7 +180,6 @@ def build_deployment( # pylint: disable=too-many-arguments, too-many-locals deployment_type: str, dev_mode: bool, number_of_agents: Optional[int] = None, - password: Optional[str] = None, packages_dir: Optional[Path] = None, open_aea_dir: Optional[Path] = None, open_autonomy_dir: Optional[Path] = None, @@ -213,7 +212,6 @@ def build_deployment( # pylint: disable=too-many-arguments, too-many-locals service_path=Path.cwd(), type_of_deployment=deployment_type, keys_file=keys_file, - private_keys_password=password, number_of_agents=number_of_agents, build_dir=build_dir, dev_mode=dev_mode, @@ -280,7 +278,6 @@ def build_and_deploy_from_token( # pylint: disable=too-many-arguments, too-many n: Optional[int], deployment_type: str, aev: bool = False, - password: Optional[str] = None, no_deploy: bool = False, detach: bool = False, ) -> None: @@ -314,7 +311,6 @@ def build_and_deploy_from_token( # pylint: disable=too-many-arguments, too-many multisig_address=multisig_address, consensus_threshold=consensus_threshold, apply_environment_variables=aev, - password=password, ) if not skip_image: click.echo("Building required images.") diff --git a/autonomy/deploy/build.py b/autonomy/deploy/build.py index 0f27ca4ac9..d4d3b9d8a2 100644 --- a/autonomy/deploy/build.py +++ b/autonomy/deploy/build.py @@ -38,7 +38,6 @@ def generate_deployment( # pylint: disable=too-many-arguments, too-many-locals service_path: Path, build_dir: Path, number_of_agents: Optional[int] = None, - private_keys_password: Optional[str] = None, dev_mode: bool = False, packages_dir: Optional[Path] = None, open_aea_dir: Optional[Path] = None, @@ -60,7 +59,6 @@ def generate_deployment( # pylint: disable=too-many-arguments, too-many-locals path=service_path, keys_file=keys_file, number_of_agents=number_of_agents, - private_keys_password=private_keys_password, agent_instances=agent_instances, apply_environment_variables=apply_environment_variables, ) From 7fc391aa186d0002d270e5b95185e77c08e8a14c Mon Sep 17 00:00:00 2001 From: angrybayblade Date: Wed, 25 Oct 2023 18:27:48 +0530 Subject: [PATCH 3/8] test: update service builder tests --- .../test_deploy/test_service_specification.py | 11 +++-------- tests/test_deployments/test_deployments.py | 1 - 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/test_autonomy/test_deploy/test_service_specification.py b/tests/test_autonomy/test_deploy/test_service_specification.py index e691835072..fe2ec72964 100644 --- a/tests/test_autonomy/test_deploy/test_service_specification.py +++ b/tests/test_autonomy/test_deploy/test_service_specification.py @@ -125,7 +125,6 @@ def test_initialize( self.keys_path, ) - assert spec.private_keys_password is None assert spec.agent_instances is None assert len(spec.keys) == 1 @@ -144,11 +143,11 @@ def test_generate_agents( assert len(agents) == 1, agents agent = spec.generate_agent(0) - assert len(agent.keys()) == 10, agent + assert len(agent.keys()) == 11, agent spec.service.overrides = [] agent = spec.generate_agent(0) - assert len(agent.keys()) == 3, agent + assert len(agent.keys()) == 4, agent def test_generate_common_vars( self, @@ -165,11 +164,7 @@ def test_generate_common_vars( assert all(var in common_vars_without_password for var in COMMON_VARS[:-1]) assert common_vars_without_password[ENV_VAR_AEA_AGENT] == spec.service.agent - spec = ServiceBuilder.from_dir( # nosec - self.service_path, - self.keys_path, - private_keys_password="some_password", - ) + spec = ServiceBuilder.from_dir(self.service_path, self.keys_path) # nosec common_vars_without_password = spec.generate_common_vars(agent_n=0) assert all(var in common_vars_without_password for var in COMMON_VARS) diff --git a/tests/test_deployments/test_deployments.py b/tests/test_deployments/test_deployments.py index 6ae4b35de7..bab7eed0cc 100644 --- a/tests/test_deployments/test_deployments.py +++ b/tests/test_deployments/test_deployments.py @@ -281,7 +281,6 @@ def test_update_agent_number_based_on_keys_file(self) -> None: builder = ServiceBuilder( service=service, keys=None, - private_keys_password=None, agent_instances=list("abcdefg"), ) assert builder.service.number_of_agents == 1_000_000 From 65d7139b54eb2d1b56d47927f3e9a0d7c23324d1 Mon Sep 17 00:00:00 2001 From: angrybayblade Date: Wed, 25 Oct 2023 18:30:01 +0530 Subject: [PATCH 4/8] docs: update API docs --- docs/api/cli/helpers/deployment.md | 2 -- docs/api/deploy/base.md | 13 ------------- docs/api/deploy/build.md | 1 - 3 files changed, 16 deletions(-) diff --git a/docs/api/cli/helpers/deployment.md b/docs/api/cli/helpers/deployment.md index c1cef0d308..17383da163 100644 --- a/docs/api/cli/helpers/deployment.md +++ b/docs/api/cli/helpers/deployment.md @@ -37,7 +37,6 @@ def build_deployment(keys_file: Path, deployment_type: str, dev_mode: bool, number_of_agents: Optional[int] = None, - password: Optional[str] = None, packages_dir: Optional[Path] = None, open_aea_dir: Optional[Path] = None, open_autonomy_dir: Optional[Path] = None, @@ -67,7 +66,6 @@ def build_and_deploy_from_token(token_id: int, n: Optional[int], deployment_type: str, aev: bool = False, - password: Optional[str] = None, no_deploy: bool = False, detach: bool = False) -> None ``` diff --git a/docs/api/deploy/base.md b/docs/api/deploy/base.md index 8fca6a7ad8..62492969a2 100644 --- a/docs/api/deploy/base.md +++ b/docs/api/deploy/base.md @@ -43,7 +43,6 @@ Class to assist with generating deployments. ```python def __init__(service: Service, keys: Optional[List[Dict[str, str]]] = None, - private_keys_password: Optional[str] = None, agent_instances: Optional[List[str]] = None, apply_environment_variables: bool = False) -> None ``` @@ -80,17 +79,6 @@ def try_get_all_participants() -> Optional[List[str]] Try get all participants from the ABCI overrides - - -#### private`_`keys`_`password - -```python -@property -def private_keys_password() -> Optional[str] -``` - -Service password for agent keys. - #### agent`_`instances @@ -134,7 +122,6 @@ def from_dir(cls, path: Path, keys_file: Optional[Path] = None, number_of_agents: Optional[int] = None, - private_keys_password: Optional[str] = None, agent_instances: Optional[List[str]] = None, apply_environment_variables: bool = False) -> "ServiceBuilder" ``` diff --git a/docs/api/deploy/build.md b/docs/api/deploy/build.md index 050d6cfc9b..711873ecad 100644 --- a/docs/api/deploy/build.md +++ b/docs/api/deploy/build.md @@ -14,7 +14,6 @@ def generate_deployment(type_of_deployment: str, service_path: Path, build_dir: Path, number_of_agents: Optional[int] = None, - private_keys_password: Optional[str] = None, dev_mode: bool = False, packages_dir: Optional[Path] = None, open_aea_dir: Optional[Path] = None, From 6bbc0baf2aa07f364cdeb45aec6e348dce1ee708 Mon Sep 17 00:00:00 2001 From: angrybayblade Date: Wed, 25 Oct 2023 18:52:58 +0530 Subject: [PATCH 5/8] docs: update deploy command docs --- docs/advanced_reference/commands/autonomy_deploy.md | 9 ++++++--- .../developer_tooling/debugging_in_the_cluster.md | 3 ++- docs/application_deployment.md | 1 - 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/advanced_reference/commands/autonomy_deploy.md b/docs/advanced_reference/commands/autonomy_deploy.md index 05c29f16ab..c9cb745c90 100644 --- a/docs/advanced_reference/commands/autonomy_deploy.md +++ b/docs/advanced_reference/commands/autonomy_deploy.md @@ -102,9 +102,6 @@ autonomy deploy build [OPTIONS] [KEYS_FILE] `-p` : Ask for password interactively. -`--password PASSWORD` -: Set password for key encryption/decryption. - `--help` : Show the help message and exit. @@ -148,12 +145,18 @@ autonomy deploy run [OPTIONS] : Show the help message and exit. ### Examples + ```bash autonomy deploy run --build-dir ./abci_build ``` Runs the service deployment stored locally in the directory `./abci_build`. +To provide password for the private keys + +```bash +OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD=PASSWORD autonomy deploy run --build-dir ./abci_build +``` ## `autonomy deploy from-token` diff --git a/docs/advanced_reference/developer_tooling/debugging_in_the_cluster.md b/docs/advanced_reference/developer_tooling/debugging_in_the_cluster.md index 032c037525..bf0c5c5cde 100644 --- a/docs/advanced_reference/developer_tooling/debugging_in_the_cluster.md +++ b/docs/advanced_reference/developer_tooling/debugging_in_the_cluster.md @@ -22,7 +22,8 @@ docker image push Finally, build the deployment and run it: ```bash -autonomy deploy build ../generated_keys.json --password ${PASSWORD} --kubernetes --dev +export OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD=${PASSWORD} +autonomy deploy build ../generated_keys.json --kubernetes --dev kubectl apply -f abci_build/ kubectl apply -f abci_build/agent_keys ``` diff --git a/docs/application_deployment.md b/docs/application_deployment.md index 67c7f1dd6e..87c11e9099 100644 --- a/docs/application_deployment.md +++ b/docs/application_deployment.md @@ -38,7 +38,6 @@ Options: --remote To use a remote registry. --local To use a local registry. -p Ask for password interactively - --password PASSWORD Set password for key encryption/decryption --help Show this message and exit. ``` From df97616654ad5d31574c7a182f1be9395745236b Mon Sep 17 00:00:00 2001 From: angrybayblade Date: Thu, 26 Oct 2023 09:35:17 +0530 Subject: [PATCH 6/8] fix: build tests --- .../test_deploy/test_build/test_deployment.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/test_autonomy/test_cli/test_deploy/test_build/test_deployment.py b/tests/test_autonomy/test_cli/test_deploy/test_build/test_deployment.py index 666882466d..589b2fc845 100644 --- a/tests/test_autonomy/test_cli/test_deploy/test_build/test_deployment.py +++ b/tests/test_autonomy/test_cli/test_deploy/test_build/test_deployment.py @@ -308,12 +308,14 @@ def test_docker_compose_password( ) build_dir = self.t / DEFAULT_BUILD_FOLDER - - assert result.exit_code == 0, result.output + assert result.exit_code == 0, result.stderr + assert ( + "WARNING: `--password` flag has been deprecated, use `OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD` to export the password value" + in result.stdout + ) assert build_dir.exists() build_dir = self.t / DEFAULT_BUILD_FOLDER - assert result.exit_code == 0, result.output assert build_dir.exists() @@ -321,8 +323,6 @@ def test_docker_compose_password( with open(docker_compose_file, "r", encoding="utf-8") as fp: docker_compose = yaml.safe_load(fp) - agents = int(len(docker_compose["services"]) / 2) - def _file_check(n: int) -> bool: return ( build_dir @@ -330,6 +330,7 @@ def _file_check(n: int) -> bool: / DEPLOYMENT_AGENT_KEY_DIRECTORY_SCHEMA.format(agent_n=n) ).exists() + agents = int(len(docker_compose["services"]) / 2) assert all(_file_check(i) for i in range(agents)) for x in range(agents): env = dict( @@ -341,7 +342,7 @@ def _file_check(n: int) -> bool: ] ) assert "AEA_PASSWORD" in env.keys() - assert env["AEA_PASSWORD"] == ETHEREUM_ENCRYPTION_PASSWORD + assert env["AEA_PASSWORD"] == "$OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD" def test_include_acn_and_hardhat_nodes( self, @@ -601,6 +602,10 @@ def test_kubernetes_build_password( ) assert result.exit_code == 0, result.output + assert ( + "WARNING: `--password` flag has been deprecated, use `OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD` to export the password value" + in result.stdout + ) assert build_dir.exists() kubernetes_config = self.load_kubernetes_config( @@ -613,7 +618,7 @@ def test_kubernetes_build_password( except (KeyError, IndexError): continue - assert agent_vars["AEA_PASSWORD"] == ETHEREUM_ENCRYPTION_PASSWORD + assert agent_vars["AEA_PASSWORD"] == "" assert all( ( From 997ed8bb6443561a85ba2d7bba3757cc5b06aafc Mon Sep 17 00:00:00 2001 From: angrybayblade Date: Thu, 26 Oct 2023 12:22:12 +0530 Subject: [PATCH 7/8] fix: test coverage --- autonomy/cli/deploy.py | 4 ++-- .../test_deploy/test_service_specification.py | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/autonomy/cli/deploy.py b/autonomy/cli/deploy.py index e6e4cfc1bb..89bb957bb1 100644 --- a/autonomy/cli/deploy.py +++ b/autonomy/cli/deploy.py @@ -187,7 +187,7 @@ def build_deployment_command( # pylint: disable=too-many-arguments, too-many-lo image_author: Optional[str] = None, ) -> None: """Build deployment setup for n agents.""" - if password is not None: + if password is not None: # pragma: nocover click.echo( "WARNING: `--password` flag has been deprecated, " "use `OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD` to export the password value" @@ -347,7 +347,7 @@ def run_deployment_from_token( # pylint: disable=too-many-arguments, too-many-l password: Optional[str] = None, ) -> None: """Run service deployment.""" - if password is not None: + if password is not None: # pragma: nocover click.echo( "WARNING: `--password` flag has been deprecated, " "use `OPEN_AUTONOMY_PRIVATE_KEY_PASSWORD` to export the password value" diff --git a/tests/test_autonomy/test_deploy/test_service_specification.py b/tests/test_autonomy/test_deploy/test_service_specification.py index fe2ec72964..73de2e55d8 100644 --- a/tests/test_autonomy/test_deploy/test_service_specification.py +++ b/tests/test_autonomy/test_deploy/test_service_specification.py @@ -149,6 +149,24 @@ def test_generate_agents( agent = spec.generate_agent(0) assert len(agent.keys()) == 4, agent + def test_get_maximum_participants( + self, + ) -> None: + """Test get_maximum_participants.""" + self._write_service(get_dummy_service_config(file_number=0)) + spec = ServiceBuilder.from_dir( + self.service_path, + self.keys_path, + ) + + spec._all_participants = list(map(str, range(2))) + assert spec.get_maximum_participants() == 2 + + with mock.patch.object(spec, attribute="verify_agent_instances"): + spec._all_participants = [] + spec.agent_instances = list(map(str, range(2))) + assert spec.get_maximum_participants() == 3 + def test_generate_common_vars( self, ) -> None: From d0daab834c00d9f2fef19069d28585d7a4f7ee98 Mon Sep 17 00:00:00 2001 From: angrybayblade Date: Thu, 26 Oct 2023 13:21:59 +0530 Subject: [PATCH 8/8] fix: service spec test --- tests/test_autonomy/test_deploy/test_service_specification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_autonomy/test_deploy/test_service_specification.py b/tests/test_autonomy/test_deploy/test_service_specification.py index 73de2e55d8..352f206171 100644 --- a/tests/test_autonomy/test_deploy/test_service_specification.py +++ b/tests/test_autonomy/test_deploy/test_service_specification.py @@ -164,7 +164,7 @@ def test_get_maximum_participants( with mock.patch.object(spec, attribute="verify_agent_instances"): spec._all_participants = [] - spec.agent_instances = list(map(str, range(2))) + spec.agent_instances = list(map(str, range(3))) assert spec.get_maximum_participants() == 3 def test_generate_common_vars(