From f5d10d5d424504da231f0463a63c18f6fdd9cac9 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 5 Dec 2022 17:09:29 +0100 Subject: [PATCH 01/13] wip --- .../ci_credentials/ci_credentials/__init__.py | 10 +- tools/ci_credentials/ci_credentials/main.py | 27 ++-- tools/ci_credentials/ci_credentials/models.py | 56 +++++++ .../{secrets_loader.py => secrets_manager.py} | 146 +++++++++--------- tools/ci_credentials/setup.py | 4 +- 5 files changed, 151 insertions(+), 92 deletions(-) create mode 100644 tools/ci_credentials/ci_credentials/models.py rename tools/ci_credentials/ci_credentials/{secrets_loader.py => secrets_manager.py} (61%) diff --git a/tools/ci_credentials/ci_credentials/__init__.py b/tools/ci_credentials/ci_credentials/__init__.py index 4f55c3781545..38a7423694b8 100644 --- a/tools/ci_credentials/ci_credentials/__init__.py +++ b/tools/ci_credentials/ci_credentials/__init__.py @@ -1,6 +1,6 @@ -# from .main import main -from .secrets_loader import SecretsLoader +# +# Copyright (c) 2022 Airbyte, Inc., all rights reserved. +# +from .secrets_manager import SecretsManager -__all__ = ( - "SecretsLoader", -) +__all__ = ("SecretsManager",) diff --git a/tools/ci_credentials/ci_credentials/main.py b/tools/ci_credentials/ci_credentials/main.py index 3147509e3711..fbaedbae848e 100644 --- a/tools/ci_credentials/ci_credentials/main.py +++ b/tools/ci_credentials/ci_credentials/main.py @@ -1,10 +1,15 @@ +# +# Copyright (c) 2022 Airbyte, Inc., all rights reserved. +# import json import os import sys from json.decoder import JSONDecodeError +import click from ci_common_utils import Logger -from . import SecretsLoader + +from . import SecretsManager logger = Logger() @@ -12,13 +17,13 @@ # credentials of GSM and GitHub secrets should be shared via shell environment - -def main() -> int: - if len(sys.argv) != 2: - return logger.error("uses one script argument only: ") +@click.command() +@click.argument("mode", type=click.Choice(("read", "write"))) +@click.argument("connector_name") +def main(mode, connector_name) -> int: # parse unique connector name, because it can have the common prefix "connectors/" - connector_name = sys.argv[1].split("/")[-1] + connector_name = connector_name.split("/")[-1] if connector_name == "all": # if needed to load all secrets connector_name = None @@ -32,12 +37,16 @@ def main() -> int: if not gsm_credentials: return logger.error("GCP_GSM_CREDENTIALS shouldn't be empty!") - loader = SecretsLoader( + secret_manager = SecretsManager( connector_name=connector_name, gsm_credentials=gsm_credentials, ) - return loader.write_to_storage(loader.read_from_gsm()) + connector_secrets = secret_manager.read_from_gsm() + if mode == "read": + return secret_manager.write_to_storage(connector_secrets) + if mode == "write": + return secret_manager.update_secrets(connector_secrets) -if __name__ == '__main__': +if __name__ == "__main__": sys.exit(main()) diff --git a/tools/ci_credentials/ci_credentials/models.py b/tools/ci_credentials/ci_credentials/models.py new file mode 100644 index 000000000000..20b04808847d --- /dev/null +++ b/tools/ci_credentials/ci_credentials/models.py @@ -0,0 +1,56 @@ +# +# Copyright (c) 2022 Airbyte, Inc., all rights reserved. +# + +from __future__ import ( # Used to evaluate type hints at runtime, a NameError: name 'ConfigObserver' is not defined is thrown otherwise + annotations, +) + +from dataclasses import dataclass + +DEFAULT_SECRET_FILE = "config" + + +@dataclass +class Secret: + connector_name: str + configuration_file_name: str + value: str + + @property + def name(self) -> str: + return self.generate_secret_name(self.connector_name, self.configuration_file_name) + + @staticmethod + def generate_secret_name(connector_name: str, configuration_file_name: str) -> str: + """ + Generates an unique GSM secret name. + Format of secret name: SECRET____CREDS + Examples: + 1. connector_name: source-linnworks, filename: dsdssds_a-b---_---_config.json + => SECRET_SOURCE-LINNWORKS_DSDSSDS_A-B__CREDS + 2. connector_name: source-s3, filename: config.json + => SECRET_SOURCE-LINNWORKS__CREDS + """ + name_parts = ["secret", connector_name] + filename_wo_ext = configuration_file_name.replace(".json", "") + if filename_wo_ext != DEFAULT_SECRET_FILE: + name_parts.append(filename_wo_ext.replace(DEFAULT_SECRET_FILE, "").strip("_-")) + name_parts.append("_creds") + return "_".join(name_parts).upper() + + @property + def directory(self) -> str: + if self.connector_name == "base-normalization": + return f"airbyte-integrations/bases/{self.connector_name}/secrets" + else: + return f"airbyte-integrations/connectors/{self.connector_name}/secrets" + + +@dataclass +class RemoteSecret(Secret): + enabled_version: str + + @classmethod + def from_secret(cls, secret: Secret, enabled_version: str) -> RemoteSecret: + return RemoteSecret(secret.connector_name, secret.configuration_file_name, secret.value, enabled_version) diff --git a/tools/ci_credentials/ci_credentials/secrets_loader.py b/tools/ci_credentials/ci_credentials/secrets_manager.py similarity index 61% rename from tools/ci_credentials/ci_credentials/secrets_loader.py rename to tools/ci_credentials/ci_credentials/secrets_manager.py index 60979c96b1be..0ee8255970b6 100644 --- a/tools/ci_credentials/ci_credentials/secrets_loader.py +++ b/tools/ci_credentials/ci_credentials/secrets_manager.py @@ -5,13 +5,15 @@ import json import os import re +from glob import glob from json.decoder import JSONDecodeError from pathlib import Path -from typing import Any, ClassVar, Mapping, Tuple +from typing import Any, ClassVar, List, Mapping from ci_common_utils import GoogleApi, Logger -DEFAULT_SECRET_FILE = "config" +from .models import DEFAULT_SECRET_FILE, RemoteSecret, Secret + DEFAULT_SECRET_FILE_WITH_EXT = DEFAULT_SECRET_FILE + ".json" GSM_SCOPES = ("https://www.googleapis.com/auth/cloud-platform",) @@ -43,8 +45,8 @@ ] -class SecretsLoader: - """Loading and saving all requested secrets into connector folders""" +class SecretsManager: + """Loading, saving and updating all requested secrets into connector folders""" logger: ClassVar[Logger] = Logger() if os.getenv("VERSION") == "dev": @@ -63,9 +65,9 @@ def api(self) -> GoogleApi: self._api = GoogleApi(self.gsm_credentials, GSM_SCOPES) return self._api - def __load_gsm_secrets(self) -> Mapping[Tuple[str, str], str]: + def __load_gsm_secrets(self) -> List[RemoteSecret]: """Loads needed GSM secrets""" - secrets = {} + secrets = [] # docs: https://cloud.google.com/secret-manager/docs/filtering#api filter = "name:SECRET_" if self.connector_name: @@ -105,8 +107,8 @@ def __load_gsm_secrets(self) -> Mapping[Tuple[str, str], str]: enabled_versions = [version["name"] for version in data["versions"] if version["state"] == "ENABLED"] if len(enabled_versions) > 1: self.logger.critical(f"{log_name} should have one enabled version at the same time!!!") - - secret_url = f"https://secretmanager.googleapis.com/v1/{enabled_versions[0]}:access" + enabled_version = enabled_versions[0] + secret_url = f"https://secretmanager.googleapis.com/v1/{enabled_version}:access" data = self.api.get(secret_url) secret_value = data.get("payload", {}).get("data") if not secret_value: @@ -121,7 +123,8 @@ def __load_gsm_secrets(self) -> Mapping[Tuple[str, str], str]: except JSONDecodeError as err: self.logger.error(f"{log_name} has non-JSON value!!! Error: {err}") continue - secrets[(connector_name, filename)] = secret_value + remote_secret = RemoteSecret(connector_name, filename, secret_value, enabled_version) + secrets.append(remote_secret) next_token = data.get("nextPageToken") if not next_token: @@ -160,82 +163,73 @@ def mask_secrets_from_action_log(self, key, value): # carry on pass - @staticmethod - def generate_secret_name(connector_name: str, filename: str) -> str: - """ - Generates an unique GSM secret name. - Format of secret name: SECRET____CREDS - Examples: - 1. connector_name: source-linnworks, filename: dsdssds_a-b---_---_config.json - => SECRET_SOURCE-LINNWORKS_DSDSSDS_A-B__CREDS - 2. connector_name: source-s3, filename: config.json - => SECRET_SOURCE-LINNWORKS__CREDS - """ - name_parts = ["secret", connector_name] - filename_wo_ext = filename.replace(".json", "") - if filename_wo_ext != DEFAULT_SECRET_FILE: - name_parts.append(filename_wo_ext.replace(DEFAULT_SECRET_FILE, "").strip("_-")) - name_parts.append("_creds") - return "_".join(name_parts).upper() - - def create_secret(self, connector_name: str, filename: str, secret_value: str) -> bool: - """ - Creates a new GSM secret with auto-generated name. - """ - secret_name = self.generate_secret_name(connector_name, filename) - self.logger.info(f"Generated the new secret name '{secret_name}' for {connector_name}({filename})") - params = { - "secretId": secret_name, - } - labels = { - "connector": connector_name, - } - if filename != DEFAULT_SECRET_FILE: - labels["filename"] = filename.replace(".json", "") - body = { - "labels": labels, - "replication": {"automatic": {}}, - } - url = f"https://secretmanager.googleapis.com/v1/projects/{self.api.project_id}/secrets" - data = self.api.post(url, json=body, params=params) - - # try to create a new version - secret_name = data["name"] - self.logger.info(f"the GSM secret {secret_name} was created") - secret_url = f"https://secretmanager.googleapis.com/v1/{secret_name}:addVersion" - body = {"payload": {"data": base64.b64encode(secret_value.encode()).decode("utf-8")}} - self.api.post(secret_url, json=body) - return True - - def read_from_gsm(self) -> int: + def read_from_gsm(self) -> List[RemoteSecret]: """Reads all necessary secrets from different sources""" secrets = self.__load_gsm_secrets() - - for k in secrets: - if not isinstance(secrets[k], tuple): - secrets[k] = ("GSM", secrets[k]) - source, _ = secrets[k] - self.logger.info(f"Register the file {k[1]}({k[0]}) from {source}") - if not len(secrets): self.logger.warning(f"not found any secrets of the connector '{self.connector_name}'") - return {} - return {k: v[1] for k, v in secrets.items()} + return [] + return secrets - def write_to_storage(self, secrets: Mapping[Tuple[str, str], str]) -> int: + def write_to_storage(self, secrets: List[RemoteSecret]) -> int: """Tries to save target secrets to the airbyte-integrations/connectors|bases/{connector_name}/secrets folder""" if not secrets: return 0 - for (connector_name, filename), secret_value in secrets.items(): - if connector_name == "base-normalization": - secrets_dir = f"airbyte-integrations/bases/{connector_name}/secrets" - else: - secrets_dir = f"airbyte-integrations/connectors/{connector_name}/secrets" - - secrets_dir = self.base_folder / secrets_dir + for secret in secrets: + secrets_dir = self.base_folder / secret.directory secrets_dir.mkdir(parents=True, exist_ok=True) - filepath = secrets_dir / filename + filepath = secrets_dir / secret.configuration_file_name with open(filepath, "w") as file: - file.write(secret_value) + file.write(secret.value) self.logger.info(f"The file {filepath} was saved") return 0 + + def _create_new_secret_version(self, new_secret: Secret, old_secret: RemoteSecret) -> RemoteSecret: + secret_url = f"https://secretmanager.googleapis.com/v1/projects/{self.api.project_id}/secrets/{new_secret.name}:addVersion" + body = {"payload": {"data": base64.b64encode(new_secret.value.encode()).decode("utf-8")}} + new_version_response = self.api.post(secret_url, json=body) + self._disable_version(old_secret.enabled_version) + return RemoteSecret.from_secret(new_secret, enabled_version=new_version_response["name"]) + + def _disable_version(self, version_name: str): + disable_version_url = f"https://secretmanager.googleapis.com/v1/{version_name}:disable" + return self.api.post(disable_version_url) + + def _get_updated_secrets(self) -> List[Secret]: + updated_configurations_glob = glob(f"airbyte-integrations/connectors/{self.connector_name}/secrets/updated_configurations/*.json") + updated_configuration_files_versions = {} + for updated_configuration_path in updated_configurations_glob: + updated_configuration_path = Path(updated_configuration_path) + with open(updated_configuration_path, "r") as updated_configuration: + updated_configuration_value = json.load(updated_configuration) + configuration_original_file_name = f"{updated_configuration_path.stem.split('|')[0]}{updated_configuration_path.suffix}" + updated_configuration_files_versions.setdefault(configuration_original_file_name, []) + updated_configuration_files_versions[configuration_original_file_name].append( + (updated_configuration_value, os.path.getctime(str(updated_configuration_path))) + ) + + for updated_configurations in updated_configuration_files_versions.values(): + updated_configurations.sort(key=lambda x: x[1]) + return [ + Secret( + connector_name=self.connector_name, + configuration_file_name=configuration_file_name, + value=json.dumps(versions_by_creation_time[-1][0]), + ) + for configuration_file_name, versions_by_creation_time in updated_configuration_files_versions.items() + ] + + def update_secrets(self, existing_secrets: List[RemoteSecret]) -> List[RemoteSecret]: + existing_secrets = {secret.name: secret for secret in existing_secrets} + updated_secrets = {secret.name: secret for secret in self._get_updated_secrets()} + new_remote_secrets = [] + for existing_secret_name in existing_secrets: + if existing_secret_name in updated_secrets and json.loads(updated_secrets[existing_secret_name].value) != json.loads( + existing_secrets[existing_secret_name].value + ): + new_secret = updated_secrets[existing_secret_name] + old_secret = existing_secrets[existing_secret_name] + new_remote_secret = self._create_new_secret_version(new_secret, old_secret) + new_remote_secrets.append(new_remote_secret) + self.logger.info(f"Updated {new_remote_secret.name} with new value") + return new_remote_secrets diff --git a/tools/ci_credentials/setup.py b/tools/ci_credentials/setup.py index 2c06e0a3cbbf..ff4eadc148f5 100644 --- a/tools/ci_credentials/setup.py +++ b/tools/ci_credentials/setup.py @@ -1,11 +1,11 @@ # -# Copyright (c) 2021 Airbyte, Inc., all rights reserved. +# Copyright (c) 2022 Airbyte, Inc., all rights reserved. # from setuptools import find_packages, setup -MAIN_REQUIREMENTS = ["requests", "ci_common_utils"] +MAIN_REQUIREMENTS = ["requests", "ci_common_utils", "click~=8.1.3"] TEST_REQUIREMENTS = ["requests-mock"] From 03bb578e7a8928c63db190f5b591102cdd7efb81 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 5 Dec 2022 17:26:55 +0100 Subject: [PATCH 02/13] cleaner cli --- tools/ci_credentials/ci_credentials/main.py | 34 +++++++++++++-------- tools/ci_credentials/setup.py | 2 +- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/tools/ci_credentials/ci_credentials/main.py b/tools/ci_credentials/ci_credentials/main.py index fbaedbae848e..3cf41e4a1f77 100644 --- a/tools/ci_credentials/ci_credentials/main.py +++ b/tools/ci_credentials/ci_credentials/main.py @@ -2,7 +2,6 @@ # Copyright (c) 2022 Airbyte, Inc., all rights reserved. # import json -import os import sys from json.decoder import JSONDecodeError @@ -17,11 +16,13 @@ # credentials of GSM and GitHub secrets should be shared via shell environment -@click.command() -@click.argument("mode", type=click.Choice(("read", "write"))) +@click.group() @click.argument("connector_name") -def main(mode, connector_name) -> int: - +@click.option("--gcp-gsm-credentials", envvar="GCP_GSM_CREDENTIALS") +@click.pass_context +def ci_credentials(ctx, connector_name: str, gcp_gsm_credentials): + ctx.ensure_object(dict) + ctx.obj["connector_name"] = connector_name # parse unique connector name, because it can have the common prefix "connectors/" connector_name = connector_name.split("/")[-1] if connector_name == "all": @@ -30,7 +31,7 @@ def main(mode, connector_name) -> int: # parse GCP_GSM_CREDENTIALS try: - gsm_credentials = json.loads(os.getenv(ENV_GCP_GSM_CREDENTIALS) or "{}") + gsm_credentials = json.loads(gcp_gsm_credentials) if gcp_gsm_credentials else {} except JSONDecodeError as e: return logger.error(f"incorrect GCP_GSM_CREDENTIALS value, error: {e}") @@ -41,12 +42,21 @@ def main(mode, connector_name) -> int: connector_name=connector_name, gsm_credentials=gsm_credentials, ) - connector_secrets = secret_manager.read_from_gsm() - if mode == "read": - return secret_manager.write_to_storage(connector_secrets) - if mode == "write": - return secret_manager.update_secrets(connector_secrets) + ctx.obj["secret_manager"] = secret_manager + ctx.obj["connector_secrets"] = secret_manager.read_from_gsm() + + +@ci_credentials.command(help="Download GSM secrets locally to the connector's secrets directory.") +@click.pass_context +def write_to_storage(ctx): + return ctx.obj["secret_manager"].write_to_storage(ctx.obj["connector_secrets"]) + + +@ci_credentials.command(help="Update GSM secrets according to the content of the secrets/updated_configurations directory.") +@click.pass_context +def update_secrets(ctx): + return ctx.obj["secret_manager"].update_secrets(ctx.obj["connector_secrets"]) if __name__ == "__main__": - sys.exit(main()) + sys.exit(ci_credentials(obj={})) diff --git a/tools/ci_credentials/setup.py b/tools/ci_credentials/setup.py index ff4eadc148f5..8427fcc2ea61 100644 --- a/tools/ci_credentials/setup.py +++ b/tools/ci_credentials/setup.py @@ -23,7 +23,7 @@ }, entry_points={ "console_scripts": [ - "ci_credentials = ci_credentials.main:main", + "ci_credentials = ci_credentials.main:ci_credentials", ], }, ) From 69f2d148632c5ee7335538dfbb6ccf80582d0a04 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 5 Dec 2022 17:33:32 +0100 Subject: [PATCH 03/13] add doc strings --- .../ci_credentials/secrets_manager.py | 36 +++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/tools/ci_credentials/ci_credentials/secrets_manager.py b/tools/ci_credentials/ci_credentials/secrets_manager.py index 0ee8255970b6..9e6617d0dbab 100644 --- a/tools/ci_credentials/ci_credentials/secrets_manager.py +++ b/tools/ci_credentials/ci_credentials/secrets_manager.py @@ -185,17 +185,39 @@ def write_to_storage(self, secrets: List[RemoteSecret]) -> int: return 0 def _create_new_secret_version(self, new_secret: Secret, old_secret: RemoteSecret) -> RemoteSecret: + """Create a new secret version from a new secret instance. Disable the previous secret version. + + Args: + new_secret (Secret): The new secret instance + old_secret (RemoteSecret): The old secret instance + + Returns: + RemoteSecret: The newly created remote secret instance + """ secret_url = f"https://secretmanager.googleapis.com/v1/projects/{self.api.project_id}/secrets/{new_secret.name}:addVersion" body = {"payload": {"data": base64.b64encode(new_secret.value.encode()).decode("utf-8")}} new_version_response = self.api.post(secret_url, json=body) self._disable_version(old_secret.enabled_version) return RemoteSecret.from_secret(new_secret, enabled_version=new_version_response["name"]) - def _disable_version(self, version_name: str): + def _disable_version(self, version_name: str) -> dict: + """Disable a GSM secret version + + Args: + version_name (str): Full name of the version (containing project id and secret name) + + Returns: + dict: API response + """ disable_version_url = f"https://secretmanager.googleapis.com/v1/{version_name}:disable" return self.api.post(disable_version_url) def _get_updated_secrets(self) -> List[Secret]: + """Find locally updated configurations files and return the most recent instance for each configuration file name. + + Returns: + List[Secret]: List of Secret instances parsed from local updated configuration files + """ updated_configurations_glob = glob(f"airbyte-integrations/connectors/{self.connector_name}/secrets/updated_configurations/*.json") updated_configuration_files_versions = {} for updated_configuration_path in updated_configurations_glob: @@ -219,7 +241,15 @@ def _get_updated_secrets(self) -> List[Secret]: for configuration_file_name, versions_by_creation_time in updated_configuration_files_versions.items() ] - def update_secrets(self, existing_secrets: List[RemoteSecret]) -> List[RemoteSecret]: + def update_secrets(self, existing_secrets: List[RemoteSecret]) -> int: + """Update existing secrets if an updated version was found locally. + + Args: + existing_secrets (List[RemoteSecret]): List of existing secrets for the current connector on GSM. + + Returns: + int: Status code + """ existing_secrets = {secret.name: secret for secret in existing_secrets} updated_secrets = {secret.name: secret for secret in self._get_updated_secrets()} new_remote_secrets = [] @@ -232,4 +262,4 @@ def update_secrets(self, existing_secrets: List[RemoteSecret]) -> List[RemoteSec new_remote_secret = self._create_new_secret_version(new_secret, old_secret) new_remote_secrets.append(new_remote_secret) self.logger.info(f"Updated {new_remote_secret.name} with new value") - return new_remote_secrets + return 0 From bc17f50de69b55937687dc82edc2c017c4dd9d67 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 5 Dec 2022 17:39:07 +0100 Subject: [PATCH 04/13] update readme --- tools/ci_credentials/README.md | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tools/ci_credentials/README.md b/tools/ci_credentials/README.md index 5a0438df2fed..6e1f87b85fa4 100644 --- a/tools/ci_credentials/README.md +++ b/tools/ci_credentials/README.md @@ -1,6 +1,8 @@ # CI Credentials +CLI tooling to read and manage GSM secrets: +- `write-to-storage` download a connector's secrets locally in the connector's `secret` folder +- `update-secrets` uploads new connector secret version that were locally updated. -Connects to GSM to download connection details. ## Development @@ -25,14 +27,22 @@ After making a change, you have to reinstall it to run the bash command: `pip in The `VERSION=dev` will make it so it knows to use your local current working directory and not the Github Action one. -Pass in a connector name. For example: +### Help ```bash -VERSION=dev ci_credentials destination-snowflake +ci_credentials --help ``` -To make sure it get's all changes every time, you can run this: +### Write to storage +To download GSM secrets to `airbyte-integrations/connectors/source-bings-ads/secrets`: +```bash +ci_credentials source-bing-ads write-to-storage +``` + +### Update secrets +To upload to GSM newly updated configurations from `airbyte-integrations/connectors/source-bings-ads/secrets/updated_configurations`: ```bash -pip install --quiet -e ./tools/ci_* && VERSION=dev ci_credentials destination-snowflake -``` \ No newline at end of file +ci_credentials source-bing-ads update-secrets +``` + From bfee38edaa8d64f5ba48a615c416c4eaa8d7d466 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 5 Dec 2022 17:39:27 +0100 Subject: [PATCH 05/13] bump version --- tools/ci_credentials/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ci_credentials/setup.py b/tools/ci_credentials/setup.py index 8427fcc2ea61..c0cdd5da7d23 100644 --- a/tools/ci_credentials/setup.py +++ b/tools/ci_credentials/setup.py @@ -10,7 +10,7 @@ TEST_REQUIREMENTS = ["requests-mock"] setup( - version="0.0.0", + version="1.0.0", name="ci_credentials", description="Load and extract CI secrets for test suites", author="Airbyte", From 02b812dc9e5354241bd9a9d341c81d9edb4657ef Mon Sep 17 00:00:00 2001 From: alafanechere Date: Mon, 5 Dec 2022 17:48:19 +0100 Subject: [PATCH 06/13] update test and publish workflows --- .github/workflows/publish-command.yml | 22 ++++++++++++++----- .github/workflows/test-command.yml | 20 +++++++++++++---- .../workflows/test-performance-command.yml | 20 +++++++++++++---- 3 files changed, 49 insertions(+), 13 deletions(-) diff --git a/.github/workflows/publish-command.yml b/.github/workflows/publish-command.yml index f9e654a1dc6c..29c07a48cef4 100644 --- a/.github/workflows/publish-command.yml +++ b/.github/workflows/publish-command.yml @@ -283,12 +283,12 @@ jobs: - name: Write Integration Test Credentials for ${{ matrix.connector }} run: | source venv/bin/activate - ci_credentials ${{ matrix.connector }} + ci_credentials ${{ matrix.connector }} write-to-storage # normalization also runs destination-specific tests, so fetch their creds also if [ 'bases/base-normalization' = "${{ matrix.connector }}" ] || [ 'base-normalization' = "${{ matrix.connector }}" ]; then - ci_credentials destination-bigquery - ci_credentials destination-postgres - ci_credentials destination-snowflake + ci_credentials destination-bigquery write-to-storage + ci_credentials destination-postgres write-to-storage + ci_credentials destination-snowflake write-to-storage fi env: GCP_GSM_CREDENTIALS: ${{ secrets.GCP_GSM_CREDENTIALS }} @@ -317,9 +317,21 @@ jobs: with: command: | echo "$SPEC_CACHE_SERVICE_ACCOUNT_KEY" > spec_cache_key_file.json && docker login -u ${DOCKER_HUB_USERNAME} -p ${DOCKER_HUB_PASSWORD} - ./tools/integrations/manage.sh publish airbyte-integrations/${{ matrix.connector }} ${{ github.event.inputs.run-tests }} --publish_spec_to_cache + ./tools/integrations/manage.sh publish airbyte-integrations/${{ matrix.connector }} ${{ github.event.inputs.run-tests }} --publish_spec_to_cache attempt_limit: 3 attempt_delay: 5000 in # ms + - name: Update Integration Test Credentials after test run for ${{ github.event.inputs.connector }} + run: | + source venv/bin/activate + ci_credentials ${{ matrix.connector }} update-secrets + # normalization also runs destination-specific tests, so fetch their creds also + if [ 'bases/base-normalization' = "${{ matrix.connector }}" ] || [ 'base-normalization' = "${{ matrix.connector }}" ]; then + ci_credentials destination-bigquery update-secrets + ci_credentials destination-postgres update-secrets + ci_credentials destination-snowflake update-secrets + fi + env: + GCP_GSM_CREDENTIALS: ${{ secrets.GCP_GSM_CREDENTIALS }} - name: Create Sentry Release if: startsWith(matrix.connector, 'connectors') && success() run: | diff --git a/.github/workflows/test-command.yml b/.github/workflows/test-command.yml index de003310976b..b24203d3be94 100644 --- a/.github/workflows/test-command.yml +++ b/.github/workflows/test-command.yml @@ -110,12 +110,12 @@ jobs: - name: Write Integration Test Credentials for ${{ github.event.inputs.connector }} run: | source venv/bin/activate - ci_credentials ${{ github.event.inputs.connector }} + ci_credentials ${{ github.event.inputs.connector }} write-to-storage # normalization also runs destination-specific tests, so fetch their creds also if [ 'bases/base-normalization' = "${{ github.event.inputs.connector }}" ] || [ 'base-normalization' = "${{ github.event.inputs.connector }}" ]; then - ci_credentials destination-bigquery - ci_credentials destination-postgres - ci_credentials destination-snowflake + ci_credentials destination-bigquery write-to-storage + ci_credentials destination-postgres write-to-storage + ci_credentials destination-snowflake write-to-storage fi env: GCP_GSM_CREDENTIALS: ${{ secrets.GCP_GSM_CREDENTIALS }} @@ -131,6 +131,18 @@ jobs: command: ./tools/bin/ci_integration_test.sh ${{ github.event.inputs.connector }} attempt_limit: 3 attempt_delay: 10000 # in ms + - name: Update Integration Test Credentials after test run for ${{ github.event.inputs.connector }} + run: | + source venv/bin/activate + ci_credentials ${{ github.event.inputs.connector }} update-secrets + # normalization also runs destination-specific tests, so fetch their creds also + if [ 'bases/base-normalization' = "${{ github.event.inputs.connector }}" ] || [ 'base-normalization' = "${{ github.event.inputs.connector }}" ]; then + ci_credentials destination-bigquery update-secrets + ci_credentials destination-postgres update-secrets + ci_credentials destination-snowflake update-secrets + fi + env: + GCP_GSM_CREDENTIALS: ${{ secrets.GCP_GSM_CREDENTIALS }} - name: Archive test reports artifacts if: github.event.inputs.comment-id && failure() uses: actions/upload-artifact@v3 diff --git a/.github/workflows/test-performance-command.yml b/.github/workflows/test-performance-command.yml index 11e51f2b2d10..5e3f05563320 100644 --- a/.github/workflows/test-performance-command.yml +++ b/.github/workflows/test-performance-command.yml @@ -107,12 +107,12 @@ jobs: - name: Write Integration Test Credentials for ${{ github.event.inputs.connector }} run: | source venv/bin/activate - ci_credentials ${{ github.event.inputs.connector }} + ci_credentials ${{ github.event.inputs.connector }} write-to-storage # normalization also runs destination-specific tests, so fetch their creds also if [ 'bases/base-normalization' = "${{ github.event.inputs.connector }}" ] || [ 'base-normalization' = "${{ github.event.inputs.connector }}" ]; then - ci_credentials destination-bigquery - ci_credentials destination-postgres - ci_credentials destination-snowflake + ci_credentials destination-bigquery write-to-storage + ci_credentials destination-postgres write-to-storage + ci_credentials destination-snowflake write-to-storage fi env: GCP_GSM_CREDENTIALS: ${{ secrets.GCP_GSM_CREDENTIALS }} @@ -124,6 +124,18 @@ jobs: ACTION_RUN_ID: ${{github.run_id}} # Oracle expects this variable to be set. Although usually present, this is not set by default on Github virtual runners. TZ: UTC + - name: Update Integration Test Credentials after test run for ${{ github.event.inputs.connector }} + run: | + source venv/bin/activate + ci_credentials ${{ github.event.inputs.connector }} update-secrets + # normalization also runs destination-specific tests, so fetch their creds also + if [ 'bases/base-normalization' = "${{ github.event.inputs.connector }}" ] || [ 'base-normalization' = "${{ github.event.inputs.connector }}" ]; then + ci_credentials destination-bigquery update-secrets + ci_credentials destination-postgres update-secrets + ci_credentials destination-snowflake update-secrets + fi + env: + GCP_GSM_CREDENTIALS: ${{ secrets.GCP_GSM_CREDENTIALS }} - name: Archive test reports artifacts if: github.event.inputs.comment-id && failure() uses: actions/upload-artifact@v3 From e2fa85dc2b8dc3e603f9ef7dc23a836c87fad851 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Tue, 6 Dec 2022 10:51:37 +0100 Subject: [PATCH 07/13] test secret manager --- .../ci_credentials/secrets_manager.py | 8 +- tools/ci_credentials/tests/test_secrets.py | 182 ------------------ 2 files changed, 4 insertions(+), 186 deletions(-) delete mode 100644 tools/ci_credentials/tests/test_secrets.py diff --git a/tools/ci_credentials/ci_credentials/secrets_manager.py b/tools/ci_credentials/ci_credentials/secrets_manager.py index 9e6617d0dbab..f129dc1c185c 100644 --- a/tools/ci_credentials/ci_credentials/secrets_manager.py +++ b/tools/ci_credentials/ci_credentials/secrets_manager.py @@ -218,9 +218,11 @@ def _get_updated_secrets(self) -> List[Secret]: Returns: List[Secret]: List of Secret instances parsed from local updated configuration files """ - updated_configurations_glob = glob(f"airbyte-integrations/connectors/{self.connector_name}/secrets/updated_configurations/*.json") + updated_configurations_glob = ( + f"{str(self.base_folder)}/airbyte-integrations/connectors/{self.connector_name}/secrets/updated_configurations/*.json" + ) updated_configuration_files_versions = {} - for updated_configuration_path in updated_configurations_glob: + for updated_configuration_path in glob(updated_configurations_glob): updated_configuration_path = Path(updated_configuration_path) with open(updated_configuration_path, "r") as updated_configuration: updated_configuration_value = json.load(updated_configuration) @@ -252,7 +254,6 @@ def update_secrets(self, existing_secrets: List[RemoteSecret]) -> int: """ existing_secrets = {secret.name: secret for secret in existing_secrets} updated_secrets = {secret.name: secret for secret in self._get_updated_secrets()} - new_remote_secrets = [] for existing_secret_name in existing_secrets: if existing_secret_name in updated_secrets and json.loads(updated_secrets[existing_secret_name].value) != json.loads( existing_secrets[existing_secret_name].value @@ -260,6 +261,5 @@ def update_secrets(self, existing_secrets: List[RemoteSecret]) -> int: new_secret = updated_secrets[existing_secret_name] old_secret = existing_secrets[existing_secret_name] new_remote_secret = self._create_new_secret_version(new_secret, old_secret) - new_remote_secrets.append(new_remote_secret) self.logger.info(f"Updated {new_remote_secret.name} with new value") return 0 diff --git a/tools/ci_credentials/tests/test_secrets.py b/tools/ci_credentials/tests/test_secrets.py deleted file mode 100644 index 84173c1df5f9..000000000000 --- a/tools/ci_credentials/tests/test_secrets.py +++ /dev/null @@ -1,182 +0,0 @@ -import base64 -import json -import re -import shutil -import tempfile -from pathlib import Path -from unittest.mock import patch - -import pytest -import requests_mock -from ci_credentials.main import ENV_GCP_GSM_CREDENTIALS -from ci_credentials.main import main - -from ci_credentials import SecretsLoader - -HERE = Path(__file__).resolve().parent -TEST_CONNECTOR_NAME = "source-test" -TEMP_FOLDER = Path(tempfile.mkdtemp()) - - -@pytest.fixture(autouse=True, scope="session") -def temp_folder(): - yield - shutil.rmtree(TEMP_FOLDER) - - -@pytest.mark.parametrize( - "connector_name,filename,expected_name", - ( - ("source-default", "config.json", "SECRET_SOURCE-DEFAULT__CREDS"), - ("source-custom-filename-1", "config_custom.json", "SECRET_SOURCE-CUSTOM-FILENAME-1_CUSTOM__CREDS"), - ("source-custom-filename-2", "auth.json", "SECRET_SOURCE-CUSTOM-FILENAME-2_AUTH__CREDS"), - ("source-custom-filename-3", "config_auth-test---___---config.json", - "SECRET_SOURCE-CUSTOM-FILENAME-3_AUTH-TEST__CREDS"), - ("source-custom-filename-4", "_____config_test---config.json", - "SECRET_SOURCE-CUSTOM-FILENAME-4_TEST__CREDS"), - ) -) -def test_secret_name_generation(connector_name, filename, expected_name): - assert SecretsLoader.generate_secret_name(connector_name, filename) == expected_name - - -def read_last_log_message(capfd): - _, err = capfd.readouterr() - print(err) - return err.split("# ")[-1].strip() - - -def test_main(capfd, monkeypatch): - # without parameters and envs - monkeypatch.delenv(ENV_GCP_GSM_CREDENTIALS, raising=False) - monkeypatch.setattr("sys.argv", [None, TEST_CONNECTOR_NAME, "fake_arg"]) - assert main() == 1 - assert "one script argument only" in read_last_log_message(capfd) - - monkeypatch.setattr("sys.argv", [None, TEST_CONNECTOR_NAME]) - # without env values - assert main() == 1 - assert "shouldn't be empty" in read_last_log_message(capfd) - - # incorrect GCP_GSM_CREDENTIALS - monkeypatch.setenv(ENV_GCP_GSM_CREDENTIALS, "non-json") - assert main() == 1 - assert "incorrect GCP_GSM_CREDENTIALS value" in read_last_log_message(capfd) - - # empty GCP_GSM_CREDENTIALS - monkeypatch.setenv(ENV_GCP_GSM_CREDENTIALS, "{}") - assert main() == 1 - assert "GCP_GSM_CREDENTIALS shouldn't be empty!" in read_last_log_message(capfd) - - # successful result - monkeypatch.setenv(ENV_GCP_GSM_CREDENTIALS, '{"test": "test"}') - - monkeypatch.setattr(SecretsLoader, "read_from_gsm", lambda *args, **kwargs: {}) - monkeypatch.setattr(SecretsLoader, "write_to_storage", lambda *args, **kwargs: 0) - assert main() == 0 - - -@pytest.mark.parametrize( - "connector_name,gsm_secrets,expected_secrets", - ( - ( - "source-gsm-only", - { - "config": {"test_key": "test_value"}, - "config_oauth": {"test_key_1": "test_key_2"}, - }, - [ - ("config.json", {"test_key": "test_value"}), - ("config_oauth.json", {"test_key_1": "test_key_2"}), - ] - ), - ), - ids=["gsm_only", ] - -) -@patch('ci_common_utils.GoogleApi.get_access_token', lambda *args: ("fake_token", None)) -@patch('ci_common_utils.GoogleApi.project_id', "fake_id") -def test_read(connector_name, gsm_secrets, expected_secrets): - matcher_gsm_list = re.compile("https://secretmanager.googleapis.com/v1/projects/.+/secrets") - secrets_list = {"secrets": [{ - "name": f"projects//secrets/SECRET_{connector_name.upper()}_{i}_CREDS", - "labels": { - "filename": k, - "connector": connector_name, - } - } for i, k in enumerate(gsm_secrets)]} - - matcher_versions = re.compile("https://secretmanager.googleapis.com/v1/.+/versions") - versions_response_list = [{"json": { - "versions": [{ - "name": f"projects//secrets/SECRET_{connector_name.upper()}_{i}_CREDS/versions/1", - "state": "ENABLED", - }] - }} for i in range(len(gsm_secrets))] - - matcher_secret = re.compile("https://secretmanager.googleapis.com/v1/.+/1:access") - secrets_response_list = [{ - "json": {"payload": {"data": base64.b64encode(json.dumps(v).encode()).decode("utf-8")}} - } for v in gsm_secrets.values()] - - matcher_version = re.compile("https://secretmanager.googleapis.com/v1/.+:addVersion") - loader = SecretsLoader(connector_name=connector_name, gsm_credentials={}) - with requests_mock.Mocker() as m: - m.get(matcher_gsm_list, json=secrets_list) - m.post(matcher_gsm_list, json={"name": ""}) - m.post(matcher_version, json={}) - m.get(matcher_versions, versions_response_list) - m.get(matcher_secret, secrets_response_list) - - secrets = [(*k, v.replace(" ", "")) for k, v in loader.read_from_gsm().items()] - expected_secrets = [(connector_name, k[0], json.dumps(k[1]).replace(" ", "")) for k in expected_secrets] - # raise Exception("%s => %s" % (secrets, expected_secrets)) - # raise Exception(set(secrets).symmetric_difference(set(expected_secrets))) - assert not set(secrets).symmetric_difference(set(expected_secrets)) - - -@pytest.mark.parametrize( - "connector_name,secrets,expected_files", - ( - ("source-test", {"test.json": "test_value"}, - ["airbyte-integrations/connectors/source-test/secrets/test.json"]), - - ("source-test2", {"test.json": "test_value", "auth.json": "test_auth"}, - ["airbyte-integrations/connectors/source-test2/secrets/test.json", - "airbyte-integrations/connectors/source-test2/secrets/auth.json"]), - - ("base-normalization", {"test.json": "test_value", "auth.json": "test_auth"}, - ["airbyte-integrations/bases/base-normalization/secrets/test.json", - "airbyte-integrations/bases/base-normalization/secrets/auth.json"]), - ), - ids=["single", "multi", "base-normalization"], -) -def test_write(connector_name, secrets, expected_files): - loader = SecretsLoader(connector_name=connector_name, gsm_credentials={}) - loader.base_folder = TEMP_FOLDER - loader.write_to_storage({(connector_name, k): v for k, v in secrets.items()}) - for expected_file in expected_files: - target_file = TEMP_FOLDER / expected_file - assert target_file.exists() - has = False - for k, v in secrets.items(): - if target_file.name == k: - with open(target_file, "r") as f: - assert f.read() == v - has = True - break - assert has, f"incorrect file data: {target_file}" - - -@pytest.mark.parametrize( - "connector_name,dict_json_value,expected_secret", - ( - ("source-default", "{\"org_id\": 111}", "::add-mask::111"), - ("source-default", "{\"org\": 111}", ""), - ) -) -def test_validate_mask_values(connector_name, dict_json_value, expected_secret, capsys): - loader = SecretsLoader(connector_name=connector_name, gsm_credentials={}) - json_value = json.loads(dict_json_value) - loader.mask_secrets_from_action_log(None, json_value) - assert expected_secret in capsys.readouterr().out From c34d98724c85aaa5864cc520c8f88d22eed453a3 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Tue, 6 Dec 2022 10:51:57 +0100 Subject: [PATCH 08/13] update setup.py --- tools/ci_credentials/setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/ci_credentials/setup.py b/tools/ci_credentials/setup.py index c0cdd5da7d23..538e79b565fd 100644 --- a/tools/ci_credentials/setup.py +++ b/tools/ci_credentials/setup.py @@ -7,12 +7,12 @@ MAIN_REQUIREMENTS = ["requests", "ci_common_utils", "click~=8.1.3"] -TEST_REQUIREMENTS = ["requests-mock"] +TEST_REQUIREMENTS = ["requests-mock", "pytest"] setup( version="1.0.0", name="ci_credentials", - description="Load and extract CI secrets for test suites", + description="CLI tooling to read and manage GSM secrets", author="Airbyte", author_email="contact@airbyte.io", packages=find_packages(), From 1e75110cadffcb25ea691363325a6b3f88b066ca Mon Sep 17 00:00:00 2001 From: alafanechere Date: Tue, 6 Dec 2022 10:59:01 +0100 Subject: [PATCH 09/13] test secret manager --- tools/ci_credentials/tests/test_models.py | 47 +++++ .../tests/test_secrets_manager.py | 194 ++++++++++++++++++ 2 files changed, 241 insertions(+) create mode 100644 tools/ci_credentials/tests/test_models.py create mode 100644 tools/ci_credentials/tests/test_secrets_manager.py diff --git a/tools/ci_credentials/tests/test_models.py b/tools/ci_credentials/tests/test_models.py new file mode 100644 index 000000000000..178f3c5e2bd0 --- /dev/null +++ b/tools/ci_credentials/tests/test_models.py @@ -0,0 +1,47 @@ +# +# Copyright (c) 2022 Airbyte, Inc., all rights reserved. +# +import pytest +from ci_credentials.models import Secret + + +@pytest.mark.parametrize( + "connector_name,filename,expected_name, expected_directory", + ( + ("source-default", "config.json", "SECRET_SOURCE-DEFAULT__CREDS", "airbyte-integrations/connectors/source-default/secrets"), + ( + "source-custom-filename-1", + "config_custom.json", + "SECRET_SOURCE-CUSTOM-FILENAME-1_CUSTOM__CREDS", + "airbyte-integrations/connectors/source-custom-filename-1/secrets", + ), + ( + "source-custom-filename-2", + "auth.json", + "SECRET_SOURCE-CUSTOM-FILENAME-2_AUTH__CREDS", + "airbyte-integrations/connectors/source-custom-filename-2/secrets", + ), + ( + "source-custom-filename-3", + "config_auth-test---___---config.json", + "SECRET_SOURCE-CUSTOM-FILENAME-3_AUTH-TEST__CREDS", + "airbyte-integrations/connectors/source-custom-filename-3/secrets", + ), + ( + "source-custom-filename-4", + "_____config_test---config.json", + "SECRET_SOURCE-CUSTOM-FILENAME-4_TEST__CREDS", + "airbyte-integrations/connectors/source-custom-filename-4/secrets", + ), + ( + "base-normalization", + "_____config_test---config.json", + "SECRET_BASE-NORMALIZATION_TEST__CREDS", + "airbyte-integrations/bases/base-normalization/secrets", + ), + ), +) +def test_secret_instantiation(connector_name, filename, expected_name, expected_directory): + secret = Secret(connector_name, filename, "secret_value") + assert secret.name == expected_name + assert secret.directory == expected_directory diff --git a/tools/ci_credentials/tests/test_secrets_manager.py b/tools/ci_credentials/tests/test_secrets_manager.py new file mode 100644 index 000000000000..6ee6133d008b --- /dev/null +++ b/tools/ci_credentials/tests/test_secrets_manager.py @@ -0,0 +1,194 @@ +# +# Copyright (c) 2022 Airbyte, Inc., all rights reserved. +# +import base64 +import json +import re +from unittest.mock import patch + +import pytest +import requests_mock +from ci_credentials import SecretsManager +from ci_credentials.models import RemoteSecret, Secret + + +@pytest.fixture +def matchers(): + return { + "secrets": re.compile("https://secretmanager.googleapis.com/v1/projects/.+/secrets"), + "versions": re.compile("https://secretmanager.googleapis.com/v1/.+/versions"), + "addVersion": re.compile("https://secretmanager.googleapis.com/v1/.+:addVersion"), + "access": re.compile("https://secretmanager.googleapis.com/v1/.+/1:access"), + "disable": re.compile("https://secretmanager.googleapis.com/v1/.+:disable"), + } + + +@pytest.mark.parametrize( + "connector_name,gsm_secrets,expected_secrets", + ( + ( + "source-gsm-only", + { + "config": {"test_key": "test_value"}, + "config_oauth": {"test_key_1": "test_key_2"}, + }, + [ + RemoteSecret( + "source-gsm-only", + "config.json", + '{"test_key":"test_value"}', + "projects//secrets/SECRET_SOURCE-GSM-ONLY_0_CREDS/versions/1", + ), + RemoteSecret( + "source-gsm-only", + "config_oauth.json", + '{"test_key_1":"test_key_2"}', + "projects//secrets/SECRET_SOURCE-GSM-ONLY_1_CREDS/versions/1", + ), + ], + ), + ), + ids=[ + "gsm_only", + ], +) +@patch("ci_common_utils.GoogleApi.get_access_token", lambda *args: ("fake_token", None)) +@patch("ci_common_utils.GoogleApi.project_id", "fake_id") +def test_read(matchers, connector_name, gsm_secrets, expected_secrets): + secrets_list = { + "secrets": [ + { + "name": f"projects//secrets/SECRET_{connector_name.upper()}_{i}_CREDS", + "labels": { + "filename": k, + "connector": connector_name, + }, + } + for i, k in enumerate(gsm_secrets) + ] + } + + versions_response_list = [ + { + "json": { + "versions": [ + { + "name": f"projects//secrets/SECRET_{connector_name.upper()}_{i}_CREDS/versions/1", + "state": "ENABLED", + } + ] + } + } + for i in range(len(gsm_secrets)) + ] + + secrets_response_list = [ + {"json": {"payload": {"data": base64.b64encode(json.dumps(v).encode()).decode("utf-8")}}} for v in gsm_secrets.values() + ] + + manager = SecretsManager(connector_name=connector_name, gsm_credentials={}) + with requests_mock.Mocker() as m: + m.get(matchers["secrets"], json=secrets_list) + m.post(matchers["secrets"], json={"name": ""}) + m.get(matchers["versions"], versions_response_list) + m.get(matchers["access"], secrets_response_list) + + secrets = manager.read_from_gsm() + assert secrets == expected_secrets + + +@pytest.mark.parametrize( + "connector_name,secrets,expected_files", + ( + ( + "source-test", + [Secret("source-test", "test_config.json", "test_value")], + ["airbyte-integrations/connectors/source-test/secrets/test_config.json"], + ), + ( + "source-test2", + [Secret("source-test2", "test.json", "test_value"), Secret("source-test2", "auth.json", "test_auth")], + [ + "airbyte-integrations/connectors/source-test2/secrets/test.json", + "airbyte-integrations/connectors/source-test2/secrets/auth.json", + ], + ), + ( + "base-normalization", + [Secret("base-normalization", "test.json", "test_value"), Secret("base-normalization", "auth.json", "test_auth")], + [ + "airbyte-integrations/bases/base-normalization/secrets/test.json", + "airbyte-integrations/bases/base-normalization/secrets/auth.json", + ], + ), + ), +) +def test_write(tmp_path, connector_name, secrets, expected_files): + manager = SecretsManager(connector_name=connector_name, gsm_credentials={}) + manager.base_folder = tmp_path + manager.write_to_storage(secrets) + for expected_file in expected_files: + target_file = tmp_path / expected_file + assert target_file.exists() + has = False + for secret in secrets: + if target_file.name == secret.configuration_file_name: + with open(target_file, "r") as f: + assert f.read() == secret.value + has = True + break + assert has, f"incorrect file data: {target_file}" + + +@pytest.mark.parametrize( + "connector_name,dict_json_value,expected_secret", + ( + ("source-default", '{"org_id": 111}', "::add-mask::111"), + ("source-default", '{"org": 111}', ""), + ), +) +def test_validate_mask_values(connector_name, dict_json_value, expected_secret, capsys): + manager = SecretsManager(connector_name=connector_name, gsm_credentials={}) + json_value = json.loads(dict_json_value) + manager.mask_secrets_from_action_log(None, json_value) + assert expected_secret in capsys.readouterr().out + + +@patch("ci_common_utils.GoogleApi.get_access_token", lambda *args: ("fake_token", None)) +@patch("ci_common_utils.GoogleApi.project_id", "fake_id") +@pytest.mark.parametrize( + "old_secret_value, updated_configurations", + [ + (json.dumps({"key": "value"}), [json.dumps({"key": "new_value_1"}), json.dumps({"key": "new_value_2"})]), + (json.dumps({"key": "value"}), [json.dumps({"key": "value"})]), + ], +) +def test_update_secrets(tmp_path, matchers, old_secret_value, updated_configurations): + existing_secret = RemoteSecret("source-test", "config.json", old_secret_value, "previous_version") + existing_secrets = [existing_secret] + + manager = SecretsManager(connector_name="source-test", gsm_credentials={}) + manager.base_folder = tmp_path + updated_configuration_directory = tmp_path / "airbyte-integrations/connectors/source-test/secrets/updated_configurations" + updated_configuration_directory.mkdir(parents=True) + + for i, updated_configuration in enumerate(updated_configurations): + stem, ext = existing_secret.configuration_file_name.split(".") + updated_configuration_file_name = f"{stem}|{i}.{ext}" + updated_configuration_path = updated_configuration_directory / updated_configuration_file_name + with open(updated_configuration_path, "w") as f: + f.write(updated_configuration) + + with requests_mock.Mocker() as m: + add_version_adapter = m.post(matchers["addVersion"], json={"name": "new_version"}) + disable_version_adapter = m.post(matchers["disable"], json={}) + assert manager.update_secrets(existing_secrets) == 0 + + if old_secret_value != updated_configurations[-1]: + # We confirm the new version was created from the latest updated_configuration value + expected_add_version_payload = {"payload": {"data": base64.b64encode(updated_configurations[-1].encode()).decode("utf-8")}} + assert add_version_adapter.last_request.json() == expected_add_version_payload + assert disable_version_adapter.called_once + else: + assert not add_version_adapter.called + assert not disable_version_adapter.called From 7460b6e7db84beb01c8b284f7709250c016f7e11 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 7 Dec 2022 10:17:06 +0100 Subject: [PATCH 10/13] fix typo --- tools/ci_credentials/ci_credentials/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/ci_credentials/ci_credentials/models.py b/tools/ci_credentials/ci_credentials/models.py index 20b04808847d..4955740efe34 100644 --- a/tools/ci_credentials/ci_credentials/models.py +++ b/tools/ci_credentials/ci_credentials/models.py @@ -2,7 +2,7 @@ # Copyright (c) 2022 Airbyte, Inc., all rights reserved. # -from __future__ import ( # Used to evaluate type hints at runtime, a NameError: name 'ConfigObserver' is not defined is thrown otherwise +from __future__ import ( # Used to evaluate type hints at runtime, a NameError: name 'RemoteSecret' is not defined is thrown otherwise annotations, ) From d3b9559c114f024f9b5271a58f967bab9eece7d5 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 7 Dec 2022 10:34:26 +0100 Subject: [PATCH 11/13] better returns --- tools/ci_credentials/ci_credentials/main.py | 9 +++++++-- .../ci_credentials/secrets_manager.py | 13 ++++++++----- tools/ci_credentials/tests/test_secrets_manager.py | 8 +++++++- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/tools/ci_credentials/ci_credentials/main.py b/tools/ci_credentials/ci_credentials/main.py index 3cf41e4a1f77..d14e959c232c 100644 --- a/tools/ci_credentials/ci_credentials/main.py +++ b/tools/ci_credentials/ci_credentials/main.py @@ -49,13 +49,18 @@ def ci_credentials(ctx, connector_name: str, gcp_gsm_credentials): @ci_credentials.command(help="Download GSM secrets locally to the connector's secrets directory.") @click.pass_context def write_to_storage(ctx): - return ctx.obj["secret_manager"].write_to_storage(ctx.obj["connector_secrets"]) + written_files = ctx.obj["secret_manager"].write_to_storage(ctx.obj["connector_secrets"]) + written_files_count = len(written_files) + click.echo(f"{written_files_count} secret files were written: {','.join(written_files)}") @ci_credentials.command(help="Update GSM secrets according to the content of the secrets/updated_configurations directory.") @click.pass_context def update_secrets(ctx): - return ctx.obj["secret_manager"].update_secrets(ctx.obj["connector_secrets"]) + new_remote_secrets = ctx.obj["secret_manager"].update_secrets(ctx.obj["connector_secrets"]) + updated_secret_names = [secret.name for secret in new_remote_secrets] + updated_secrets_count = len(new_remote_secrets) + click.echo(f"Updated {updated_secrets_count} secrets: {','.join(updated_secret_names)}") if __name__ == "__main__": diff --git a/tools/ci_credentials/ci_credentials/secrets_manager.py b/tools/ci_credentials/ci_credentials/secrets_manager.py index f129dc1c185c..17f4d33ad5e7 100644 --- a/tools/ci_credentials/ci_credentials/secrets_manager.py +++ b/tools/ci_credentials/ci_credentials/secrets_manager.py @@ -171,8 +171,9 @@ def read_from_gsm(self) -> List[RemoteSecret]: return [] return secrets - def write_to_storage(self, secrets: List[RemoteSecret]) -> int: + def write_to_storage(self, secrets: List[RemoteSecret]) -> List[str]: """Tries to save target secrets to the airbyte-integrations/connectors|bases/{connector_name}/secrets folder""" + written_files = [] if not secrets: return 0 for secret in secrets: @@ -181,8 +182,8 @@ def write_to_storage(self, secrets: List[RemoteSecret]) -> int: filepath = secrets_dir / secret.configuration_file_name with open(filepath, "w") as file: file.write(secret.value) - self.logger.info(f"The file {filepath} was saved") - return 0 + written_files.append(filepath) + return written_files def _create_new_secret_version(self, new_secret: Secret, old_secret: RemoteSecret) -> RemoteSecret: """Create a new secret version from a new secret instance. Disable the previous secret version. @@ -243,7 +244,7 @@ def _get_updated_secrets(self) -> List[Secret]: for configuration_file_name, versions_by_creation_time in updated_configuration_files_versions.items() ] - def update_secrets(self, existing_secrets: List[RemoteSecret]) -> int: + def update_secrets(self, existing_secrets: List[RemoteSecret]) -> List[RemoteSecret]: """Update existing secrets if an updated version was found locally. Args: @@ -254,6 +255,7 @@ def update_secrets(self, existing_secrets: List[RemoteSecret]) -> int: """ existing_secrets = {secret.name: secret for secret in existing_secrets} updated_secrets = {secret.name: secret for secret in self._get_updated_secrets()} + new_remote_secrets = [] for existing_secret_name in existing_secrets: if existing_secret_name in updated_secrets and json.loads(updated_secrets[existing_secret_name].value) != json.loads( existing_secrets[existing_secret_name].value @@ -261,5 +263,6 @@ def update_secrets(self, existing_secrets: List[RemoteSecret]) -> int: new_secret = updated_secrets[existing_secret_name] old_secret = existing_secrets[existing_secret_name] new_remote_secret = self._create_new_secret_version(new_secret, old_secret) + new_remote_secrets.append(new_remote_secret) self.logger.info(f"Updated {new_remote_secret.name} with new value") - return 0 + return new_remote_secrets diff --git a/tools/ci_credentials/tests/test_secrets_manager.py b/tools/ci_credentials/tests/test_secrets_manager.py index 6ee6133d008b..349db85a7671 100644 --- a/tools/ci_credentials/tests/test_secrets_manager.py +++ b/tools/ci_credentials/tests/test_secrets_manager.py @@ -182,13 +182,19 @@ def test_update_secrets(tmp_path, matchers, old_secret_value, updated_configurat with requests_mock.Mocker() as m: add_version_adapter = m.post(matchers["addVersion"], json={"name": "new_version"}) disable_version_adapter = m.post(matchers["disable"], json={}) - assert manager.update_secrets(existing_secrets) == 0 + updated_secrets = manager.update_secrets(existing_secrets) if old_secret_value != updated_configurations[-1]: # We confirm the new version was created from the latest updated_configuration value + for secret in updated_secrets: + assert secret.connector_name == "source-test" + assert secret.configuration_file_name == "config.json" + assert secret.value == updated_configurations[-1] + assert secret.enabled_version == "new_version" expected_add_version_payload = {"payload": {"data": base64.b64encode(updated_configurations[-1].encode()).decode("utf-8")}} assert add_version_adapter.last_request.json() == expected_add_version_payload assert disable_version_adapter.called_once else: + assert not updated_secrets assert not add_version_adapter.called assert not disable_version_adapter.called From ed8c4288882eb189384c145ccd7b6534e3156b9a Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 7 Dec 2022 10:40:14 +0100 Subject: [PATCH 12/13] better returns --- tools/ci_credentials/tests/test_secrets_manager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/ci_credentials/tests/test_secrets_manager.py b/tools/ci_credentials/tests/test_secrets_manager.py index 349db85a7671..c0239dd44d7f 100644 --- a/tools/ci_credentials/tests/test_secrets_manager.py +++ b/tools/ci_credentials/tests/test_secrets_manager.py @@ -126,10 +126,11 @@ def test_read(matchers, connector_name, gsm_secrets, expected_secrets): def test_write(tmp_path, connector_name, secrets, expected_files): manager = SecretsManager(connector_name=connector_name, gsm_credentials={}) manager.base_folder = tmp_path - manager.write_to_storage(secrets) + written_files = manager.write_to_storage(secrets) for expected_file in expected_files: target_file = tmp_path / expected_file assert target_file.exists() + assert target_file in written_files has = False for secret in secrets: if target_file.name == secret.configuration_file_name: From 1656af2888a1ba821300638ebbb0955e72f91b7a Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 7 Dec 2022 10:43:09 +0100 Subject: [PATCH 13/13] better returns --- tools/ci_credentials/ci_credentials/main.py | 2 +- .../ci_credentials/secrets_manager.py | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/ci_credentials/ci_credentials/main.py b/tools/ci_credentials/ci_credentials/main.py index d14e959c232c..3620149092d2 100644 --- a/tools/ci_credentials/ci_credentials/main.py +++ b/tools/ci_credentials/ci_credentials/main.py @@ -51,7 +51,7 @@ def ci_credentials(ctx, connector_name: str, gcp_gsm_credentials): def write_to_storage(ctx): written_files = ctx.obj["secret_manager"].write_to_storage(ctx.obj["connector_secrets"]) written_files_count = len(written_files) - click.echo(f"{written_files_count} secret files were written: {','.join(written_files)}") + click.echo(f"{written_files_count} secret files were written: {','.join([str(path) for path in written_files])}") @ci_credentials.command(help="Update GSM secrets according to the content of the secrets/updated_configurations directory.") diff --git a/tools/ci_credentials/ci_credentials/secrets_manager.py b/tools/ci_credentials/ci_credentials/secrets_manager.py index 17f4d33ad5e7..4f164d5bfd0b 100644 --- a/tools/ci_credentials/ci_credentials/secrets_manager.py +++ b/tools/ci_credentials/ci_credentials/secrets_manager.py @@ -171,8 +171,15 @@ def read_from_gsm(self) -> List[RemoteSecret]: return [] return secrets - def write_to_storage(self, secrets: List[RemoteSecret]) -> List[str]: - """Tries to save target secrets to the airbyte-integrations/connectors|bases/{connector_name}/secrets folder""" + def write_to_storage(self, secrets: List[RemoteSecret]) -> List[Path]: + """Save target secrets to the airbyte-integrations/connectors|bases/{connector_name}/secrets folder + + Args: + secrets (List[RemoteSecret]): List of remote secret to write locally + + Returns: + List[Path]: List of paths were the secrets were written + """ written_files = [] if not secrets: return 0 @@ -251,7 +258,7 @@ def update_secrets(self, existing_secrets: List[RemoteSecret]) -> List[RemoteSec existing_secrets (List[RemoteSecret]): List of existing secrets for the current connector on GSM. Returns: - int: Status code + List[RemoteSecret]: List of updated secrets as RemoteSecret instances """ existing_secrets = {secret.name: secret for secret in existing_secrets} updated_secrets = {secret.name: secret for secret in self._get_updated_secrets()}