Merge pull request #410 from noqdev/bug/en-2119-fix-merge-model-int-h…

…andling

Fix merge model int handling

.github/ISSUE_TEMPLATE/bug_report.md

@@ -12,6 +12,7 @@ A clear and concise description of what the bug is.
 
 **To Reproduce**
 Steps to reproduce the behavior:
+
 1. Go to '...'
 2. Click on '....'
 3. Scroll down to '....'
@@ -24,9 +25,15 @@ A clear and concise description of what you expected to happen.
 If applicable, add screenshots to help explain your problem.
 
 **Desktop (please complete the following information):**
- - OS: [e.g. iOS]
- - Browser [e.g. chrome, safari]
- - Version [e.g. 22]
+
+- OS: [e.g. iOS]
+- Browser [e.g. chrome, safari]
+- Version [e.g. 22]
 
 **Additional context**
 Add any other context about the problem here.
+
+**Community Engagement**
+Your vote counts! Please support this bug report by adding a 👍 reaction to the original issue, which will aid the community and maintainers in addressing this problem.
+
+Please refrain from adding "+1" or "me too" comments, as these create unnecessary noise for issue followers and do not help in prioritizing the issue. If you wish to contribute to solving this issue or have submitted a pull request, please leave a comment.

.github/ISSUE_TEMPLATE/feature_request.md

@@ -18,3 +18,8 @@ A clear and concise description of any alternative solutions or features you've
 
 **Additional context**
 Add any other context or screenshots about the feature request here.
+
+**Community Engagement**
+Help us prioritize this request and express your support by adding a 👍 reaction to the original issue. This will assist both the community and the maintainers in addressing this request.
+
+Please avoid leaving "+1" or "me too" comments as they create extra noise for issue followers and do not assist in prioritizing the request. If you are considering working on this issue or have already submitted a pull request, kindly leave a comment.

.github/workflows/run-test.yml

@@ -31,7 +31,7 @@ jobs:
         id: run-test
         run: |
           python3 -m venv env
-          . env/bin/activate && pip install poetry setuptools pip --upgrade && poetry install && make test
+          . env/bin/activate && pip install poetry setuptools pip --upgrade && poetry install && pre-commit run -a && make test
       - name: Upload coverage reports to Codecov
         if: ${{ github.repository == 'noqdev/iambic' }}
         uses: codecov/codecov-action@v3

.pre-commit-config.yaml

@@ -3,24 +3,21 @@ default_language_version:
     python: python3.10
 repos:
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.1.0 # Use the ref you want to point at
+  rev: v4.1.0
   hooks:
     - id: trailing-whitespace
     - id: check-ast
     - id: check-case-conflict
     - id: debug-statements
     - id: check-yaml
       args: [--allow-multiple-documents, --unsafe]
+- repo: https://github.com/timothycrosley/isort
+  rev: "5.12.0"
+  hooks:
+    - id: isort
+      pass_filenames: true
 - repo: local
   hooks:
-  - id: isort
-    name: isort
-    stages: [commit]
-    language: system
-    entry: poetry run isort
-    types: [python]
-    args: ["--profile", "black", "-p", "iambic"]
-
   - id: black
     name: black
     stages: [commit]

CHANGELOG.md

@@ -1,6 +1,21 @@
 
 # Change Log
 
+## 0.7.6 (May 15th, 2023)
+
+BUG FIXES:
+
+* Handling merge models when the new value is an int
+* Flatten multiline comment when they are not attached to a YAML dict key
+
+ENHANCEMENTS:
+* Added an `iambic convert` command to convert an AWS policy to the IAMbic formatted yaml
+
+THANKS:
+
+* `Shreyas D` for reporting the merge model issue
+* `Phil H, Michael W` for suggesting the `iambic convert` command
+
 ## 0.7.3 (May 10th, 2023)
 
 BUG FIXES:

functional_tests/aws/group/test_update_template.py

@@ -4,9 +4,9 @@
 from unittest import IsolatedAsyncioTestCase
 
 import dateparser
+
 from functional_tests.aws.group.utils import generate_group_template_from_base
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.output.text import screen_render_resource_changes
 from iambic.plugins.v0_1_0.aws.iam.group.models import AwsIamGroupTemplate
 from iambic.plugins.v0_1_0.aws.iam.group.utils import get_group_across_accounts

functional_tests/aws/managed_policy/test_update_template.py

@@ -7,7 +7,6 @@
     generate_managed_policy_template_from_base,
 )
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.core import noq_json as json
 from iambic.output.text import screen_render_resource_changes
 from iambic.plugins.v0_1_0.aws.models import Tag

functional_tests/aws/managed_policy/utils.py

@@ -4,7 +4,6 @@
 import uuid
 
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.core.iambic_enum import Command
 from iambic.core.logger import log
 from iambic.core.models import ExecutionMessage

functional_tests/aws/permission_set/test_create_template.py

@@ -8,7 +8,6 @@
     generate_permission_set_template_from_base,
 )
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.output.text import screen_render_resource_changes
 
 

functional_tests/aws/role/test_template_generation.py

@@ -8,7 +8,6 @@
     role_full_import,
 )
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.plugins.v0_1_0.aws.event_bridge.models import RoleMessageDetails
 from iambic.plugins.v0_1_0.aws.iam.role.models import AwsIamRoleTemplate
 

functional_tests/aws/user/test_create_template.py

@@ -4,7 +4,6 @@
 
 from functional_tests.aws.user.utils import generate_user_template_from_base
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.plugins.v0_1_0.aws.iam.user.utils import get_user_across_accounts
 
 

functional_tests/azure_ad/base_test_case.py

@@ -5,7 +5,6 @@
 from unittest import IsolatedAsyncioTestCase
 
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.plugins.v0_1_0.azure_ad.group.models import GroupTemplateProperties
 from iambic.plugins.v0_1_0.azure_ad.group.utils import list_groups
 from iambic.plugins.v0_1_0.azure_ad.user.models import UserTemplateProperties

functional_tests/azure_ad/group/utils.py

@@ -5,7 +5,6 @@
 import uuid
 
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.core.iambic_enum import Command
 from iambic.core.models import ExecutionMessage
 from iambic.plugins.v0_1_0.azure_ad.group.models import (

functional_tests/azure_ad/user/utils.py

@@ -4,7 +4,6 @@
 import random
 
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.plugins.v0_1_0.azure_ad.user.models import AzureActiveDirectoryUserTemplate
 
 

functional_tests/okta/group/test_okta_group.py

@@ -4,7 +4,6 @@
 import os
 
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.core.iambic_enum import IambicManaged
 from iambic.core.parser import load_templates
 from iambic.main import run_apply

functional_tests/test_config_discovery.py

@@ -4,7 +4,6 @@
 from unittest import IsolatedAsyncioTestCase
 
 from functional_tests.conftest import IAMBIC_TEST_DETAILS
-
 from iambic.config.dynamic_config import load_config
 from iambic.core.iambic_enum import Command
 from iambic.core.models import ExecutionMessage

functional_tests/test_wizard.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import os
 import random
 import tempfile

iambic/core/template_generation.py

@@ -11,6 +11,7 @@
 from iambic.core.models import AccessModelMixin, BaseModel, BaseTemplate, ProviderChild
 from iambic.core.parser import load_templates
 from iambic.core.utils import (
+    IAMBIC_ERR_MSG,
     evaluate_on_provider,
     gather_templates,
     get_provider_value,
@@ -898,7 +899,7 @@ def merge_model_list(
     return merged_list
 
 
-def merge_model(
+def merge_model(  # noqa: C901
     new_model: BaseModel,
     existing_model: BaseModel,
     all_provider_children: list[ProviderChild],
@@ -990,16 +991,55 @@ def merge_model(
                     elif not isinstance(new_value, list):
                         new_value = [new_value]
 
-                    new_value = merge_access_model_list(
-                        new_value, existing_value, all_provider_children
-                    )
+                    if all(isinstance(x, AccessModelMixin) for x in new_value):
+                        try:
+                            new_value = merge_access_model_list(
+                                new_value, existing_value, all_provider_children
+                            )
+                        except Exception as err:
+                            log.exception(
+                                f"Failed to merge template attribute. {IAMBIC_ERR_MSG}",
+                                key=key,
+                                new_value=[
+                                    {"value": str(val), "type": type(val)}
+                                    for val in new_value
+                                ],
+                                existing_value=[
+                                    {"value": str(val), "type": type(val)}
+                                    for val in existing_value
+                                ],
+                                err=repr(err),
+                            )
+                            raise
+
                     setattr(
                         merged_model, key, new_value if value_as_list else new_value[0]
                     )
                 elif isinstance(inner_element, BaseModel):
-                    new_value = merge_model_list(
-                        new_value, existing_value, all_provider_children
-                    )
+                    if not isinstance(new_value, list):
+                        new_value = [new_value]
+
+                    if all(isinstance(x, BaseModel) for x in new_value):
+                        try:
+                            new_value = merge_model_list(
+                                new_value, existing_value, all_provider_children
+                            )
+                        except Exception as err:
+                            log.exception(
+                                f"Failed to merge template attribute. {IAMBIC_ERR_MSG}",
+                                key=key,
+                                new_value=[
+                                    {"value": str(val), "type": type(val)}
+                                    for val in new_value
+                                ],
+                                existing_value=[
+                                    {"value": str(val), "type": type(val)}
+                                    for val in existing_value
+                                ],
+                                err=repr(err),
+                            )
+                            raise
+
                     setattr(
                         merged_model, key, new_value if value_as_list else new_value[0]
                     )
@@ -1030,7 +1070,7 @@ def merge_model(
                 setattr(merged_model, key, new_value)
             else:
                 raise TypeError(
-                    f"Type of {type(new_value)} is not supported. Please file a github issue"
+                    f"Type of {type(new_value)} is not supported. {IAMBIC_ERR_MSG}"
                 )
         elif key not in iambic_fields:
             setattr(merged_model, key, new_value)

iambic/core/utils.py

@@ -30,6 +30,11 @@
 
 NOQ_TEMPLATE_REGEX = r".*template_type:\n?.*NOQ::"
 RATE_LIMIT_STORAGE: dict[str, int] = {}
+IAMBIC_ERR_MSG = (
+    "Please file a github issue or message us on the slack community channel. "
+    "Include as much of the traceback and this error message as possible. "
+    "Be sure to redact any sensitive information."
+)
 
 __WRITABLE_DIRECTORY__ = pathlib.Path.home()
 

iambic/plugins/v0_1_0/aws/pyproject.toml

@@ -11,8 +11,13 @@ readme = "README.md"
 exclude = ["build_util/*"]
 
 [tool.isort]
+known_first_party = ["iambic"]
+src_paths = ["iambic","test","functional_tests","build_utils"]
+known_third_party = ["_pytest", "okta", "github", "pydantic", "rich"]
 profile = "black"
-add_imports = "from __future__ import annotations"
+add_imports = ["from __future__ import annotations"]
+skip_gitignore = true
+line_length = 88
 
 [tool.poetry.dependencies]
 python = "^3.10"

iambic/plugins/v0_1_0/aws/template_generation.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from typing import Union
 
 from iambic.core.template_generation import (

iambic/plugins/v0_1_0/google_workspace/pyproject.toml

@@ -11,8 +11,13 @@ readme = "README.md"
 exclude = ["build_util/*"]
 
 [tool.isort]
+known_first_party = ["iambic"]
+src_paths = ["iambic","test","functional_tests","build_utils"]
+known_third_party = ["_pytest", "okta", "github", "pydantic", "rich"]
 profile = "black"
-add_imports = "from __future__ import annotations"
+add_imports = ["from __future__ import annotations"]
+skip_gitignore = true
+line_length = 88
 
 [tool.poetry.dependencies]
 python = "^3.10"

iambic/plugins/v0_1_0/okta/pyproject.toml

@@ -11,8 +11,13 @@ readme = "README.md"
 exclude = ["build_util/*"]
 
 [tool.isort]
+known_first_party = ["iambic"]
+src_paths = ["iambic","test","functional_tests","build_utils"]
+known_third_party = ["_pytest", "okta", "github", "pydantic", "rich"]
 profile = "black"
-add_imports = "from __future__ import annotations"
+add_imports = ["from __future__ import annotations"]
+skip_gitignore = true
+line_length = 88
 
 [tool.poetry.dependencies]
 python = "^3.10"

iambic/vendor/lambda_multiprocessing/__init__.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from iambic.vendor.lambda_multiprocessing.main import AsyncResult, Pool, TimeoutError
 
 __all__ = ["Pool", "TimeoutError", "AsyncResult"]