Repository: getmoto/moto
Pull request #8335 (2024-11-19 10:46:39)

SecretsManager: Improve list-secrets to exclude deleted secrets
Original Issue: https://github.com/localstack/localstack/issues/11635

Description:
This PR addresses an issue where deleted secrets were still being displayed in the list-secrets API response.

Changes:
Modified list-secrets API: The list-secrets API has been modified to filter out deleted secrets by default.

Updated Unit Tests: Unit tests have been updated to verify the correct behavior of the list-secrets API.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/getmoto/moto/pull/8335?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 94.58%. Comparing base [(`edc7947`)](https://app.codecov.io/gh/getmoto/moto/commit/edc7947883a82f8a995d98e9a3a21240ea90a53b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) to head [(`dc0ed54`)](https://app.codecov.io/gh/getmoto/moto/commit/dc0ed54f5ea7e21b46408b3340e449012afdb318?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto).

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##           master    #8335   +/-   ##
=======================================
  Coverage   94.57%   94.58%           
=======================================
  Files        1159     1159           
  Lines      101037   101041    +4     
=======================================
+ Hits        95558    95565    +7     
+ Misses       5479     5476    -3     
```

| [Flag](https://app.codecov.io/gh/getmoto/moto/pull/8335/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) | Coverage Δ | |
|---|---|---|
| [servertests](https://app.codecov.io/gh/getmoto/moto/pull/8335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) | `28.90% <0.00%> (-0.01%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/getmoto/moto/pull/8335/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) | `94.55% <100.00%> (+<0.01%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/getmoto/moto/pull/8335?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto).

----
🚨 Try these New Features:

- [Flaky Tests Detection](https://docs.codecov.com/docs/test-result-ingestion-beta) - Detect and resolve failed and flaky tests
- [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis) - Avoid shipping oversized bundles

Patch:
diff --git a/moto/ds/models.py b/moto/ds/models.py
index 25f0e78c2..b55e95932 100644
--- a/moto/ds/models.py
+++ b/moto/ds/models.py
@@ -222,7 +222,7 @@ class Directory(BaseModel):  # pylint: disable=too-many-instance-attributes
         """Enable/disable ldaps based on whether new_state is True or False.
         This method is only for MicrosoftAD.
         """
-        if self.directory_type != "MicrosoftAD":
+        if self.directory_type not in ("MicrosoftAD", "ADConnector"):
             raise UnsupportedOperationException(
                 "LDAPS operations are not supported for this Directory Type."
             )
@@ -673,7 +673,7 @@ class DirectoryServiceBackend(BaseBackend):
         """Describe LDAPS settings for a Directory"""
         self._validate_directory_id(directory_id)
         directory = self.directories[directory_id]
-        if directory.directory_type != "MicrosoftAD":
+        if directory.directory_type not in ("MicrosoftAD", "ADConnector"):
             raise UnsupportedOperationException(
                 "LDAPS operations are not supported for this Directory Type."
             )
diff --git a/moto/ecs/models.py b/moto/ecs/models.py
index a39f3fb7f..beb898d87 100644
--- a/moto/ecs/models.py
+++ b/moto/ecs/models.py
@@ -14,6 +14,7 @@ from moto.moto_api._internal import mock_random
 from moto.moto_api._internal.managed_state_model import ManagedState
 from moto.utilities.utils import ARN_PARTITION_REGEX, get_partition
 
+from ..ec2.exceptions import InvalidSecurityGroupNotFoundError, InvalidSubnetIdError
 from ..ec2.utils import random_private_ip
 from .exceptions import (
     ClusterNotFoundException,
@@ -548,6 +549,7 @@ class Service(BaseObject, CloudFormationModel):
         launch_type: Optional[str] = None,
         service_registries: Optional[List[Dict[str, Any]]] = None,
         platform_version: Optional[str] = None,
+        network_configuration: Optional[Dict[str, Dict[str, List[str]]]] = None,
         propagate_tags: str = "NONE",
     ):
         self.cluster_name = cluster.name
@@ -598,6 +600,37 @@ class Service(BaseObject, CloudFormationModel):
             self.deployments = []
         self.propagate_tags = propagate_tags
 
+        if network_configuration is not None:
+            self.network_configuration = self._validate_network(network_configuration)
+        else:
+            self.network_configuration = {}
+
+    def _validate_network(
+        self, nc: Dict[str, Dict[str, List[str]]]
+    ) -> Dict[str, Dict[str, List[str]]]:
+        c = nc["awsvpcConfiguration"]
+        if len(c["subnets"]) == 0:
+            raise InvalidParameterException("subnets can not be empty.")
+
+        ec2_backend = ec2_backends[self._account_id][self.region_name]
+        try:
+            ec2_backend.describe_subnets(subnet_ids=c["subnets"])
+        except InvalidSubnetIdError as exc:
+            subnet_id = exc.message.split("'")[1]
+            raise InvalidParameterException(
+                f"Error retrieving subnet information for [{subnet_id}]: {exc.message} (ErrorCode: {exc.error_type})"
+            )
+
+        try:
+            ec2_backend.describe_security_groups(group_ids=c["securityGroups"])
+        except InvalidSecurityGroupNotFoundError as exc:
+            sg = exc.message.split("'{'")[1].split("'}'")[0]
+            raise InvalidParameterException(
+                f"Error retrieving security group information for [{sg}]: "
+                f"The security group '{sg}' does not exist (ErrorCode: InvalidGroup.NotFound)"
+            )
+        return nc
+
     @property
     def arn(self) -> str:
         if self._backend.enable_long_arn_for_name(name="serviceLongArnFormat"):
@@ -633,6 +666,7 @@ class Service(BaseObject, CloudFormationModel):
                 deployment["updatedAt"] = unix_time(
                     deployment["updatedAt"].replace(tzinfo=None)
                 )
+        response_object["networkConfiguration"] = self.network_configuration
 
         return response_object
 
@@ -1613,6 +1647,7 @@ class EC2ContainerServiceBackend(BaseBackend):
         service_registries: Optional[List[Dict[str, Any]]] = None,
         platform_version: Optional[str] = None,
         propagate_tags: str = "NONE",
+        network_configuration: Optional[Dict[str, Dict[str, List[str]]]] = None,
     ) -> Service:
         cluster = self._get_cluster(cluster_str)
 
@@ -1640,6 +1675,7 @@ class EC2ContainerServiceBackend(BaseBackend):
             service_registries=service_registries,
             platform_version=platform_version,
             propagate_tags=propagate_tags,
+            network_configuration=network_configuration,
         )
         cluster_service_pair = f"{cluster.name}:{service_name}"
         self.services[cluster_service_pair] = service
diff --git a/moto/ecs/responses.py b/moto/ecs/responses.py
index 090069c96..8d9ca0fc3 100644
--- a/moto/ecs/responses.py
+++ b/moto/ecs/responses.py
@@ -262,6 +262,7 @@ class EC2ContainerServiceResponse(BaseResponse):
         launch_type = self._get_param("launchType")
         platform_version = self._get_param("platformVersion")
         propagate_tags = self._get_param("propagateTags") or "NONE"
+        network_configuration = self._get_param("networkConfiguration")
         service = self.ecs_backend.create_service(
             cluster_str,
             service_name,
@@ -272,6 +273,7 @@ class EC2ContainerServiceResponse(BaseResponse):
             tags,
             deployment_controller,
             launch_type,
+            network_configuration=network_configuration,
             service_registries=service_registries,
             platform_version=platform_version,
             propagate_tags=propagate_tags,
diff --git a/moto/secretsmanager/models.py b/moto/secretsmanager/models.py
index b2f185fb0..a0c7acad0 100644
--- a/moto/secretsmanager/models.py
+++ b/moto/secretsmanager/models.py
@@ -907,9 +907,13 @@ class SecretsManagerBackend(BaseBackend):
         filters: List[Dict[str, Any]],
         max_results: int = MAX_RESULTS_DEFAULT,
         next_token: Optional[str] = None,
+        include_planned_deletion: bool = False,
     ) -> Tuple[List[Dict[str, Any]], Optional[str]]:
         secret_list: List[Dict[str, Any]] = []
         for secret in self.secrets.values():
+            if hasattr(secret, "deleted_date"):
+                if secret.deleted_date and not include_planned_deletion:
+                    continue
             if _matches(secret, filters):
                 secret_list.append(secret.to_dict())
 
diff --git a/moto/secretsmanager/responses.py b/moto/secretsmanager/responses.py
index 72a7403fa..2cfc7a3f6 100644
--- a/moto/secretsmanager/responses.py
+++ b/moto/secretsmanager/responses.py
@@ -175,8 +175,14 @@ class SecretsManagerResponse(BaseResponse):
         _validate_filters(filters)
         max_results = self._get_int_param("MaxResults")
         next_token = self._get_param("NextToken")
+        include_planned_deletion = self._get_param(
+            "IncludePlannedDeletion", if_none=False
+        )
         secret_list, next_token = self.backend.list_secrets(
-            filters=filters, max_results=max_results, next_token=next_token
+            filters=filters,
+            max_results=max_results,
+            next_token=next_token,
+            include_planned_deletion=include_planned_deletion,
         )
         return json.dumps(dict(SecretList=secret_list, NextToken=next_token))
 


Test patch:
diff --git a/tests/test_ds/test_ds_ad_connect.py b/tests/test_ds/test_ds_ad_connect.py
index e95bc4c41..1418ee478 100644
--- a/tests/test_ds/test_ds_ad_connect.py
+++ b/tests/test_ds/test_ds_ad_connect.py
@@ -287,3 +287,34 @@ def test_ds_get_connect_directory_limits():
     assert limits["ConnectedDirectoriesLimitReached"]
     assert not limits["CloudOnlyDirectoriesCurrentCount"]
     assert not limits["CloudOnlyMicrosoftADCurrentCount"]
+
+
+@mock_aws
+def test_enable_describe_disable_ldaps():
+    """Test good and bad invocations of describe_directories()."""
+    client = boto3.client("ds", region_name=TEST_REGION)
+    ec2_client = boto3.client("ec2", region_name=TEST_REGION)
+
+    directory_id = create_test_ad_connector(client, ec2_client)
+
+    # Describe LDAPS settings for AD Connector without LDAPS enabled
+    ldaps = client.describe_ldaps_settings(DirectoryId=directory_id)[
+        "LDAPSSettingsInfo"
+    ]
+    assert ldaps == []
+
+    # Enable LDAPS for AD Connector and verify it is enabled
+    client.enable_ldaps(DirectoryId=directory_id, Type="Client")
+    ldaps = client.describe_ldaps_settings(DirectoryId=directory_id)[
+        "LDAPSSettingsInfo"
+    ]
+    assert len(ldaps) == 1
+    assert ldaps[0]["LDAPSStatus"] == "Enabled"
+
+    # Disable LDAPS for AD Connector and verify it is disabled
+    client.disable_ldaps(DirectoryId=directory_id, Type="Client")
+    ldaps = client.describe_ldaps_settings(DirectoryId=directory_id)[
+        "LDAPSSettingsInfo"
+    ]
+    assert len(ldaps) == 1
+    assert ldaps[0]["LDAPSStatus"] == "Disabled"
diff --git a/tests/test_ecs/test_ecs_boto3.py b/tests/test_ecs/test_ecs_boto3.py
index 5f6baba84..465178f1b 100644
--- a/tests/test_ecs/test_ecs_boto3.py
+++ b/tests/test_ecs/test_ecs_boto3.py
@@ -648,52 +648,53 @@ def test_deregister_task_definition_2():
 @mock_aws
 def test_create_service():
     client = boto3.client("ecs", region_name=ECS_REGION)
-    client.create_cluster(clusterName="test_ecs_cluster")
-    client.register_task_definition(
-        family="test_ecs_task",
-        containerDefinitions=[
-            {
-                "name": "hello_world",
-                "image": "docker/hello-world:latest",
-                "cpu": 1024,
-                "memory": 400,
-                "essential": True,
-                "environment": [
-                    {"name": "AWS_ACCESS_KEY_ID", "value": "SOME_ACCESS_KEY"}
-                ],
-                "logConfiguration": {"logDriver": "json-file"},
-            }
-        ],
-    )
+    ec2 = boto3.resource("ec2", region_name=ECS_REGION)
+
+    # ECS setup
+    subnet, sg = setup_ecs(client, ec2)
     response = client.create_service(
         cluster="test_ecs_cluster",
         serviceName="test_ecs_service",
         taskDefinition="test_ecs_task",
         desiredCount=2,
         platformVersion="2",
+        networkConfiguration={
+            "awsvpcConfiguration": {
+                "subnets": [subnet.id],
+                "securityGroups": [sg.id],
+            }
+        },
     )
     assert (
         response["service"]["clusterArn"]
         == f"arn:aws:ecs:us-east-1:{ACCOUNT_ID}:cluster/test_ecs_cluster"
     )
-    assert response["service"]["desiredCount"] == 2
-    assert len(response["service"]["events"]) == 0
-    assert len(response["service"]["loadBalancers"]) == 0
-    assert response["service"]["pendingCount"] == 2
-    assert response["service"]["runningCount"] == 0
+    svc = response["service"]
+    assert svc["desiredCount"] == 2
+    assert len(svc["events"]) == 0
+    assert len(svc["loadBalancers"]) == 0
+    assert svc["pendingCount"] == 2
+    assert svc["runningCount"] == 0
     assert (
-        response["service"]["serviceArn"]
+        svc["serviceArn"]
         == f"arn:aws:ecs:us-east-1:{ACCOUNT_ID}:service/test_ecs_cluster/test_ecs_service"
     )
-    assert response["service"]["serviceName"] == "test_ecs_service"
-    assert response["service"]["status"] == "ACTIVE"
+    assert svc["serviceName"] == "test_ecs_service"
+    assert svc["status"] == "ACTIVE"
     assert (
-        response["service"]["taskDefinition"]
+        svc["taskDefinition"]
         == f"arn:aws:ecs:us-east-1:{ACCOUNT_ID}:task-definition/test_ecs_task:1"
     )
-    assert response["service"]["schedulingStrategy"] == "REPLICA"
-    assert response["service"]["launchType"] == "EC2"
-    assert response["service"]["platformVersion"] == "2"
+    assert svc["schedulingStrategy"] == "REPLICA"
+    assert svc["launchType"] == "EC2"
+    assert svc["platformVersion"] == "2"
+
+    assert "networkConfiguration" in svc
+    nc = svc["networkConfiguration"]["awsvpcConfiguration"]
+    assert len(["subnets"]) == 1
+    assert len(["securityGroups"]) == 1
+    assert nc["subnets"][0] == subnet.id
+    assert nc["securityGroups"][0] == sg.id
 
 
 @mock_aws
@@ -3844,3 +3845,54 @@ def setup_ecs_cluster_with_ec2_instance(client, test_cluster_name):
             }
         ],
     )
+
+
+c1 = {
+    "awsvpcConfiguration": {
+        "subnets": ["fake-subnet"],
+    }
+}
+e1 = "Error retrieving subnet information for [fake-subnet]: The subnet ID 'fake-subnet' does not exist (ErrorCode: InvalidSubnetID.NotFound)"
+c2 = {
+    "awsvpcConfiguration": {
+        "subnets": "[subnet.id]",
+        "securityGroups": ["sg-123456"],
+    }
+}
+e2 = "Error retrieving security group information for [sg-123456]: The security group 'sg-123456' does not exist (ErrorCode: InvalidGroup.NotFound)"
+c3 = {
+    "awsvpcConfiguration": {
+        "subnets": [],
+    }
+}
+e3 = "subnets can not be empty."
+
+
+@pytest.mark.parametrize(
+    "net_config,error_message",
+    [(c1, e1), (c2, e2), (c3, e3)],
+)
+@mock_aws
+def test_service_exceptions(net_config, error_message):
+    client = boto3.client("ecs", region_name=ECS_REGION)
+    ec2 = boto3.resource("ec2", region_name=ECS_REGION)
+
+    # ECS setup
+    subnet, sg = setup_ecs(client, ec2)
+    if net_config["awsvpcConfiguration"]["subnets"] == "[subnet.id]":
+        # sub real subnet for evaluation
+        net_config["awsvpcConfiguration"]["subnets"] = [subnet.id]
+
+    with pytest.raises(ClientError) as exc:
+        client.create_service(
+            cluster="test_ecs_cluster",
+            serviceName="test_ecs_service",
+            taskDefinition="test_ecs_task",
+            desiredCount=2,
+            platformVersion="2",
+            networkConfiguration=net_config,
+        )
+    ex = exc.value
+    assert ex.response["ResponseMetadata"]["HTTPStatusCode"] == 400
+    assert ex.response["Error"]["Code"] == "InvalidParameterException"
+    assert ex.response["Error"]["Message"] == error_message
diff --git a/tests/test_secretsmanager/test_list_secrets.py b/tests/test_secretsmanager/test_list_secrets.py
index 1acb8a9d3..37de56a78 100644
--- a/tests/test_secretsmanager/test_list_secrets.py
+++ b/tests/test_secretsmanager/test_list_secrets.py
@@ -285,3 +285,36 @@ def test_filter_with_owning_service():
 
     resp = conn.list_secrets(Filters=[{"Key": "owning-service", "Values": ["n/a"]}])
     assert resp["SecretList"] == []
+
+
+@mock_aws
+def test_with_include_planned_deleted_secrets():
+    conn = boto_client()
+
+    conn.create_secret(Name="foo", SecretString="secret")
+    conn.create_secret(Name="bar", SecretString="secret")
+
+    secrets = conn.list_secrets()
+    assert len(secrets["SecretList"]) == 2
+
+    conn.delete_secret(SecretId="foo")
+
+    # By default list secrets doesn't include deleted secrets
+    secrets = conn.list_secrets()
+    assert len(secrets["SecretList"]) == 1
+    assert secrets["SecretList"][0]["ARN"] is not None
+    assert secrets["SecretList"][0]["Name"] == "bar"
+    assert secrets["SecretList"][0]["SecretVersionsToStages"] is not None
+
+    # list secrets when IncludePlannedDeletion param included
+    secrets = conn.list_secrets(IncludePlannedDeletion=True)
+    assert len(secrets["SecretList"]) == 2
+
+    # list secret with filter and IncludePlannedDeletion params
+    secrets = conn.list_secrets(
+        IncludePlannedDeletion=True, Filters=[{"Key": "name", "Values": ["foo"]}]
+    )
+    assert len(secrets["SecretList"]) == 1
+    assert secrets["SecretList"][0]["ARN"] is not None
+    assert secrets["SecretList"][0]["Name"] == "foo"
+    assert secrets["SecretList"][0]["SecretVersionsToStages"] is not None