From 69e4eba0a9a3d58576a52c4d90d66bddb884ce99 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Sun, 6 Mar 2022 01:15:40 +0100 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=90=99=20octavia-cli:=20implement=20s?= =?UTF-8?q?ecrement=20management?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- octavia-cli/README.md | 16 ++++++-- .../destination_postgres/expected.yaml | 6 +-- .../destination_s3/expected.yaml | 6 +-- .../destination_s3/input_spec.yaml | 6 +-- .../source_postgres/expected.yaml | 6 +-- octavia-cli/octavia_cli/apply/diff_helpers.py | 19 ++++------ octavia-cli/octavia_cli/apply/resources.py | 37 ++++++++++--------- octavia-cli/octavia_cli/apply/yaml_loaders.py | 35 ++++++++++++++++++ octavia-cli/octavia_cli/generate/renderer.py | 4 +- .../templates/source_or_destination.yaml.j2 | 2 +- .../test_apply/test_diff_helpers.py | 10 ++--- .../unit_tests/test_apply/test_resources.py | 33 +++++++++-------- .../test_apply/test_yaml_loaders.py | 37 +++++++++++++++++++ .../unit_tests/test_generate/test_renderer.py | 2 +- 14 files changed, 149 insertions(+), 70 deletions(-) create mode 100644 octavia-cli/octavia_cli/apply/yaml_loaders.py create mode 100644 octavia-cli/unit_tests/test_apply/test_yaml_loaders.py diff --git a/octavia-cli/README.md b/octavia-cli/README.md index 26b2242a834b8..ef9e0fcca11a8 100644 --- a/octavia-cli/README.md +++ b/octavia-cli/README.md @@ -21,13 +21,22 @@ SUB_BUILD=OCTAVIA_CLI ./gradlew build #from the root of the repo 2. Run the CLI from docker: ```bash docker run airbyte/octavia-cli:dev -```` +``` 3. Create an `octavia` alias in your `.bashrc` or `.zshrc`: -````bash +```bash echo 'alias octavia="docker run airbyte/octavia-cli:dev"' >> ~/.zshrc source ~/.zshrc octavia -```` +``` + +# Secret management +Sources and destinations configurations have credential fields that you **do not want to store as plain text and version on Git**. +`octavia` offers secret management through environment variables expansion: +```yaml +configuration: + password: ${MY_PASSWORD} +``` +If you have set a `MY_PASSWORD` environment variable, `octavia apply` will load its value into the `password` field. # Current development status Octavia is currently under development. @@ -38,6 +47,7 @@ We welcome community contributions! | Date | Milestone | |------------|-------------------------------------| +| 2022-03-06 | Implement secret management through environment variable expansion | | 2022-03-02 | Implement `octavia apply` (sources and destination only)| | 2022-02-06 | Implement `octavia generate` (sources and destination only)| | 2022-01-25 | Implement `octavia init` + some context checks| diff --git a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_postgres/expected.yaml b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_postgres/expected.yaml index a4881201b3526..22de2875e6e8f 100644 --- a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_postgres/expected.yaml +++ b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_postgres/expected.yaml @@ -13,7 +13,7 @@ configuration: database: # REQUIRED | string | Name of the database. schema: "public" # REQUIRED | string | The default schema tables are written to if the source does not specify a namespace. The usual value for this field is "public". | Example: public username: # REQUIRED | string | Username to use to access the database. - password: # SECRET | OPTIONAL | string | Password associated with the username. + password: ${PASSWORD} # SECRET (please store in environment variables) | OPTIONAL | string | Password associated with the username. ssl: # OPTIONAL | boolean | Encrypt data using SSL. tunnel_method: ## -------- Pick one valid structure among the examples below: -------- @@ -23,10 +23,10 @@ configuration: # tunnel_host: # REQUIRED | string | Hostname of the jump server host that allows inbound ssh tunnel. # tunnel_port: 22 # REQUIRED | integer | Port on the proxy/jump server that accepts inbound ssh connections. | Example: 22 # tunnel_user: # REQUIRED | string | OS-level username for logging into the jump server host. - # ssh_key: # SECRET | REQUIRED | string | OS-level user account ssh key credentials in RSA PEM format ( created with ssh-keygen -t rsa -m PEM -f myuser_rsa ) + # ssh_key: ${SSH_KEY} # SECRET (please store in environment variables) | REQUIRED | string | OS-level user account ssh key credentials in RSA PEM format ( created with ssh-keygen -t rsa -m PEM -f myuser_rsa ) ## -------- Another valid structure for tunnel_method: -------- # tunnel_method: "SSH_PASSWORD_AUTH" # REQUIRED | string | Connect through a jump server tunnel host using username and password authentication # tunnel_host: # REQUIRED | string | Hostname of the jump server host that allows inbound ssh tunnel. # tunnel_port: 22 # REQUIRED | integer | Port on the proxy/jump server that accepts inbound ssh connections. | Example: 22 # tunnel_user: # REQUIRED | string | OS-level username for logging into the jump server host - # tunnel_user_password: # SECRET | REQUIRED | string | OS-level password for logging into the jump server host + # tunnel_user_password: ${TUNNEL_USER_PASSWORD} # SECRET (please store in environment variables) | REQUIRED | string | OS-level password for logging into the jump server host diff --git a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/expected.yaml b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/expected.yaml index 6b6c7a0ceb351..0d2d835c3d522 100644 --- a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/expected.yaml +++ b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/expected.yaml @@ -12,8 +12,8 @@ configuration: s3_bucket_name: # REQUIRED | string | The name of the S3 bucket. | Example: airbyte_sync s3_bucket_path: # REQUIRED | string | Directory under the S3 bucket where data will be written. | Example: data_sync/test s3_bucket_region: # REQUIRED | string | The region of the S3 bucket. - access_key_id: # SECRET | OPTIONAL | string | The access key id to access the S3 bucket. Airbyte requires Read and Write permissions to the given bucket, if not set, Airbyte will rely on Instance Profile. | Example: A012345678910EXAMPLE - secret_access_key: # SECRET | OPTIONAL | string | The corresponding secret to the access key id, if S3 Key Id is set, then S3 Access Key must also be provided | Example: a012345678910ABCDEFGH/AbCdEfGhEXAMPLEKEY + access_key_id: ${ACCESS_KEY_ID} # SECRET (please store in environment variables) | OPTIONAL | string | The access key id to access the S3 bucket. Airbyte requires Read and Write permissions to the given bucket, if not set, Airbyte will rely on Instance Profile. | Example: A012345678910EXAMPLE + secret_access_key: ${SECRET_ACCESS_KEY} # SECRET (please store in environment variables) | OPTIONAL | string | The corresponding secret to the access key id, if S3 Key Id is set, then S3 Access Key must also be provided | Example: a012345678910ABCDEFGH/AbCdEfGhEXAMPLEKEY format: ## -------- Pick one valid structure among the examples below: -------- format_type: "Avro" # REQUIRED | string} @@ -31,7 +31,7 @@ configuration: ## -------- Another valid structure for compression_codec: -------- # codec: "zstandard" # REQUIRED | string # compression_level: 3 # REQUIRED | integer | Negative levels are 'fast' modes akin to lz4 or snappy, levels above 9 are generally for archival purposes, and levels above 18 use a lot of memory. - # include_checksum: # OPTIONAL | boolean | If true, include a checksum with each data block. + # include_hash: # OPTIONAL | boolean | If true, include a hash with each data block. ## -------- Another valid structure for compression_codec: -------- # codec: "snappy" # REQUIRED | string part_size_mb: 5 # OPTIONAL | integer | This is the size of a "Part" being buffered in memory. It limits the memory usage when writing. Larger values will allow to upload a bigger files and improve the speed, but consumes9 more memory. Allowed values: min=5MB, max=525MB Default: 5MB. | Example: 5 diff --git a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/input_spec.yaml b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/input_spec.yaml index 2d332c60efd8f..ed2b974503a5c 100644 --- a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/input_spec.yaml +++ b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/input_spec.yaml @@ -180,9 +180,9 @@ spec: default: 3 minimum: -5 maximum: 22 - include_checksum: - title: "Include checksum" - description: "If true, include a checksum with each data block." + include_hash: + title: "Include hash" + description: "If true, include a hash with each data block." type: "boolean" default: false - title: "snappy" diff --git a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/source_postgres/expected.yaml b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/source_postgres/expected.yaml index 42d32e70b73b3..5f32b03ddd0a7 100644 --- a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/source_postgres/expected.yaml +++ b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/source_postgres/expected.yaml @@ -13,7 +13,7 @@ configuration: database: # REQUIRED | string | Name of the database. schemas: ["public"] # OPTIONAL | array | The list of schemas to sync from. Defaults to user. Case sensitive. username: # REQUIRED | string | Username to use to access the database. - password: # SECRET | OPTIONAL | string | Password associated with the username. + password: ${PASSWORD} # SECRET (please store in environment variables) | OPTIONAL | string | Password associated with the username. ssl: # OPTIONAL | boolean | Encrypt client/server communications for increased security. replication_method: ## -------- Pick one valid structure among the examples below: -------- @@ -31,10 +31,10 @@ configuration: # tunnel_host: # REQUIRED | string | Hostname of the jump server host that allows inbound ssh tunnel. # tunnel_port: 22 # REQUIRED | integer | Port on the proxy/jump server that accepts inbound ssh connections. | Example: 22 # tunnel_user: # REQUIRED | string | OS-level username for logging into the jump server host. - # ssh_key: # SECRET | REQUIRED | string | OS-level user account ssh key credentials in RSA PEM format ( created with ssh-keygen -t rsa -m PEM -f myuser_rsa ) + # ssh_key: ${SSH_KEY} # SECRET (please store in environment variables) | REQUIRED | string | OS-level user account ssh key credentials in RSA PEM format ( created with ssh-keygen -t rsa -m PEM -f myuser_rsa ) ## -------- Another valid structure for tunnel_method: -------- # tunnel_method: "SSH_PASSWORD_AUTH" # REQUIRED | string | Connect through a jump server tunnel host using username and password authentication # tunnel_host: # REQUIRED | string | Hostname of the jump server host that allows inbound ssh tunnel. # tunnel_port: 22 # REQUIRED | integer | Port on the proxy/jump server that accepts inbound ssh connections. | Example: 22 # tunnel_user: # REQUIRED | string | OS-level username for logging into the jump server host - # tunnel_user_password: # SECRET | REQUIRED | string | OS-level password for logging into the jump server host + # tunnel_user_password: ${TUNNEL_USER_PASSWORD} # SECRET (please store in environment variables) | REQUIRED | string | OS-level password for logging into the jump server host diff --git a/octavia-cli/octavia_cli/apply/diff_helpers.py b/octavia-cli/octavia_cli/apply/diff_helpers.py index a5d5e2e3b8e74..1c18eeecb9d4d 100644 --- a/octavia-cli/octavia_cli/apply/diff_helpers.py +++ b/octavia-cli/octavia_cli/apply/diff_helpers.py @@ -3,6 +3,7 @@ # import hashlib +import json from typing import Any import click @@ -11,23 +12,17 @@ SECRET_MASK = "**********" -def compute_checksum(file_path: str) -> str: - """Compute SHA256 checksum from a file +def hash_config(configuration: dict) -> str: + """Computes a SHA256 hash from a dictionnary. Args: - file_path (str): Path for the file for which you want to compute a checksum. + configuration (dict): The configuration to hash Returns: - str: The computed hash digest + str: _description_ """ - BLOCK_SIZE = 65536 - file_hash = hashlib.sha256() - with open(file_path, "rb") as f: - fb = f.read(BLOCK_SIZE) - while len(fb) > 0: - file_hash.update(fb) - fb = f.read(BLOCK_SIZE) - return file_hash.hexdigest() + stringified = json.dumps(configuration, sort_keys=True) + return hashlib.sha256(stringified.encode("utf-8")).hexdigest() def exclude_secrets_from_diff(obj: Any, path: str) -> bool: diff --git a/octavia-cli/octavia_cli/apply/resources.py b/octavia-cli/octavia_cli/apply/resources.py index 4efe338b535ef..9dfad4594abe3 100644 --- a/octavia-cli/octavia_cli/apply/resources.py +++ b/octavia-cli/octavia_cli/apply/resources.py @@ -23,7 +23,8 @@ from airbyte_api_client.model.source_update import SourceUpdate from click import ClickException -from .diff_helpers import compute_checksum, compute_diff +from .diff_helpers import compute_diff, hash_config +from .yaml_loaders import EnvVarLoader class DuplicateResourceError(ClickException): @@ -39,27 +40,27 @@ class InvalidConfigurationError(ClickException): class ResourceState: - def __init__(self, configuration_path: str, resource_id: str, generation_timestamp: int, configuration_checksum: str): + def __init__(self, configuration_path: str, resource_id: str, generation_timestamp: int, configuration_hash: str): """This constructor is meant to be private. Construction shall be made with create or from_file class methods. Args: - configuration_path (str): Path to the configuration path the state relates to. + configuration_path (str): Path to the configuration this state relates to. resource_id (str): Id of the resource the state relates to. generation_timestamp (int): State generation timestamp. - configuration_checksum (str): Checksum of the configuration file. + configuration_hash (str): Checksum of the configuration file. """ self.configuration_path = configuration_path self.resource_id = resource_id self.generation_timestamp = generation_timestamp - self.configuration_checksum = configuration_checksum + self.configuration_hash = configuration_hash self.path = os.path.join(os.path.dirname(self.configuration_path), "state.yaml") def as_dict(self): return { - "configuration_path": self.configuration_path, "resource_id": self.resource_id, "generation_timestamp": self.generation_timestamp, - "configuration_checksum": self.configuration_checksum, + "configuration_path": self.configuration_path, + "configuration_hash": self.configuration_hash, } def _save(self) -> None: @@ -68,19 +69,20 @@ def _save(self) -> None: yaml.dump(self.as_dict(), state_file) @classmethod - def create(cls, configuration_path: str, resource_id: str) -> "ResourceState": + def create(cls, configuration_path: str, configuration: dict, resource_id: str) -> "ResourceState": """Create a state for a resource configuration. Args: configuration_path (str): Path to the YAML file defining the resource. + configuration (dict): Configuration object that will be hashed. resource_id (str): UUID of the resource. Returns: ResourceState: state representing the resource. """ generation_timestamp = int(time.time()) - configuration_checksum = compute_checksum(configuration_path) - state = ResourceState(configuration_path, resource_id, generation_timestamp, configuration_checksum) + configuration_hash = hash_config(configuration) + state = ResourceState(configuration_path, resource_id, generation_timestamp, configuration_hash) state._save() return state @@ -100,7 +102,7 @@ def from_file(cls, file_path: str) -> "ResourceState": raw_state["configuration_path"], raw_state["resource_id"], raw_state["generation_timestamp"], - raw_state["configuration_checksum"], + raw_state["configuration_hash"], ) @@ -194,9 +196,7 @@ def __init__( self.configuration_path = configuration_path self.api_instance = self.api(api_client) self.state = self._get_state_from_file() - self.local_file_changed = ( - True if self.state is None else compute_checksum(self.configuration_path) != self.state.configuration_checksum - ) + self.local_file_changed = True if self.state is None else hash_config(self.local_configuration) != self.state.configuration_hash @property def remote_resource(self): @@ -274,7 +274,9 @@ def get_diff_with_remote_resource(self) -> str: return diff.pretty() def _create_or_update( - self, operation_fn: Callable, payload: Union[SourceCreate, SourceUpdate, DestinationCreate, DestinationUpdate] + self, + operation_fn: Callable, + payload: Union[SourceCreate, SourceUpdate, DestinationCreate, DestinationUpdate], ) -> Union[SourceRead, DestinationRead]: """Wrapper to trigger create or update of remote resource. @@ -291,7 +293,7 @@ def _create_or_update( """ try: result = operation_fn(self.api_instance, payload) - return result, ResourceState.create(self.configuration_path, result[self.resource_id_field]) + return result, ResourceState.create(self.configuration_path, self.local_configuration, result[self.resource_id_field]) except airbyte_api_client.ApiException as api_error: if api_error.status == 422: # This API response error is really verbose, but it embodies all the details about why the config is not valid. @@ -417,10 +419,11 @@ def factory(api_client: airbyte_api_client.ApiClient, workspace_id: str, configu Union[Source, Destination]: The resource object created from the YAML config. """ with open(configuration_path, "r") as f: - local_configuration = yaml.load(f, yaml.FullLoader) + local_configuration = yaml.load(f, EnvVarLoader) if local_configuration["definition_type"] == "source": return Source(api_client, workspace_id, local_configuration, configuration_path) if local_configuration["definition_type"] == "destination": return Destination(api_client, workspace_id, local_configuration, configuration_path) + else: raise NotImplementedError(f"Resource {local_configuration['definition_type']} was not yet implemented") diff --git a/octavia-cli/octavia_cli/apply/yaml_loaders.py b/octavia-cli/octavia_cli/apply/yaml_loaders.py new file mode 100644 index 0000000000000..9e2585d54d10e --- /dev/null +++ b/octavia-cli/octavia_cli/apply/yaml_loaders.py @@ -0,0 +1,35 @@ +# +# Copyright (c) 2021 Airbyte, Inc., all rights reserved. +# + +import os +import re +from typing import Any + +import yaml + +ENV_VAR_MATCHER_PATTERN = re.compile(r".*\$\{([^}^{]+)\}.*") + + +def env_var_replacer(loader: yaml.Loader, node: yaml.Node) -> Any: + """Convert a YAML node to a Python object, expanding variable. + + Args: + loader (yaml.Loader): Not used + node (yaml.Node): Yaml node to convert to python object + + Returns: + Any: Python object with expanded vars. + """ + return os.path.expandvars(node.value) + + +class EnvVarLoader(yaml.SafeLoader): + pass + + +# All yaml nodes matching the regex will be tagged as !environment_variable. +EnvVarLoader.add_implicit_resolver("!environment_variable", ENV_VAR_MATCHER_PATTERN, None) + +# All yaml nodes tagged as !environment_variable will be constructed with the env_var_replacer callback. +EnvVarLoader.add_constructor("!environment_variable", env_var_replacer) diff --git a/octavia-cli/octavia_cli/generate/renderer.py b/octavia-cli/octavia_cli/generate/renderer.py index 324e92075eb4a..596e725e12007 100644 --- a/octavia-cli/octavia_cli/generate/renderer.py +++ b/octavia-cli/octavia_cli/generate/renderer.py @@ -89,7 +89,7 @@ def _get_type_comment(self) -> str: return self.type if self.type else None def _get_secret_comment(self) -> str: - return "SECRET" if self.airbyte_secret else None + return "SECRET (please store in environment variables)" if self.airbyte_secret else None def _get_description_comment(self) -> str: return self.description if self.description else None @@ -109,6 +109,8 @@ def _get_example_comment(self) -> str: def _get_default(self) -> str: if self.const: return self.const + if self.airbyte_secret: + return f"${{{self.name.upper()}}}" return self.default @staticmethod diff --git a/octavia-cli/octavia_cli/templates/source_or_destination.yaml.j2 b/octavia-cli/octavia_cli/templates/source_or_destination.yaml.j2 index 34c8d0501e258..3ea86d4902a1f 100644 --- a/octavia-cli/octavia_cli/templates/source_or_destination.yaml.j2 +++ b/octavia-cli/octavia_cli/templates/source_or_destination.yaml.j2 @@ -7,7 +7,7 @@ definition_image: {{ definition.docker_repository }} definition_version: {{ definition.docker_image_tag }} {%- macro render_field(field, is_commented) %} -{%- if is_commented %}# {% endif %}{{ field.name }}:{% if field.default %} {{ field.default | tojson() }}{% endif %} # {{ field.comment }} +{%- if is_commented %}# {% endif %}{{ field.name }}:{% if field.default %} {% if field.airbyte_secret %}{{ field.default }}{% else %}{{ field.default | tojson() }}{% endif %}{% endif %} # {{ field.comment }} {%- endmacro %} {%- macro render_sub_fields(sub_fields, is_commented) %} diff --git a/octavia-cli/unit_tests/test_apply/test_diff_helpers.py b/octavia-cli/unit_tests/test_apply/test_diff_helpers.py index 02e3b0c44483c..e6ca3ea9986f7 100644 --- a/octavia-cli/unit_tests/test_apply/test_diff_helpers.py +++ b/octavia-cli/unit_tests/test_apply/test_diff_helpers.py @@ -2,17 +2,13 @@ # Copyright (c) 2021 Airbyte, Inc., all rights reserved. # -from unittest.mock import mock_open, patch - import pytest from octavia_cli.apply import diff_helpers -def test_compute_checksum(mocker): - with patch("builtins.open", mock_open(read_data=b"data")) as mock_file: - digest = diff_helpers.compute_checksum("test_file_path") - assert digest == "3a6eb0790f39ac87c94f3856b2dd2c5d110e6811602261a9a923d3bb23adc8b7" - mock_file.assert_called_with("test_file_path", "rb") +def test_hash_config(): + data_to_hash = {"example": "foo"} + assert diff_helpers.hash_config(data_to_hash) == "8d621bd700ff9a864bc603f56b4ec73536110b37d814dd4629767e898da70bef" @pytest.mark.parametrize( diff --git a/octavia-cli/unit_tests/test_apply/test_resources.py b/octavia-cli/unit_tests/test_apply/test_resources.py index f4702715c9976..8b0327de1ada4 100644 --- a/octavia-cli/unit_tests/test_apply/test_resources.py +++ b/octavia-cli/unit_tests/test_apply/test_resources.py @@ -6,31 +6,31 @@ import pytest from airbyte_api_client import ApiException -from octavia_cli.apply import resources +from octavia_cli.apply import resources, yaml_loaders class TestResourceState: def test_init(self, mocker): mocker.patch.object(resources, "os") - state = resources.ResourceState("config_path", "resource_id", 123, "config_checksum") + state = resources.ResourceState("config_path", "resource_id", 123, "config_hash") assert state.configuration_path == "config_path" assert state.resource_id == "resource_id" assert state.generation_timestamp == 123 - assert state.configuration_checksum == "config_checksum" + assert state.configuration_hash == "config_hash" assert state.path == resources.os.path.join.return_value resources.os.path.dirname.assert_called_with("config_path") resources.os.path.join.assert_called_with(resources.os.path.dirname.return_value, "state.yaml") @pytest.fixture def state(self): - return resources.ResourceState("config_path", "resource_id", 123, "config_checksum") + return resources.ResourceState("config_path", "resource_id", 123, "config_hash") def test_as_dict(self, state): assert state.as_dict() == { "configuration_path": state.configuration_path, "resource_id": state.resource_id, "generation_timestamp": state.generation_timestamp, - "configuration_checksum": state.configuration_checksum, + "configuration_hash": state.configuration_hash, } def test_save(self, mocker, state): @@ -45,15 +45,15 @@ def test_save(self, mocker, state): def test_create(self, mocker): mocker.patch.object(resources.time, "time", mocker.Mock(return_value=0)) - mocker.patch.object(resources, "compute_checksum", mocker.Mock(return_value="my_checksum")) + mocker.patch.object(resources, "hash_config", mocker.Mock(return_value="my_hash")) mocker.patch.object(resources.ResourceState, "_save") - state = resources.ResourceState.create("config_path", "resource_id") + state = resources.ResourceState.create("config_path", {"my": "config"}, "resource_id") assert isinstance(state, resources.ResourceState) resources.ResourceState._save.assert_called_once() assert state.configuration_path == "config_path" assert state.resource_id == "resource_id" assert state.generation_timestamp == 0 - assert state.configuration_checksum == "my_checksum" + assert state.configuration_hash == "my_hash" def test_from_file(self, mocker): mocker.patch.object(resources, "yaml") @@ -61,7 +61,7 @@ def test_from_file(self, mocker): "configuration_path": "config_path", "resource_id": "resource_id", "generation_timestamp": 0, - "configuration_checksum": "my_checksum", + "configuration_hash": "my_hash", } with patch("builtins.open", mock_open(read_data="data")) as mock_file: state = resources.ResourceState.from_file("state.yaml") @@ -70,7 +70,7 @@ def test_from_file(self, mocker): assert state.configuration_path == "config_path" assert state.resource_id == "resource_id" assert state.generation_timestamp == 0 - assert state.configuration_checksum == "my_checksum" + assert state.configuration_hash == "my_hash" @pytest.fixture @@ -93,7 +93,7 @@ def patch_base_class(self, mocker): def test_init_no_remote_resource(self, mocker, patch_base_class, mock_api_client, local_configuration): mocker.patch.object(resources.BaseResource, "_get_state_from_file", mocker.Mock(return_value=None)) mocker.patch.object(resources.BaseResource, "_get_remote_resource", mocker.Mock(return_value=False)) - mocker.patch.object(resources, "compute_checksum") + mocker.patch.object(resources, "hash_config") resource = resources.BaseResource(mock_api_client, "workspace_id", local_configuration, "bar.yaml") assert resource.workspace_id == "workspace_id" assert resource.local_configuration == local_configuration @@ -108,11 +108,11 @@ def test_init_no_remote_resource(self, mocker, patch_base_class, mock_api_client def test_init_with_remote_resource_not_changed(self, mocker, patch_base_class, mock_api_client, local_configuration): mocker.patch.object( - resources.BaseResource, "_get_state_from_file", mocker.Mock(return_value=mocker.Mock(configuration_checksum="my_checksum")) + resources.BaseResource, "_get_state_from_file", mocker.Mock(return_value=mocker.Mock(configuration_hash="my_hash")) ) mocker.patch.object(resources.BaseResource, "_get_remote_resource", mocker.Mock(return_value={"resource_id": "my_resource_id"})) - mocker.patch.object(resources, "compute_checksum", mocker.Mock(return_value="my_checksum")) + mocker.patch.object(resources, "hash_config", mocker.Mock(return_value="my_hash")) resource = resources.BaseResource(mock_api_client, "workspace_id", local_configuration, "bar.yaml") assert resource.was_created is True assert resource.local_file_changed is False @@ -122,10 +122,10 @@ def test_init_with_remote_resource_changed(self, mocker, patch_base_class, mock_ mocker.patch.object( resources.BaseResource, "_get_state_from_file", - mocker.Mock(return_value=mocker.Mock(configuration_checksum="my_state_checksum")), + mocker.Mock(return_value=mocker.Mock(configuration_hash="my_state_hash")), ) mocker.patch.object(resources.BaseResource, "_get_remote_resource", mocker.Mock(return_value={"resource_id": "my_resource_id"})) - mocker.patch.object(resources, "compute_checksum", mocker.Mock(return_value="my_new_checksum")) + mocker.patch.object(resources, "hash_config", mocker.Mock(return_value="my_new_hash")) resource = resources.BaseResource(mock_api_client, "workspace_id", local_configuration, "bar.yaml") assert resource.was_created is True assert resource.local_file_changed is True @@ -208,7 +208,7 @@ def test_create_or_update(self, mocker, resource): result, state = resource._create_or_update(operation_fn, payload) assert result == expected_results assert state == resources.ResourceState.create.return_value - resources.ResourceState.create.assert_called_with(resource.configuration_path, "resource_id") + resources.ResourceState.create.assert_called_with(resource.configuration_path, resource.local_configuration, "resource_id") @pytest.mark.parametrize( "response_status,expected_error", @@ -315,6 +315,7 @@ def test_factory(mocker, mock_api_client, local_configuration, resource_to_mock, with patch("builtins.open", mock_open(read_data="data")) as mock_file: if not expected_error: resource = resources.factory(mock_api_client, "workspace_id", "my_config.yaml") + resources.yaml.load.assert_called_with(mock_file.return_value, yaml_loaders.EnvVarLoader) resource == getattr(resources, resource_to_mock).return_value mock_file.assert_called_with("my_config.yaml", "r") else: diff --git a/octavia-cli/unit_tests/test_apply/test_yaml_loaders.py b/octavia-cli/unit_tests/test_apply/test_yaml_loaders.py new file mode 100644 index 0000000000000..84879753b7eb7 --- /dev/null +++ b/octavia-cli/unit_tests/test_apply/test_yaml_loaders.py @@ -0,0 +1,37 @@ +# +# Copyright (c) 2021 Airbyte, Inc., all rights reserved. +# + +import os + +import pytest +import yaml +from octavia_cli.apply import yaml_loaders + + +def test_env_var_replacer(mocker): + mocker.patch.object(yaml_loaders, "os") + mock_node = mocker.Mock() + assert yaml_loaders.env_var_replacer(mocker.Mock(), mock_node) == yaml_loaders.os.path.expandvars.return_value + yaml_loaders.os.path.expandvars.assert_called_with(mock_node.value) + + +@pytest.fixture +def test_env_vars(): + old_environ = dict(os.environ) + secret_env_vars = {"MY_SECRET_PASSWORD": "🤫", "ANOTHER_SECRET_VALUE": "🔒"} + os.environ.update(secret_env_vars) + yield secret_env_vars + os.environ.clear() + os.environ.update(old_environ) + + +def test_env_var_loader(test_env_vars): + assert yaml_loaders.EnvVarLoader.yaml_implicit_resolvers[None] == [("!environment_variable", yaml_loaders.ENV_VAR_MATCHER_PATTERN)] + assert yaml_loaders.EnvVarLoader.yaml_constructors["!environment_variable"] == yaml_loaders.env_var_replacer + test_yaml = "my_secret_password: ${MY_SECRET_PASSWORD}\nanother_secret_value: ${ANOTHER_SECRET_VALUE}" + deserialized = yaml.load(test_yaml, yaml_loaders.EnvVarLoader) + assert deserialized == { + "my_secret_password": test_env_vars["MY_SECRET_PASSWORD"], + "another_secret_value": test_env_vars["ANOTHER_SECRET_VALUE"], + } diff --git a/octavia-cli/unit_tests/test_generate/test_renderer.py b/octavia-cli/unit_tests/test_generate/test_renderer.py index 902561cc6d56a..4b6631bd02fab 100644 --- a/octavia-cli/unit_tests/test_generate/test_renderer.py +++ b/octavia-cli/unit_tests/test_generate/test_renderer.py @@ -90,7 +90,7 @@ def test__get_type_comment(self): def test__get_secret_comment(self): field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) field_to_render.airbyte_secret = True - assert field_to_render._get_secret_comment() == "SECRET" + assert field_to_render._get_secret_comment() == "SECRET (please store in environment variables)" field_to_render.airbyte_secret = False assert field_to_render._get_secret_comment() is None From d828d01b4556f062233d2379d070a32cde4a7a82 Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 9 Mar 2022 17:49:04 +0100 Subject: [PATCH 2/3] fix other conflicts --- .../unit_tests/test_apply/test_resources.py | 2 +- .../unit_tests/test_generate/test_renderer.py | 220 ------------------ .../test_generate/test_renderers.py | 2 +- 3 files changed, 2 insertions(+), 222 deletions(-) delete mode 100644 octavia-cli/unit_tests/test_generate/test_renderer.py diff --git a/octavia-cli/unit_tests/test_apply/test_resources.py b/octavia-cli/unit_tests/test_apply/test_resources.py index 7613bbd96f487..59b90361a708b 100644 --- a/octavia-cli/unit_tests/test_apply/test_resources.py +++ b/octavia-cli/unit_tests/test_apply/test_resources.py @@ -493,7 +493,7 @@ def test_factory(mocker, mock_api_client, local_configuration, resource_to_mock, mocker.patch.object(resources, "yaml") if resource_to_mock is not None: mocker.patch.object(resources, resource_to_mock) - resources.yaml.safe_load.return_value = local_configuration + resources.yaml.load.return_value = local_configuration with patch("builtins.open", mock_open(read_data="data")) as mock_file: if not expected_error: resource = resources.factory(mock_api_client, "workspace_id", "my_config.yaml") diff --git a/octavia-cli/unit_tests/test_generate/test_renderer.py b/octavia-cli/unit_tests/test_generate/test_renderer.py deleted file mode 100644 index 4b6631bd02fab..0000000000000 --- a/octavia-cli/unit_tests/test_generate/test_renderer.py +++ /dev/null @@ -1,220 +0,0 @@ -# -# Copyright (c) 2021 Airbyte, Inc., all rights reserved. -# - -from unittest.mock import mock_open, patch - -import pytest -from octavia_cli.generate import renderer - - -class TestFieldToRender: - def test_init(self, mocker): - mocker.patch.object(renderer.FieldToRender, "_get_one_of_values") - mocker.patch.object(renderer, "get_object_fields") - mocker.patch.object(renderer.FieldToRender, "_get_array_items") - mocker.patch.object(renderer.FieldToRender, "_build_comment") - mocker.patch.object(renderer.FieldToRender, "_get_default") - - field_metadata = mocker.Mock() - field_to_render = renderer.FieldToRender("field_name", True, field_metadata) - assert field_to_render.name == "field_name" - assert field_to_render.required - assert field_to_render.field_metadata == field_metadata - assert field_to_render.one_of_values == field_to_render._get_one_of_values.return_value - assert field_to_render.object_properties == renderer.get_object_fields.return_value - assert field_to_render.array_items == field_to_render._get_array_items.return_value - assert field_to_render.comment == field_to_render._build_comment.return_value - assert field_to_render.default == field_to_render._get_default.return_value - field_to_render._build_comment.assert_called_with( - [ - field_to_render._get_secret_comment, - field_to_render._get_required_comment, - field_to_render._get_type_comment, - field_to_render._get_description_comment, - field_to_render._get_example_comment, - ] - ) - - def test_get_attr(self): - field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) - assert field_to_render.foo == "bar" - assert field_to_render.not_existing is None - - def test_is_array_of_objects(self): - field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) - field_to_render.type = "array" - field_to_render.items = {"type": "object"} - assert field_to_render.is_array_of_objects - field_to_render.type = "array" - field_to_render.items = {"type": "int"} - assert not field_to_render.is_array_of_objects - - def test__get_one_of_values(self, mocker): - field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) - field_to_render.oneOf = False - assert field_to_render._get_one_of_values() == [] - - mocker.patch.object(renderer, "get_object_fields") - one_of_value = mocker.Mock() - field_to_render.oneOf = [one_of_value] - one_of_values = field_to_render._get_one_of_values() - renderer.get_object_fields.assert_called_once_with(one_of_value) - assert one_of_values == [renderer.get_object_fields.return_value] - - def test__get_array_items(self, mocker): - mocker.patch.object(renderer, "parse_fields") - mocker.patch.object(renderer.FieldToRender, "is_array_of_objects", False) - - field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) - assert field_to_render._get_array_items() == [] - field_to_render.items = {"required": [], "properties": []} - mocker.patch.object(renderer.FieldToRender, "is_array_of_objects", True) - assert field_to_render._get_array_items() == renderer.parse_fields.return_value - renderer.parse_fields.assert_called_with([], []) - - def test__get_required_comment(self): - field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) - field_to_render.required = True - assert field_to_render._get_required_comment() == "REQUIRED" - field_to_render.required = False - assert field_to_render._get_required_comment() == "OPTIONAL" - - def test__get_type_comment(self): - field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) - field_to_render.type = "mytype" - assert field_to_render._get_type_comment() == "mytype" - field_to_render.type = None - assert field_to_render._get_type_comment() is None - - def test__get_secret_comment(self): - field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) - field_to_render.airbyte_secret = True - assert field_to_render._get_secret_comment() == "SECRET (please store in environment variables)" - field_to_render.airbyte_secret = False - assert field_to_render._get_secret_comment() is None - - def test__get_description_comment(self): - field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) - field_to_render.description = "foo" - assert field_to_render._get_description_comment() == "foo" - field_to_render.description = None - assert field_to_render._get_description_comment() is None - - @pytest.mark.parametrize( - "examples_value,expected_output", - [ - (["foo", "bar"], "Examples: foo, bar"), - (["foo"], "Example: foo"), - ("foo", "Example: foo"), - ([5432], "Example: 5432"), - (None, None), - ], - ) - def test__get_example_comment(self, examples_value, expected_output): - field_to_render = renderer.FieldToRender("field_name", True, {"foo": "bar"}) - field_to_render.examples = examples_value - assert field_to_render._get_example_comment() == expected_output - - @pytest.mark.parametrize( - "field_metadata,expected_default", - [ - ({"const": "foo", "default": "bar"}, "foo"), - ({"default": "bar"}, "bar"), - ({}, None), - ], - ) - def test__get_default(self, field_metadata, expected_default): - field_to_render = renderer.FieldToRender("field_name", True, field_metadata) - assert field_to_render.default == expected_default - - def test__build_comment(self, mocker): - comment_functions = [mocker.Mock(return_value="foo"), mocker.Mock(return_value=None), mocker.Mock(return_value="bar")] - comment = renderer.FieldToRender._build_comment(comment_functions) - assert comment == "foo | bar" - - -def test_parse_fields(): - required_fields = ["foo"] - properties = {"foo": {}, "bar": {}} - fields_to_render = renderer.parse_fields(required_fields, properties) - assert fields_to_render[0].name == "foo" - assert fields_to_render[0].required - assert fields_to_render[1].name == "bar" - assert not fields_to_render[1].required - - -def test_get_object_fields(mocker): - mocker.patch.object(renderer, "parse_fields") - field_metadata = {"properties": {"foo": {}, "bar": {}}, "required": ["foo"]} - object_properties = renderer.get_object_fields(field_metadata) - assert object_properties == renderer.parse_fields.return_value - renderer.parse_fields.assert_called_with(["foo"], field_metadata["properties"]) - field_metadata = {} - assert renderer.get_object_fields(field_metadata) == [] - - -class TestConnectionSpecificationRenderer: - def test_init(self, mocker): - assert renderer.ConnectionSpecificationRenderer.TEMPLATE == renderer.JINJA_ENV.get_template("source_or_destination.yaml.j2") - definition = mocker.Mock() - spec_renderer = renderer.ConnectionSpecificationRenderer("my_resource_name", definition) - assert spec_renderer.resource_name == "my_resource_name" - assert spec_renderer.definition == definition - - def test__parse_connection_specification(self, mocker): - mocker.patch.object(renderer, "parse_fields") - schema = {"required": ["foo"], "properties": {"foo": "bar"}} - definition = mocker.Mock() - spec_renderer = renderer.ConnectionSpecificationRenderer("my_resource_name", definition) - parsed_schema = spec_renderer._parse_connection_specification(schema) - assert renderer.parse_fields.call_count == 1 - assert parsed_schema[0], renderer.parse_fields.return_value - renderer.parse_fields.assert_called_with(["foo"], {"foo": "bar"}) - - def test__parse_connection_specification_one_of(self, mocker): - mocker.patch.object(renderer, "parse_fields") - schema = {"oneOf": [{"required": ["foo"], "properties": {"foo": "bar"}}, {"required": ["free"], "properties": {"free": "beer"}}]} - spec_renderer = renderer.ConnectionSpecificationRenderer("my_resource_name", mocker.Mock()) - parsed_schema = spec_renderer._parse_connection_specification(schema) - assert renderer.parse_fields.call_count == 2 - assert parsed_schema[0], renderer.parse_fields.return_value - assert parsed_schema[1], renderer.parse_fields.return_value - assert len(parsed_schema) == len(schema["oneOf"]) - renderer.parse_fields.assert_called_with(["free"], {"free": "beer"}) - - def test__get_output_path(self, mocker): - mocker.patch.object(renderer, "os") - renderer.os.path.exists.return_value = False - spec_renderer = renderer.ConnectionSpecificationRenderer("my_resource_name", mocker.Mock(type="source")) - renderer.os.path.join.side_effect = ["./source/my_resource_name", "./source/my_resource_name/configuration.yaml"] - output_path = spec_renderer._get_output_path(".") - renderer.os.makedirs.assert_called_once() - renderer.os.path.join.assert_has_calls( - [ - mocker.call(".", "sources", "my_resource_name"), - mocker.call("./source/my_resource_name", "configuration.yaml"), - ] - ) - assert output_path == "./source/my_resource_name/configuration.yaml" - - def test_write_yaml(self, mocker): - - mocker.patch.object(renderer.ConnectionSpecificationRenderer, "_get_output_path") - mocker.patch.object(renderer.ConnectionSpecificationRenderer, "_parse_connection_specification") - mocker.patch.object( - renderer.ConnectionSpecificationRenderer, "TEMPLATE", mocker.Mock(render=mocker.Mock(return_value="rendered_string")) - ) - - spec_renderer = renderer.ConnectionSpecificationRenderer("my_resource_name", mocker.Mock(type="source")) - with patch("builtins.open", mock_open()) as mock_file: - output_path = spec_renderer.write_yaml(".") - assert output_path == spec_renderer._get_output_path.return_value - spec_renderer.TEMPLATE.render.assert_called_with( - { - "resource_name": "my_resource_name", - "definition": spec_renderer.definition, - "configuration_fields": spec_renderer._parse_connection_specification.return_value, - } - ) - mock_file.assert_called_with(output_path, "w") diff --git a/octavia-cli/unit_tests/test_generate/test_renderers.py b/octavia-cli/unit_tests/test_generate/test_renderers.py index bb434791b943a..6e46afda75251 100644 --- a/octavia-cli/unit_tests/test_generate/test_renderers.py +++ b/octavia-cli/unit_tests/test_generate/test_renderers.py @@ -90,7 +90,7 @@ def test__get_type_comment(self): def test__get_secret_comment(self): field_to_render = renderers.FieldToRender("field_name", True, {"foo": "bar"}) field_to_render.airbyte_secret = True - assert field_to_render._get_secret_comment() == "SECRET" + assert field_to_render._get_secret_comment() == "SECRET (please store in environment variables)" field_to_render.airbyte_secret = False assert field_to_render._get_secret_comment() is None From e58f53559a51f65ad8af3b7794d5a4ba9d21e83e Mon Sep 17 00:00:00 2001 From: alafanechere Date: Wed, 9 Mar 2022 20:24:11 +0100 Subject: [PATCH 3/3] fix brutal search and replace error --- .../expected_rendered_yaml/destination_s3/expected.yaml | 2 +- .../expected_rendered_yaml/destination_s3/input_spec.yaml | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/expected.yaml b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/expected.yaml index 0d2d835c3d522..6d43275fef7c8 100644 --- a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/expected.yaml +++ b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/expected.yaml @@ -31,7 +31,7 @@ configuration: ## -------- Another valid structure for compression_codec: -------- # codec: "zstandard" # REQUIRED | string # compression_level: 3 # REQUIRED | integer | Negative levels are 'fast' modes akin to lz4 or snappy, levels above 9 are generally for archival purposes, and levels above 18 use a lot of memory. - # include_hash: # OPTIONAL | boolean | If true, include a hash with each data block. + # include_checksum: # OPTIONAL | boolean | If true, include a checksum with each data block. ## -------- Another valid structure for compression_codec: -------- # codec: "snappy" # REQUIRED | string part_size_mb: 5 # OPTIONAL | integer | This is the size of a "Part" being buffered in memory. It limits the memory usage when writing. Larger values will allow to upload a bigger files and improve the speed, but consumes9 more memory. Allowed values: min=5MB, max=525MB Default: 5MB. | Example: 5 diff --git a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/input_spec.yaml b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/input_spec.yaml index ed2b974503a5c..2d332c60efd8f 100644 --- a/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/input_spec.yaml +++ b/octavia-cli/integration_tests/test_generate/expected_rendered_yaml/destination_s3/input_spec.yaml @@ -180,9 +180,9 @@ spec: default: 3 minimum: -5 maximum: 22 - include_hash: - title: "Include hash" - description: "If true, include a hash with each data block." + include_checksum: + title: "Include checksum" + description: "If true, include a checksum with each data block." type: "boolean" default: false - title: "snappy"