Repository: getmoto/moto
Pull request #8086 (2024-09-05 06:45:46)

ec2: fix security groups describe rule response
When using the `describe-security-group-rules` call with a security-group-id `filter`, it correctly returns the egress and ingress rules. However, if we try to use the `describe-security-group-rules` call and specify the `--security-group-rule-ids`, it does not return the rule as expected. 

This PR will now consider both egress and ingress rules while using `--security-group-rule-ids` param in the command. 

Fixes: https://github.com/localstack/localstack/issues/11445

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/getmoto/moto/pull/8086?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.44%. Comparing base [(`e0f5581`)](https://app.codecov.io/gh/getmoto/moto/commit/e0f5581f68cf2391db760f61ecefccfbff69810c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) to head [(`a2ec425`)](https://app.codecov.io/gh/getmoto/moto/commit/a2ec425a6a86a77b7b502ea7af3718d494baccd3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto).
> Report is 1 commits behind head on master.

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


```diff
@@           Coverage Diff           @@
##           master    #8086   +/-   ##
=======================================
  Coverage   94.44%   94.44%           
=======================================
  Files        1141     1141           
  Lines       98175    98174    -1     
=======================================
+ Hits        92723    92724    +1     
+ Misses       5452     5450    -2     
```

| [Flag](https://app.codecov.io/gh/getmoto/moto/pull/8086/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/8086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) | `28.96% <0.00%> (+<0.01%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/getmoto/moto/pull/8086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getmoto) | `94.42% <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/8086?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).


Patch:
diff --git a/moto/ec2/models/security_groups.py b/moto/ec2/models/security_groups.py
index eac8cf5f2..c9e7b98f8 100644
--- a/moto/ec2/models/security_groups.py
+++ b/moto/ec2/models/security_groups.py
@@ -634,12 +634,17 @@ class SecurityGroupBackend:
         results = []
 
         if sg_rule_ids:
+            rules_sources = [
+                self.sg_old_ingress_ruls.items(),
+                self.sg_old_egress_ruls.items(),
+            ]
             # go thru all the rules in the backend to find a match
             for sg_rule_id in sg_rule_ids:
-                for sg in self.sg_old_ingress_ruls:
-                    for rule in self.sg_old_ingress_ruls[sg]:
-                        if rule.id == sg_rule_id:
-                            results.append(rule)
+                for rules in rules_sources:
+                    for sg, rules_list in rules:
+                        for rule in rules_list:
+                            if rule.id == sg_rule_id:
+                                results.append(rule)
 
             return results
 
diff --git a/moto/stepfunctions/models.py b/moto/stepfunctions/models.py
index d2ea33a56..f8008e0cf 100644
--- a/moto/stepfunctions/models.py
+++ b/moto/stepfunctions/models.py
@@ -45,6 +45,7 @@ class StateMachine(CloudFormationModel):
         self.tags: List[Dict[str, str]] = []
         if tags:
             self.add_tags(tags)
+        self.version = 0
 
     def start_execution(
         self,
@@ -521,6 +522,7 @@ class StepFunctionBackend(BaseBackend):
         definition: str,
         roleArn: str,
         tags: Optional[List[Dict[str, str]]] = None,
+        publish: Optional[bool] = None,
     ) -> StateMachine:
         self._validate_name(name)
         self._validate_role_arn(roleArn)
@@ -529,6 +531,8 @@ class StepFunctionBackend(BaseBackend):
             return self.describe_state_machine(arn)
         except StateMachineDoesNotExist:
             state_machine = StateMachine(arn, name, definition, roleArn, tags)
+            if publish:
+                state_machine.version += 1
             self.state_machines.append(state_machine)
             return state_machine
 
@@ -552,7 +556,13 @@ class StepFunctionBackend(BaseBackend):
             self.state_machines.remove(sm)
 
     def update_state_machine(
-        self, arn: str, definition: Optional[str] = None, role_arn: Optional[str] = None
+        self,
+        arn: str,
+        definition: Optional[str] = None,
+        role_arn: Optional[str] = None,
+        logging_configuration: Optional[Dict[str, bool]] = None,
+        tracing_configuration: Optional[Dict[str, bool]] = None,
+        publish: Optional[bool] = None,
     ) -> StateMachine:
         sm = self.describe_state_machine(arn)
         updates = {
@@ -560,6 +570,8 @@ class StepFunctionBackend(BaseBackend):
             "roleArn": role_arn,
         }
         sm.update(**updates)
+        if publish:
+            sm.version += 1
         return sm
 
     def start_execution(
diff --git a/moto/stepfunctions/parser/models.py b/moto/stepfunctions/parser/models.py
index ca4abf7cc..ca75eff7f 100644
--- a/moto/stepfunctions/parser/models.py
+++ b/moto/stepfunctions/parser/models.py
@@ -14,7 +14,6 @@ from moto.stepfunctions.parser.api import (
     LoggingConfiguration,
     MissingRequiredParameter,
     Name,
-    Publish,
     ResourceNotFound,
     SendTaskFailureOutput,
     SendTaskHeartbeatOutput,
@@ -81,11 +80,16 @@ class StepFunctionsParserBackend(StepFunctionBackend):
         definition: str,
         roleArn: str,
         tags: Optional[List[Dict[str, str]]] = None,
+        publish: Optional[bool] = None,
     ) -> StateMachine:
         StepFunctionsParserBackend._validate_definition(definition=definition)
 
         return super().create_state_machine(
-            name=name, definition=definition, roleArn=roleArn, tags=tags
+            name=name,
+            definition=definition,
+            roleArn=roleArn,
+            tags=tags,
+            publish=publish,
         )
 
     def send_task_heartbeat(self, task_token: TaskToken) -> SendTaskHeartbeatOutput:
@@ -188,7 +192,7 @@ class StepFunctionsParserBackend(StepFunctionBackend):
         role_arn: str = None,
         logging_configuration: LoggingConfiguration = None,
         tracing_configuration: TracingConfiguration = None,
-        publish: Publish = None,
+        publish: Optional[bool] = None,
         version_description: VersionDescription = None,
     ) -> StateMachine:
         if not any(
@@ -201,7 +205,14 @@ class StepFunctionsParserBackend(StepFunctionBackend):
         if definition is not None:
             self._validate_definition(definition=definition)
 
-        return super().update_state_machine(arn, definition, role_arn)
+        return super().update_state_machine(
+            arn,
+            definition,
+            role_arn,
+            logging_configuration=logging_configuration,
+            tracing_configuration=tracing_configuration,
+            publish=publish,
+        )
 
     def describe_map_run(self, map_run_arn: str) -> Dict[str, Any]:
         for execution in self._get_executions():
diff --git a/moto/stepfunctions/responses.py b/moto/stepfunctions/responses.py
index 70633c164..b258b460c 100644
--- a/moto/stepfunctions/responses.py
+++ b/moto/stepfunctions/responses.py
@@ -28,13 +28,22 @@ class StepFunctionResponse(BaseResponse):
         definition = self._get_param("definition")
         roleArn = self._get_param("roleArn")
         tags = self._get_param("tags")
+        publish = self._get_param("publish")
         state_machine = self.stepfunction_backend.create_state_machine(
-            name=name, definition=definition, roleArn=roleArn, tags=tags
+            name=name,
+            definition=definition,
+            roleArn=roleArn,
+            tags=tags,
+            publish=publish,
         )
         response = {
             "creationDate": state_machine.creation_date,
             "stateMachineArn": state_machine.arn,
         }
+        if publish:
+            response["stateMachineVersionArn"] = (
+                f"{state_machine.arn}:{state_machine.version}"
+            )
         return 200, {}, json.dumps(response)
 
     def list_state_machines(self) -> TYPE_RESPONSE:
@@ -83,12 +92,22 @@ class StepFunctionResponse(BaseResponse):
         arn = self._get_param("stateMachineArn")
         definition = self._get_param("definition")
         role_arn = self._get_param("roleArn")
+        tracing_config = self._get_param("tracingConfiguration")
+        publish = self._get_param("publish")
         state_machine = self.stepfunction_backend.update_state_machine(
-            arn=arn, definition=definition, role_arn=role_arn
+            arn=arn,
+            definition=definition,
+            role_arn=role_arn,
+            tracing_configuration=tracing_config,
+            publish=publish,
         )
         response = {
             "updateDate": state_machine.update_date,
         }
+        if publish:
+            response["stateMachineVersionArn"] = (
+                f"{state_machine.arn}:{state_machine.version}"
+            )
         return 200, {}, json.dumps(response)
 
     def list_tags_for_resource(self) -> TYPE_RESPONSE:


Test patch:
diff --git a/tests/test_ec2/test_security_groups.py b/tests/test_ec2/test_security_groups.py
index 519b5c2c6..2dc3c797b 100644
--- a/tests/test_ec2/test_security_groups.py
+++ b/tests/test_ec2/test_security_groups.py
@@ -684,6 +684,7 @@ def test_create_and_describe_security_grp_rule():
         Description="Test SG", GroupName=sg_name, VpcId=vpc.id
     )
 
+    # check that the default rule is present using filters
     response = client.describe_security_group_rules(
         Filters=[{"Name": "group-id", "Values": [sg["GroupId"]]}]
     )
@@ -692,11 +693,21 @@ def test_create_and_describe_security_grp_rule():
     # Only the default rule is present
     assert len(rules) == 1
 
+    def _verify_egress_rule(rule):
+        assert rule["IsEgress"] is True
+        assert rule["IpProtocol"] == "-1"
+        assert rule["CidrIpv4"] == "0.0.0.0/0"
+        assert "GroupId" in rule
+
     # Test default egress rule content
-    assert rules[0]["IsEgress"] is True
-    assert rules[0]["IpProtocol"] == "-1"
-    assert rules[0]["CidrIpv4"] == "0.0.0.0/0"
-    assert "GroupId" in rules[0]
+    _verify_egress_rule(rules[0])
+
+    # check that the default rule is present using security group rule ids
+    response = client.describe_security_group_rules(
+        SecurityGroupRuleIds=[rules[0]["SecurityGroupRuleId"]]
+    )
+    rules = response["SecurityGroupRules"]
+    _verify_egress_rule(rules[0])
 
 
 @mock_aws
diff --git a/tests/test_stepfunctions/parser/test_stepfunctions.py b/tests/test_stepfunctions/parser/test_stepfunctions.py
index 515400a52..96c8d2402 100644
--- a/tests/test_stepfunctions/parser/test_stepfunctions.py
+++ b/tests/test_stepfunctions/parser/test_stepfunctions.py
@@ -1,11 +1,20 @@
 import json
+from time import sleep
 from unittest import SkipTest
+from uuid import uuid4
 
+import boto3
 import pytest
 
 from moto import settings
 
-from . import aws_verified, verify_execution_result
+from . import (
+    allow_aws_request,
+    aws_verified,
+    sfn_allow_dynamodb,
+    sfn_role_policy,
+    verify_execution_result,
+)
 
 
 @aws_verified
@@ -82,3 +91,60 @@ def test_state_machine_with_json_regex():
         "json_regex",
         exec_input=json.dumps(_input),
     )
+
+
+@aws_verified
+@pytest.mark.aws_verified
+def test_version_is_only_available_when_published():
+    from tests.test_stepfunctions.test_stepfunctions import simple_definition
+
+    iam = boto3.client("iam", region_name="us-east-1")
+    role_name = f"sfn_role_{str(uuid4())[0:6]}"
+    sfn_role = iam.create_role(
+        RoleName=role_name,
+        AssumeRolePolicyDocument=json.dumps(sfn_role_policy),
+        Path="/",
+    )["Role"]["Arn"]
+    iam.put_role_policy(
+        PolicyDocument=json.dumps(sfn_allow_dynamodb),
+        PolicyName="allowLambdaInvoke",
+        RoleName=role_name,
+    )
+    sleep(10 if allow_aws_request() else 0)
+
+    client = boto3.client("stepfunctions", region_name="us-east-1")
+
+    try:
+        name1 = f"sfn_name_{str(uuid4())[0:6]}"
+        response = client.create_state_machine(
+            name=name1, definition=simple_definition, roleArn=sfn_role
+        )
+        assert "stateMachineVersionArn" not in response
+        arn1 = response["stateMachineArn"]
+
+        resp = client.update_state_machine(
+            stateMachineArn=arn1, publish=True, tracingConfiguration={"enabled": True}
+        )
+        assert resp["stateMachineVersionArn"] == f"{arn1}:1"
+
+        resp = client.update_state_machine(
+            stateMachineArn=arn1, tracingConfiguration={"enabled": False}
+        )
+        assert "stateMachineVersionArn" not in resp
+
+        name2 = f"sfn_name_{str(uuid4())[0:6]}"
+        response = client.create_state_machine(
+            name=name2, definition=simple_definition, roleArn=sfn_role, publish=True
+        )
+        arn2 = response["stateMachineArn"]
+        assert response["stateMachineVersionArn"] == f"{arn2}:1"
+
+        resp = client.update_state_machine(
+            stateMachineArn=arn2, publish=True, tracingConfiguration={"enabled": True}
+        )
+        assert resp["stateMachineVersionArn"] == f"{arn2}:2"
+    finally:
+        client.delete_state_machine(stateMachineArn=arn1)
+        client.delete_state_machine(stateMachineArn=arn2)
+        iam.delete_role_policy(RoleName=role_name, PolicyName="allowLambdaInvoke")
+        iam.delete_role(RoleName=role_name)
diff --git a/tests/test_stepfunctions/test_stepfunctions.py b/tests/test_stepfunctions/test_stepfunctions.py
index 3cf112aa5..1f7688ff7 100644
--- a/tests/test_stepfunctions/test_stepfunctions.py
+++ b/tests/test_stepfunctions/test_stepfunctions.py
@@ -3,6 +3,7 @@ import os
 import re
 from datetime import datetime
 from unittest import SkipTest, mock
+from uuid import uuid4
 
 import boto3
 import pytest
@@ -11,6 +12,7 @@ from dateutil.tz import tzutc
 
 from moto import mock_aws
 from moto.core import DEFAULT_ACCOUNT_ID as ACCOUNT_ID
+from tests.test_stepfunctions.parser import sfn_role_policy
 
 region = "us-east-1"
 simple_definition = (
@@ -979,5 +981,47 @@ def test_state_machine_execution_name_limits():
     )
 
 
+@mock_aws
+def test_version_is_only_available_when_published():
+    iam = boto3.client("iam", region_name="us-east-1")
+    role_name = f"sfn_role_{str(uuid4())[0:6]}"
+    sfn_role = iam.create_role(
+        RoleName=role_name,
+        AssumeRolePolicyDocument=json.dumps(sfn_role_policy),
+        Path="/",
+    )["Role"]["Arn"]
+
+    client = boto3.client("stepfunctions", region_name="us-east-1")
+
+    name1 = f"sfn_name_{str(uuid4())[0:6]}"
+    response = client.create_state_machine(
+        name=name1, definition=simple_definition, roleArn=sfn_role
+    )
+    assert "stateMachineVersionArn" not in response
+    arn1 = response["stateMachineArn"]
+
+    resp = client.update_state_machine(
+        stateMachineArn=arn1, publish=True, tracingConfiguration={"enabled": True}
+    )
+    assert resp["stateMachineVersionArn"] == f"{arn1}:1"
+
+    resp = client.update_state_machine(
+        stateMachineArn=arn1, tracingConfiguration={"enabled": False}
+    )
+    assert "stateMachineVersionArn" not in resp
+
+    name2 = f"sfn_name_{str(uuid4())[0:6]}"
+    response = client.create_state_machine(
+        name=name2, definition=simple_definition, roleArn=sfn_role, publish=True
+    )
+    arn2 = response["stateMachineArn"]
+    assert response["stateMachineVersionArn"] == f"{arn2}:1"
+
+    resp = client.update_state_machine(
+        stateMachineArn=arn2, publish=True, tracingConfiguration={"enabled": True}
+    )
+    assert resp["stateMachineVersionArn"] == f"{arn2}:2"
+
+
 def _get_default_role():
     return "arn:aws:iam::" + ACCOUNT_ID + ":role/unknown_sf_role"