Repository: getmoto/moto
Pull request #8149 (2024-09-24 01:00:27)

[FSx] Update file system id generation to expected regex
This change intends to address https://github.com/getmoto/moto/issues/8148

Modifies how fsx fs id is generated so that:
* there are not collisions for requests that happen in parallel
* it complies with the expected regex.


Patch:
diff --git a/moto/ec2/models/tags.py b/moto/ec2/models/tags.py
index fdf9a1205..82b70d1e6 100644
--- a/moto/ec2/models/tags.py
+++ b/moto/ec2/models/tags.py
@@ -26,7 +26,14 @@ class TagBackend:
             if resource_id in self.tags:
                 if (
                     len(self.tags[resource_id])
-                    + len([tag for tag in tags if not tag.startswith("aws:")])
+                    + len(
+                        [
+                            tag
+                            for tag in tags
+                            if not tag.startswith("aws:")
+                            and tag not in self.tags[resource_id]
+                        ]
+                    )
                     > 50
                 ):
                     raise TagLimitExceeded()
diff --git a/moto/firehose/models.py b/moto/firehose/models.py
index 45165953b..ffc7ac989 100644
--- a/moto/firehose/models.py
+++ b/moto/firehose/models.py
@@ -517,8 +517,8 @@ class FirehoseBackend(BaseBackend):
                 request_responses = self.put_http_records(
                     destination["HttpEndpoint"], records
                 )
-            elif "Elasticsearch" in destination or "Redshift" in destination:
-                # This isn't implmented as these services aren't implemented,
+            elif {"Elasticsearch", "Redshift", "Snowflake"} & set(destination):
+                # This isn't implemented as these services aren't implemented,
                 # so ignore the data, but return a "proper" response.
                 request_responses = [
                     {"RecordId": str(mock_random.uuid4())} for _ in range(len(records))
diff --git a/moto/fsx/models.py b/moto/fsx/models.py
index 554974ec5..abceebbd1 100644
--- a/moto/fsx/models.py
+++ b/moto/fsx/models.py
@@ -1,7 +1,7 @@
 """FSxBackend class with methods for supported APIs."""
 
-from datetime import datetime
 from typing import Any, Dict, List, Optional, Tuple
+from uuid import uuid4
 
 from moto.core.base_backend import BackendDict, BaseBackend
 from moto.core.common_models import BaseModel
@@ -36,7 +36,7 @@ class FileSystem(BaseModel):
         ontap_configuration: Optional[Dict[str, Any]],
         open_zfs_configuration: Optional[Dict[str, Any]],
     ) -> None:
-        self.file_system_id = f"fs-moto{datetime.now().strftime('%Y%m%d%H%M%S')}"
+        self.file_system_id = f"fs-{uuid4().hex[:8]}"
         self.file_system_type = file_system_type
         if self.file_system_type not in FileSystemType.list_values():
             raise ValueError(f"Invalid FileSystemType: {self.file_system_type}")
diff --git a/moto/rds/models.py b/moto/rds/models.py
index a03630690..21c5f083a 100644
--- a/moto/rds/models.py
+++ b/moto/rds/models.py
@@ -2904,26 +2904,36 @@ class OptionGroup(RDSBaseModel):
           <EngineName>{{ option_group.engine_name }}</EngineName>
           <OptionGroupDescription>{{ option_group.description }}</OptionGroupDescription>
           <OptionGroupArn>{{ option_group.arn }}</OptionGroupArn>
-          <Options/>
+          <Options>
+              {% for name, option_settings in option_group.options.items() %}
+              <Option>
+                <OptionName>{{ name }}</OptionName>
+                <OptionSettings>
+                  {% for setting in option_settings %}
+                  <OptionSetting>
+                    <Name>{{ setting.get("Name") }}</Name>
+                    <Value>{{ setting.get("Value") }}</Value>
+                  </OptionSetting>
+                  {% endfor %}
+                </OptionSettings>
+              </Option>
+              {% endfor %}
+          </Options>
         </OptionGroup>"""
         )
         return template.render(option_group=self)
 
-    def remove_options(
-        self,
-        options_to_remove: Any,  # pylint: disable=unused-argument
-    ) -> None:
-        # TODO: Check for option in self.options and remove if exists. Raise
-        # error otherwise
-        return
-
-    def add_options(
-        self,
-        options_to_add: Any,  # pylint: disable=unused-argument
-    ) -> None:
-        # TODO: Validate option and add it to self.options. If invalid raise
-        # error
-        return
+    def remove_options(self, options_to_remove: Any) -> None:
+        for option in options_to_remove:
+            if isinstance(option, str):
+                self.options.pop(option, None)
+
+    def add_options(self, options_to_add: Any) -> None:
+        for option in options_to_add:
+            if isinstance(option, str):
+                self.options[option] = {}
+            elif isinstance(option, dict):
+                self.options[option["OptionName"]] = option["OptionSettings"]
 
 
 class DBParameterGroup(CloudFormationModel, RDSBaseModel):
diff --git a/moto/rds/responses.py b/moto/rds/responses.py
index 53a7bcbf9..8be586ff0 100644
--- a/moto/rds/responses.py
+++ b/moto/rds/responses.py
@@ -489,34 +489,11 @@ class RDSResponse(BaseResponse):
 
     def modify_option_group(self) -> str:
         option_group_name = self._get_param("OptionGroupName")
-        count = 1
-        options_to_include = []
-        # TODO: This can probably be refactored with a single call to super.get_multi_param, but there are not enough tests (yet) to verify this
-        while self._get_param(f"OptionsToInclude.member.{count}.OptionName"):
-            options_to_include.append(
-                {
-                    "Port": self._get_param(f"OptionsToInclude.member.{count}.Port"),
-                    "OptionName": self._get_param(
-                        f"OptionsToInclude.member.{count}.OptionName"
-                    ),
-                    "DBSecurityGroupMemberships": self._get_param(
-                        f"OptionsToInclude.member.{count}.DBSecurityGroupMemberships"
-                    ),
-                    "OptionSettings": self._get_param(
-                        f"OptionsToInclude.member.{count}.OptionSettings"
-                    ),
-                    "VpcSecurityGroupMemberships": self._get_param(
-                        f"OptionsToInclude.member.{count}.VpcSecurityGroupMemberships"
-                    ),
-                }
-            )
-            count += 1
+        options_to_include = (self._get_multi_param_dict("OptionsToInclude") or {}).get(
+            "OptionConfiguration", []
+        )
+        options_to_remove = self._get_params().get("OptionsToRemove", [])
 
-        count = 1
-        options_to_remove = []
-        while self._get_param(f"OptionsToRemove.member.{count}"):
-            options_to_remove.append(self._get_param(f"OptionsToRemove.member.{count}"))
-            count += 1
         option_group = self.backend.modify_option_group(
             option_group_name, options_to_include, options_to_remove
         )


Test patch:
diff --git a/.github/workflows/tests_sdk_ruby.yml b/.github/workflows/tests_sdk_ruby.yml
index 189d87f0d..f953187bb 100644
--- a/.github/workflows/tests_sdk_ruby.yml
+++ b/.github/workflows/tests_sdk_ruby.yml
@@ -13,7 +13,7 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: Set up Ruby ${{ matrix.ruby-version }}
-        uses: ruby/setup-ruby@a6b46b8a08edb18935835849f2a17072d5cc8c73
+        uses: ruby/setup-ruby@f321cf5a4d1533575411f8752cf25b86478b0442
         with:
           ruby-version: ${{ matrix.ruby-version }}
       - name: Set up Python 3.8
diff --git a/tests/test_firehose/test_firehose_put.py b/tests/test_firehose/test_firehose_put.py
index 8d58d6ce5..851bd17ed 100644
--- a/tests/test_firehose/test_firehose_put.py
+++ b/tests/test_firehose/test_firehose_put.py
@@ -149,3 +149,69 @@ def test_put_record_batch_extended_s3_destination():
     assert re.match(
         r"^[0-9]{4}/[0-9]{2}/[0-9]{2}/[0-9]{2}/", bucket_objects["Contents"][0]["Key"]
     )
+
+
+@mock_aws
+def test_put_record_snowflake_destination():
+    """Test invocations of put_record() to a Snowflake destination."""
+    client = boto3.client("firehose", region_name=TEST_REGION)
+
+    stream_name = f"test_put_record_{mock_random.get_random_hex(6)}"
+    client.create_delivery_stream(
+        DeliveryStreamName=stream_name,
+        SnowflakeDestinationConfiguration={
+            "RoleARN": f"arn:aws:iam::{ACCOUNT_ID}:role/firehose_delivery_role",
+            "AccountUrl": f"fake-account.{TEST_REGION}.snowflakecomputing.com",
+            "Database": "myDatabase",
+            "Schema": "mySchema",
+            "Table": "myTable",
+            "S3BackupMode": "FailedDataOnly",
+            "S3Configuration": {
+                "RoleARN": f"arn:aws:iam::{ACCOUNT_ID}:role/firehose_delivery_role",
+                "BucketARN": "arn:aws:s3:::firehose-test",
+            },
+        },
+    )
+    result = client.put_record(
+        DeliveryStreamName=stream_name, Record={"Data": "some test data"}
+    )
+
+    assert set(result.keys()) == {"RecordId", "Encrypted", "ResponseMetadata"}
+    assert result["Encrypted"] is False
+    assert result["ResponseMetadata"]["HTTPStatusCode"] == 200
+
+
+@mock_aws
+def test_put_record_batch_snowflake_destination():
+    """Test invocations of put_record_batch() to a Snowflake destination."""
+    client = boto3.client("firehose", region_name=TEST_REGION)
+
+    stream_name = f"test_put_record_{mock_random.get_random_hex(6)}"
+    client.create_delivery_stream(
+        DeliveryStreamName=stream_name,
+        SnowflakeDestinationConfiguration={
+            "RoleARN": f"arn:aws:iam::{ACCOUNT_ID}:role/firehose_delivery_role",
+            "AccountUrl": f"fake-account.{TEST_REGION}.snowflakecomputing.com",
+            "Database": "myDatabase",
+            "Schema": "mySchema",
+            "Table": "myTable",
+            "S3BackupMode": "FailedDataOnly",
+            "S3Configuration": {
+                "RoleARN": f"arn:aws:iam::{ACCOUNT_ID}:role/firehose_delivery_role",
+                "BucketARN": "arn:aws:s3:::firehose-test",
+            },
+        },
+    )
+
+    records = [{"Data": "one"}, {"Data": "two"}, {"Data": "three"}]
+    result = client.put_record_batch(DeliveryStreamName=stream_name, Records=records)
+
+    assert set(result.keys()) == {
+        "RequestResponses",
+        "FailedPutCount",
+        "ResponseMetadata",
+        "Encrypted",
+    }
+    assert len(result["RequestResponses"]) == 3
+    assert result["Encrypted"] is False
+    assert result["ResponseMetadata"]["HTTPStatusCode"] == 200
diff --git a/tests/test_fsx/test_fsx.py b/tests/test_fsx/test_fsx.py
index c9ea7b716..8246d3a8b 100644
--- a/tests/test_fsx/test_fsx.py
+++ b/tests/test_fsx/test_fsx.py
@@ -1,5 +1,6 @@
 """Unit tests for fsx-supported APIs."""
 
+import re
 import time
 
 import boto3
@@ -32,7 +33,7 @@ def test_create_filesystem(client):
 
     file_system = resp["FileSystem"]
 
-    assert file_system["FileSystemId"].startswith("fs-moto")
+    assert re.match(r"^fs-[0-9a-f]{8,}$", file_system["FileSystemId"])
     assert file_system["FileSystemType"] == "LUSTRE"
 
 
@@ -90,7 +91,6 @@ def test_delete_file_system():
         SecurityGroupIds=FAKE_SECURITY_GROUP_IDS,
     )
 
-    assert fs["FileSystem"]["FileSystemId"].startswith("fs-moto")
     file_system_id = fs["FileSystem"]["FileSystemId"]
     resp = client.delete_file_system(FileSystemId=file_system_id)
     assert resp["FileSystemId"] == file_system_id
diff --git a/tests/test_rds/test_rds.py b/tests/test_rds/test_rds.py
index 6ed50fb0e..68913255f 100644
--- a/tests/test_rds/test_rds.py
+++ b/tests/test_rds/test_rds.py
@@ -1,4 +1,5 @@
 import datetime
+from uuid import uuid4
 
 import boto3
 import pytest
@@ -6,6 +7,7 @@ from botocore.exceptions import ClientError
 
 from moto import mock_aws
 from moto.core import DEFAULT_ACCOUNT_ID as ACCOUNT_ID
+from tests import aws_verified
 
 DEFAULT_REGION = "us-west-2"
 
@@ -1468,27 +1470,96 @@ def test_describe_option_group_options():
         )
 
 
-@mock_aws
+@pytest.mark.aws_verified
+@aws_verified
 def test_modify_option_group():
-    conn = boto3.client("rds", region_name=DEFAULT_REGION)
+    conn = boto3.client("rds", region_name="us-east-1")
+    option_group_name = f"og-{str(uuid4())[0:6]}"
     conn.create_option_group(
-        OptionGroupName="test",
+        OptionGroupName=option_group_name,
         EngineName="mysql",
         MajorEngineVersion="5.6",
         OptionGroupDescription="test option group",
     )
-    # TODO: create option and validate before deleting.
-    # if Someone can tell me how the hell to use this function
-    # to add options to an option_group, I can finish coding this.
-    result = conn.modify_option_group(
-        OptionGroupName="test",
-        OptionsToInclude=[],
-        OptionsToRemove=["MEMCACHED"],
-        ApplyImmediately=True,
-    )
-    assert result["OptionGroup"]["EngineName"] == "mysql"
-    assert result["OptionGroup"]["Options"] == []
-    assert result["OptionGroup"]["OptionGroupName"] == "test"
+
+    try:
+        # Verify OptionsToRemove do not have to exist
+        option_group = conn.modify_option_group(
+            OptionGroupName=option_group_name,
+            OptionsToInclude=[],
+            OptionsToRemove=["MEMCACHED"],
+            ApplyImmediately=True,
+        )["OptionGroup"]
+        assert option_group["EngineName"] == "mysql"
+        assert option_group["Options"] == []
+        assert option_group["OptionGroupName"] == option_group_name
+
+        option_groups = conn.describe_option_groups(OptionGroupName=option_group_name)[
+            "OptionGroupsList"
+        ]
+        assert option_groups[0]["Options"] == []
+
+        # Include option
+        # https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Appendix.MySQL.Options.AuditPlugin.html
+        conn.modify_option_group(
+            OptionGroupName=option_group_name,
+            OptionsToInclude=[{"OptionName": "MARIADB_AUDIT_PLUGIN"}],
+            OptionsToRemove=[],
+            ApplyImmediately=True,
+        )["OptionGroup"]
+
+        # Verify it was added successfully
+        option_groups = conn.describe_option_groups(OptionGroupName=option_group_name)[
+            "OptionGroupsList"
+        ]
+
+        options = option_groups[0]["Options"]
+        assert len(options) == 1
+        assert options[0]["OptionName"] == "MARIADB_AUDIT_PLUGIN"
+        # AWS automatically adds a description + default option settings, but Moto does not support that yet
+
+        # Change setting for an existing option
+        conn.modify_option_group(
+            OptionGroupName=option_group_name,
+            OptionsToInclude=[
+                {
+                    "OptionName": "MARIADB_AUDIT_PLUGIN",
+                    "OptionSettings": [
+                        {"Name": "SERVER_AUDIT_FILE_ROTATE_SIZE", "Value": "1000"},
+                    ],
+                }
+            ],
+            ApplyImmediately=True,
+        )["OptionGroup"]
+
+        # Verify it was added successfully
+        option_groups = conn.describe_option_groups(OptionGroupName=option_group_name)[
+            "OptionGroupsList"
+        ]
+
+        options = option_groups[0]["Options"]
+        assert len(options) == 1
+        assert options[0]["OptionName"] == "MARIADB_AUDIT_PLUGIN"
+
+        option_settings = options[0]["OptionSettings"]
+        audit_plugin = [
+            o for o in option_settings if o["Name"] == "SERVER_AUDIT_FILE_ROTATE_SIZE"
+        ][0]
+        audit_plugin["Name"] == "SERVER_AUDIT_FILE_ROTATE_SIZE"
+        audit_plugin["Value"] == "1000"
+
+        # Verify option can be deleted
+        conn.modify_option_group(
+            OptionGroupName=option_group_name,
+            OptionsToRemove=["MARIADB_AUDIT_PLUGIN"],
+            ApplyImmediately=True,
+        )
+        option_groups = conn.describe_option_groups(OptionGroupName=option_group_name)[
+            "OptionGroupsList"
+        ]
+        assert option_groups[0]["Options"] == []
+    finally:
+        conn.delete_option_group(OptionGroupName=option_group_name)
 
 
 @mock_aws