Skip to content

Commit

Permalink
[Integration][AWS] Fixed JSON key issues caused by custom properties …
Browse files Browse the repository at this point in the history
…enum (#1277)

# Description

What - This PR addresses an issue where CustomProperties (a StrEnum) was
being used as dictionary keys directly, causing invalid JSON/dictionary
serialization by including both the enum key and value in the output.
The fix ensures that only the StrEnum.value is used for dictionary keys.

Why - The existing implementation leads to serialization issues,
creating invalid JSON or dictionaries that cannot be processed
downstream

How - Updated all instances where CustomProperties is used as dictionary
keys to explicitly use .value for their string representation

## Type of change

Please leave one option from the following and delete the rest:

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] New Integration (non-breaking change which adds a new integration)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Non-breaking change (fix of existing functionality that will not
change current behavior)
- [ ] Documentation (added/updated documentation)

<h4> All tests should be run against the port production
environment(using a testing org). </h4>

### Core testing checklist

- [ ] Integration able to create all default resources from scratch
- [ ] Resync finishes successfully
- [ ] Resync able to create entities
- [ ] Resync able to update entities
- [ ] Resync able to detect and delete entities
- [ ] Scheduled resync able to abort existing resync and start a new one
- [ ] Tested with at least 2 integrations from scratch
- [ ] Tested with Kafka and Polling event listeners
- [ ] Tested deletion of entities that don't pass the selector


### Integration testing checklist

- [ ] Integration able to create all default resources from scratch
- [ ] Resync able to create entities
- [ ] Resync able to update entities
- [ ] Resync able to detect and delete entities
- [ ] Resync finishes successfully
- [ ] If new resource kind is added or updated in the integration, add
example raw data, mapping and expected result to the `examples` folder
in the integration directory.
- [ ] If resource kind is updated, run the integration with the example
data and check if the expected result is achieved
- [ ] If new resource kind is added or updated, validate that
live-events for that resource are working as expected
- [ ] Docs PR link [here](#)

### Preflight checklist

- [ ] Handled rate limiting
- [ ] Handled pagination
- [ ] Implemented the code in async
- [ ] Support Multi account

## Screenshots

Include screenshots from your environment showing how the resources of
the integration will look.

## API Documentation

Provide links to the API documentation used for this integration.
  • Loading branch information
oiadebayo authored Jan 9, 2025
1 parent 5473284 commit aafb26c
Show file tree
Hide file tree
Showing 5 changed files with 271 additions and 7 deletions.
8 changes: 8 additions & 0 deletions integrations/aws/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

<!-- towncrier release notes start -->

## 0.2.81 (2025-01-08)


### Bug Fixes

- Updated the serialized response to include valid custom property json key by accessing the StrEnum value properly.


## 0.2.80 (2025-01-08)


Expand Down
2 changes: 1 addition & 1 deletion integrations/aws/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "aws"
version = "0.2.80"
version = "0.2.81"
description = "This integration will map all your resources in all the available accounts to your Port entities"
authors = ["Shalev Avhar <[email protected]>", "Erik Zaadi <[email protected]>"]

Expand Down
179 changes: 179 additions & 0 deletions integrations/aws/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import pytest
from unittest.mock import AsyncMock, MagicMock, patch
import json
from contextlib import asynccontextmanager
from typing import Any, AsyncGenerator, Dict, Generator

from port_ocean.context.ocean import initialize_port_ocean_context
from port_ocean.context.event import EventContext
from port_ocean.exceptions.context import PortOceanContextAlreadyInitializedError
from aws.session_manager import SessionManager

MOCK_ORG_URL: str = "https://mock-organization-url.com"
MOCK_PERSONAL_ACCESS_TOKEN: str = "mock-personal_access_token"


@pytest.fixture(autouse=True)
def mock_ocean_context() -> None:
"""Mock the PortOcean context to prevent initialization errors."""
try:
mock_ocean_app: MagicMock = MagicMock()
mock_ocean_app.config.integration.config = {
"organization_url": MOCK_ORG_URL,
"personal_access_token": MOCK_PERSONAL_ACCESS_TOKEN,
}
mock_ocean_app.integration_router = MagicMock()
mock_ocean_app.port_client = MagicMock()
initialize_port_ocean_context(mock_ocean_app)
except PortOceanContextAlreadyInitializedError:
pass


@pytest.fixture
def mock_event_context() -> Generator[MagicMock, None, None]:
"""Mock the event context."""
mock_event: MagicMock = MagicMock(spec=EventContext)

with patch("port_ocean.context.event.event_context", mock_event):
yield mock_event


@pytest.fixture
def mock_session() -> AsyncMock:
"""Creates a mocked session with a client factory and credentials."""
mock_session: AsyncMock = AsyncMock()
mock_session.region_name = "us-west-2"

@asynccontextmanager
async def mock_client(
service_name: str, **kwargs: Any
) -> AsyncGenerator[Any, None]:
if service_name == "cloudformation":

class MockCloudFormationClient:
async def describe_method(self, **kwargs: Any) -> Dict[str, Any]:
return {
"NextToken": None,
"ResourceList": [
{
"Properties": {"Name": "test-resource"},
"Identifier": "test-id",
}
],
}

yield MockCloudFormationClient()
elif service_name == "cloudcontrol":

class MockCloudControlClient:
async def list_resources(self, **kwargs: Any) -> Dict[str, Any]:
return {
"NextToken": None,
"ResourceDescriptions": [
{
"Properties": json.dumps({"Name": "test-resource"}),
"Identifier": "test-id",
}
],
}

yield MockCloudControlClient()

else:
raise NotImplementedError(f"Client for service '{service_name}' not mocked")

# Provide a mock for get_credentials
class MockFrozenCredentials:
access_key: str = "mock_access_key"
secret_key: str = "mock_secret_key"
token: str = "mock_session_token"

class MockCredentials:
async def get_frozen_credentials(self) -> MockFrozenCredentials:
return MockFrozenCredentials()

mock_session.get_credentials.return_value = MockCredentials()
mock_session.client = mock_client
return mock_session


@pytest.fixture
def mock_account_id() -> str:
"""Mocks the account ID."""
return "123456789012"


@pytest.fixture
def mock_resource_config() -> MagicMock:
"""Mocks the resource config."""
mock_resource_config: MagicMock = MagicMock()
mock_resource_config.selector.is_region_allowed.return_value = True
return mock_resource_config


@pytest.fixture(autouse=True)
def mock_application_creds_patch() -> Generator[None, None, None]:
"""
Patch SessionManager._get_application_credentials and
SessionManager._update_available_access_credentials with side_effect
to prevent actual calls.
"""

def mock_get_application_credentials() -> "MockApplicationCredentials":
return MockApplicationCredentials()

def mock_update_available_access_credentials() -> None:
pass

with (
patch.object(
SessionManager,
"_get_application_credentials",
side_effect=mock_get_application_credentials,
),
patch.object(
SessionManager,
"_update_available_access_credentials",
side_effect=mock_update_available_access_credentials,
),
):

class MockAioboto3Session:
"""A fake session object that supports async with for .client(...) calls."""

def __init__(self, region_name: str = "us-west-2"):
self.region_name: str = region_name

@asynccontextmanager
async def client(
self, service_name: str, **kwargs: Any
) -> AsyncGenerator[AsyncMock, None]:
if service_name == "sts":
mock_client: AsyncMock = AsyncMock()
mock_client.get_caller_identity.return_value = {
"Account": "123456789012"
}
yield mock_client
else:
yield AsyncMock()

class MockApplicationCredentials:
"""Simulates the object that your SessionManager code expects."""

def __init__(self, *args: Any, **kwargs: Any):
self.aws_access_key_id: str = "mock_access_key_id"
self.aws_secret_access_key: str = "mock_secret_access_key"
self.region_name: str = "us-west-2"
self.account_id: str = "123456789012"

async def create_session(
self, *args: Any, **kwargs: Any
) -> MockAioboto3Session:
"""
Return an object that looks like an aioboto3.Session,
i.e. has .client(...) that returns an async context manager
for services like 'sts' and 'organizations'.
"""
return MockAioboto3Session()

yield
77 changes: 77 additions & 0 deletions integrations/aws/tests/utils/test_resources.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import pytest
from unittest.mock import AsyncMock, MagicMock, patch
from typing import Any, Dict, List
from utils.misc import CustomProperties
from utils.resources import (
resync_custom_kind,
resync_cloudcontrol,
)


@pytest.mark.asyncio
async def test_resync_custom_kind(
mock_session: AsyncMock,
mock_account_id: str,
mock_resource_config: MagicMock,
) -> None:
"""Test that resync_custom_kind produces valid output."""
with patch(
"utils.resources._session_manager.find_account_id_by_session",
return_value=mock_account_id,
):
async for result in resync_custom_kind(
kind="AWS::CloudFormation::Stack",
session=mock_session,
service_name="cloudformation",
describe_method="describe_method",
list_param="ResourceList",
marker_param="NextToken",
resource_config=mock_resource_config,
):
assert isinstance(result, list)
for resource in result:
assert (
resource[CustomProperties.KIND.value]
== "AWS::CloudFormation::Stack"
)
assert resource[CustomProperties.ACCOUNT_ID.value] == mock_account_id
assert resource[CustomProperties.REGION.value] == "us-west-2"
assert "Properties" in resource


@pytest.mark.asyncio
async def test_resync_cloudcontrol(
mock_session: AsyncMock,
mock_account_id: str,
mock_resource_config: MagicMock,
mock_event_context: MagicMock,
) -> None:
"""Test that resync_cloudcontrol produces valid output."""

async def mock_gather(*args: Any, **kwargs: Any) -> List[Dict[str, Any]]:
return [
{
"Identifier": "test-id",
"Properties": {"Name": "mocked-resource"},
"AdditionalInfo": "mocked-info",
}
]

with (
patch("utils.resources.asyncio.gather", return_value=mock_gather()),
patch(
"utils.resources._session_manager.find_account_id_by_session",
return_value=mock_account_id,
),
):
async for result in resync_cloudcontrol(
kind="AWS::S3::Bucket",
session=mock_session,
resource_config=mock_resource_config,
):
assert isinstance(result, list)
for resource in result:
assert resource[CustomProperties.KIND.value] == "AWS::S3::Bucket"
assert resource[CustomProperties.ACCOUNT_ID.value] == mock_account_id
assert resource[CustomProperties.REGION.value] == "us-west-2"
assert "Properties" in resource
12 changes: 6 additions & 6 deletions integrations/aws/utils/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ async def resync_custom_kind(
if results:
yield [
{
CustomProperties.KIND: kind,
CustomProperties.ACCOUNT_ID: account_id,
CustomProperties.REGION: region,
CustomProperties.KIND.value: kind,
CustomProperties.ACCOUNT_ID.value: account_id,
CustomProperties.REGION.value: region,
**fix_unserializable_date_properties(resource),
}
for resource in results
Expand Down Expand Up @@ -239,9 +239,9 @@ async def resync_cloudcontrol(
serialized = instance.copy()
serialized.update(
{
CustomProperties.KIND: kind,
CustomProperties.ACCOUNT_ID: account_id,
CustomProperties.REGION: region,
CustomProperties.KIND.value: kind,
CustomProperties.ACCOUNT_ID.value: account_id,
CustomProperties.REGION.value: region,
}
)
page_resources.append(
Expand Down

0 comments on commit aafb26c

Please sign in to comment.