Repository navigation
feat: core-service operation depth + EC2/DynamoDB correctness hardening - #97
Merged
Merged
Conversation
Contributor
Reviewer's GuideExtends EC2 and IAM service support by wiring 24 additional AWS-compatible operations through the existing SQLite-backed stores, adding minimal schema migrations to support security group rules and IAM group policies, and covering them with new boto3-style compatibility tests. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
tests
skyoo2003
added a commit
that referenced
this pull request
Jul 18, 2026
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 7 issues, and left some high level feedback:
- The new SetInstanceState and InstancesExist helpers issue one query per instance ID; consider switching to a single query with an IN clause (or otherwise batching) to avoid N+1 patterns when callers pass many instances.
- DescribeAvailabilityZones hardcodes only us-east-1 AZs while DescribeRegions returns multiple regions; if clients run against a non-us-east-1 region this may be confusing—consider deriving zones from the requested region or documenting/centrally defining the default region behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new SetInstanceState and InstancesExist helpers issue one query per instance ID; consider switching to a single query with an IN clause (or otherwise batching) to avoid N+1 patterns when callers pass many instances.
- DescribeAvailabilityZones hardcodes only us-east-1 AZs while DescribeRegions returns multiple regions; if clients run against a non-us-east-1 region this may be confusing—consider deriving zones from the requested region or documenting/centrally defining the default region behavior.
## Individual Comments
### Comment 1
<location path="internal/services/ec2/provider.go" line_range="405-414" />
<code_context>
+// instanceIDsFromForm collects InstanceId.1, InstanceId.2, … from a Query form.
+func instanceIDsFromForm(form url.Values) []string {
+ var ids []string
+ for i := 1; ; i++ {
+ id := form.Get(fmt.Sprintf("InstanceId.%d", i))
+ if id == "" {
</code_context>
<issue_to_address>
**issue (bug_risk):** Port parsing in parseSGRules silently treats invalid ports as 0, which may hide malformed inputs.
In `parseSGRules`, both structured and legacy parsing use `strconv.Atoi` but ignore its error, treating invalid ports as 0. This silently accepts malformed/non-numeric ports and diverges from AWS behavior. Please validate the conversion and either return an error or drop those rules so callers are aware of invalid input.
</issue_to_address>
### Comment 2
<location path="internal/services/ec2/store.go" line_range="611-620" />
<code_context>
+ return nil
+}
+
+// DeleteSecurityGroup removes a security group by group ID or group name.
+func (s *EC2Store) DeleteSecurityGroup(accountID, identifier string) error {
+ res, err := s.db().Exec(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Deleting a security group does not clean up its rules, leaving potentially orphaned security_group_rules rows.
`DeleteSecurityGroup` deletes from `security_groups` but leaves matching `security_group_rules` rows in place, causing orphaned data and potential query/maintenance issues. Consider adding a cascading delete (via code or FK) or explicitly deleting related `security_group_rules` entries in this operation.
</issue_to_address>
### Comment 3
<location path="internal/services/ec2/provider.go" line_range="700-709" />
<code_context>
})
}
+func (p *Provider) handleAssociateAddress(form url.Values) (*plugin.Response, error) {
+ allocationID := form.Get("AllocationId")
+ instanceID := form.Get("InstanceId")
+ if allocationID == "" || instanceID == "" {
+ return ec2XMLError("MissingParameter", "AllocationId and InstanceId are required", http.StatusBadRequest), nil
+ }
+ assocID, err := p.store.AssociateAddress(defaultAccountID, allocationID, instanceID)
+ if err != nil {
+ if errors.Is(err, ErrAddressNotFound) {
+ return ec2XMLError("InvalidAllocationID.NotFound", err.Error(), http.StatusBadRequest), nil
+ }
+ return nil, err
+ }
+ type associateAddressResponse struct {
+ XMLName xml.Name `xml:"AssociateAddressResponse"`
+ Return bool `xml:"return"`
+ AssociationId string `xml:"associationId"`
+ }
+ return ec2XMLResponse(http.StatusOK, associateAddressResponse{Return: true, AssociationId: assocID})
+}
+
</code_context>
<issue_to_address>
**question (bug_risk):** AssociateAddress does not validate that the target instance exists before associating.
In `handleAssociateAddress`, only the allocation is validated; `InstanceId` is not checked. This means associations can be created for nonexistent instances, causing inconsistent address state. Please reuse `InstancesExist` (or similar) to validate the instance and return `InvalidInstanceID.NotFound` (or equivalent) when it doesn’t exist.
</issue_to_address>
### Comment 4
<location path="tests/compatibility/test_ec2.py" line_range="276-285" />
<code_context>
+ assert not sg2.get("IpPermissions")
+
+
+def test_elastic_ip_association(ec2_client):
+ iid = ec2_client.run_instances(ImageId="ami-eip", MinCount=1, MaxCount=1)[
+ "Instances"
+ ][0]["InstanceId"]
+ alloc = ec2_client.allocate_address(Domain="vpc")
+ alloc_id = alloc["AllocationId"]
+
+ assoc = ec2_client.associate_address(AllocationId=alloc_id, InstanceId=iid)
+ assoc_id = assoc["AssociationId"]
+ assert assoc_id.startswith("eipassoc-")
+
+ addrs = ec2_client.describe_addresses()["Addresses"]
+ mine = [a for a in addrs if a["AllocationId"] == alloc_id][0]
+ assert mine["InstanceId"] == iid
+
+ ec2_client.disassociate_address(AssociationId=assoc_id)
+ ec2_client.release_address(AllocationId=alloc_id)
+ remaining = [
+ a["AllocationId"] for a in ec2_client.describe_addresses()["Addresses"]
+ ]
+ assert alloc_id not in remaining
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for Elastic IP error cases (invalid allocation/association IDs) and DescribeAddresses shape
The success path is well covered. To fully exercise the new store methods and error types, please also add tests that:
- Call `associate_address` with an invalid `AllocationId` and assert the expected error code.
- Call `disassociate_address` with an unknown `AssociationId` and assert `InvalidAssociationID.NotFound`.
- Call `release_address` with an unknown `AllocationId` and assert `InvalidAllocationID.NotFound`.
- Optionally, verify `describe_addresses()` includes the expected keys (`AssociationId`, `InstanceId`, `Domain`) for both associated and unassociated addresses.
These will cover the negative paths and help prevent regressions in error handling.
Suggested implementation:
```python
def test_elastic_ip_association(ec2_client):
iid = ec2_client.run_instances(ImageId="ami-eip", MinCount=1, MaxCount=1)[
"Instances"
][0]["InstanceId"]
alloc = ec2_client.allocate_address(Domain="vpc")
alloc_id = alloc["AllocationId"]
assoc = ec2_client.associate_address(AllocationId=alloc_id, InstanceId=iid)
assoc_id = assoc["AssociationId"]
assert assoc_id.startswith("eipassoc-")
addrs = ec2_client.describe_addresses()["Addresses"]
mine = [a for a in addrs if a["AllocationId"] == alloc_id][0]
assert mine["InstanceId"] == iid
ec2_client.disassociate_address(AssociationId=assoc_id)
ec2_client.release_address(AllocationId=alloc_id)
remaining = [
a["AllocationId"] for a in ec2_client.describe_addresses()["Addresses"]
]
assert alloc_id not in remaining
def test_elastic_ip_associate_invalid_allocation(ec2_client):
iid = ec2_client.run_instances(ImageId="ami-eip", MinCount=1, MaxCount=1)[
"Instances"
][0]["InstanceId"]
invalid_alloc_id = "eipalloc-00000000"
with pytest.raises(ClientError) as excinfo:
ec2_client.associate_address(AllocationId=invalid_alloc_id, InstanceId=iid)
err = excinfo.value.response["Error"]
assert err["Code"] == "InvalidAllocationID.NotFound"
assert invalid_alloc_id in err.get("Message", "")
def test_elastic_ip_disassociate_invalid_association(ec2_client):
invalid_assoc_id = "eipassoc-00000000"
with pytest.raises(ClientError) as excinfo:
ec2_client.disassociate_address(AssociationId=invalid_assoc_id)
err = excinfo.value.response["Error"]
assert err["Code"] == "InvalidAssociationID.NotFound"
assert invalid_assoc_id in err.get("Message", "")
def test_elastic_ip_release_invalid_allocation(ec2_client):
invalid_alloc_id = "eipalloc-00000000"
with pytest.raises(ClientError) as excinfo:
ec2_client.release_address(AllocationId=invalid_alloc_id)
err = excinfo.value.response["Error"]
assert err["Code"] == "InvalidAllocationID.NotFound"
assert invalid_alloc_id in err.get("Message", "")
def test_elastic_ip_describe_addresses_shape(ec2_client):
iid = ec2_client.run_instances(ImageId="ami-eip", MinCount=1, MaxCount=1)[
"Instances"
][0]["InstanceId"]
associated = ec2_client.allocate_address(Domain="vpc")
unassociated = ec2_client.allocate_address(Domain="vpc")
assoc_resp = ec2_client.associate_address(
AllocationId=associated["AllocationId"],
InstanceId=iid,
)
assoc_id = assoc_resp["AssociationId"]
addrs = ec2_client.describe_addresses()["Addresses"]
assoc_entry = next(
a for a in addrs if a["AllocationId"] == associated["AllocationId"]
)
unassoc_entry = next(
a for a in addrs if a["AllocationId"] == unassociated["AllocationId"]
)
# Associated address has association + instance
assert assoc_entry["Domain"] == "vpc"
assert assoc_entry["AssociationId"] == assoc_id
assert assoc_entry["InstanceId"] == iid
# Unassociated address still has expected base keys
assert unassoc_entry["Domain"] == "vpc"
assert "AllocationId" in unassoc_entry
# And it should not be associated to any instance
assert "AssociationId" not in unassoc_entry
assert "InstanceId" not in unassoc_entry
# Cleanup
ec2_client.disassociate_address(AssociationId=assoc_id)
ec2_client.release_address(AllocationId=associated["AllocationId"])
ec2_client.release_address(AllocationId=unassociated["AllocationId"])
```
1. These new tests use `pytest.raises` and `ClientError`. If not already present at the top of `tests/compatibility/test_ec2.py`, add:
- `import pytest`
- `from botocore.exceptions import ClientError`
2. If your implementation returns different error codes/messages for these cases, adjust the asserted `"Code"` strings (e.g., `"InvalidAllocationID.NotFound"`, `"InvalidAssociationID.NotFound"`) to match your backend’s behavior.
</issue_to_address>
### Comment 5
<location path="tests/compatibility/test_ec2.py" line_range="299-307" />
<code_context>
+ assert alloc_id not in remaining
+
+
+def test_delete_tags(ec2_client):
+ iid = ec2_client.run_instances(ImageId="ami-tagdel", MinCount=1, MaxCount=1)[
+ "Instances"
+ ][0]["InstanceId"]
+ ec2_client.create_tags(Resources=[iid], Tags=[{"Key": "team", "Value": "x"}])
+ ec2_client.delete_tags(Resources=[iid], Tags=[{"Key": "team"}])
+ desc = ec2_client.describe_instances(InstanceIds=[iid])
+ tags = desc["Reservations"][0]["Instances"][0].get("Tags", [])
+ assert "team" not in {t["Key"] for t in tags}
</code_context>
<issue_to_address>
**suggestion (testing):** Extend tag tests to exercise DescribeTags and delete-all-tags semantics
Since the backing store and handlers now also support `DescribeTags` and additional `DeleteTags` semantics, it would be good to add tests that:
- Call `describe_tags()` and verify tags created via `create_tags` are returned with the correct `ResourceId`, `Key`, and `Value`.
- Use `delete_tags(Resources=[...], Tags=[])` to confirm that all tags are removed from one or more resources in a single call.
This will exercise the new query logic and protect `handleDescribeTags` / `handleDeleteTags` from regressions.
</issue_to_address>
### Comment 6
<location path="tests/compatibility/test_iam_sts.py" line_range="287-296" />
<code_context>
assert not any(t["Key"] == "purpose" for t in resp2["Tags"])
+
+
+def test_group_managed_policies(iam_client):
+ iam_client.create_group(GroupName="devs")
+ arn = "arn:aws:iam::aws:policy/AmazonS3ReadOnlyAccess"
+ iam_client.attach_group_policy(GroupName="devs", PolicyArn=arn)
+ attached = iam_client.list_attached_group_policies(GroupName="devs")[
+ "AttachedPolicies"
+ ]
+ assert any(p["PolicyArn"] == arn for p in attached)
+
+ iam_client.detach_group_policy(GroupName="devs", PolicyArn=arn)
+ attached2 = iam_client.list_attached_group_policies(GroupName="devs")[
+ "AttachedPolicies"
+ ]
+ assert not any(p["PolicyArn"] == arn for p in attached2)
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding IAM group managed policy tests for error conditions (detach non-attached, missing parameters)
Since detach now returns `NoSuchEntity` for non-attached policies and validates required parameters, it would be useful to add tests that:
- Call `detach_group_policy` on a group where the policy is not attached and assert a `ClientError` with code `NoSuchEntity`.
- Optionally assert that `list_attached_group_policies` for a group with no policies returns an empty `AttachedPolicies` list.
This will exercise the new `ErrPolicyNotFound` mapping and align error handling with the user/role policy APIs.
Suggested implementation:
```python
def test_group_managed_policies(iam_client):
iam_client.create_group(GroupName="devs")
arn = "arn:aws:iam::aws:policy/AmazonS3ReadOnlyAccess"
# attach and verify
iam_client.attach_group_policy(GroupName="devs", PolicyArn=arn)
attached = iam_client.list_attached_group_policies(GroupName="devs")[
"AttachedPolicies"
]
assert any(p["PolicyArn"] == arn for p in attached)
# detach and verify no longer attached
iam_client.detach_group_policy(GroupName="devs", PolicyArn=arn)
attached_after_detach = iam_client.list_attached_group_policies(GroupName="devs")[
"AttachedPolicies"
]
assert not any(p["PolicyArn"] == arn for p in attached_after_detach)
assert attached_after_detach == []
# detaching a non-attached policy should raise NoSuchEntity
with pytest.raises(ClientError) as excinfo:
iam_client.detach_group_policy(GroupName="devs", PolicyArn=arn)
err = excinfo.value.response["Error"]
assert err["Code"] == "NoSuchEntity"
assert "policy" in err.get("Message", "").lower()
```
If `pytest` and `ClientError` are not already imported in `tests/compatibility/test_iam_sts.py`, you’ll need to:
1. Add `import pytest`.
2. Add `from botocore.exceptions import ClientError`.
</issue_to_address>
### Comment 7
<location path="tests/compatibility/test_iam_sts.py" line_range="303-312" />
<code_context>
+ assert not any(p["PolicyArn"] == arn for p in attached2)
+
+
+def test_group_inline_policies(iam_client):
+ iam_client.create_group(GroupName="ops")
+ doc = '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}'
+ iam_client.put_group_policy(
+ GroupName="ops", PolicyName="inline1", PolicyDocument=doc
+ )
+
+ names = iam_client.list_group_policies(GroupName="ops")["PolicyNames"]
+ assert "inline1" in names
+
+ got = iam_client.get_group_policy(GroupName="ops", PolicyName="inline1")
+ assert got["GroupName"] == "ops"
+ assert got["PolicyName"] == "inline1"
+
+ iam_client.delete_group_policy(GroupName="ops", PolicyName="inline1")
+ names2 = iam_client.list_group_policies(GroupName="ops")["PolicyNames"]
+ assert "inline1" not in names2
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for Get/DeleteGroupPolicy on non-existent policies and list-empty behaviour
The inline group policy tests cover the happy path well. To fully exercise the new APIs, please also add tests that:
- Call `get_group_policy` and `delete_group_policy` for a non-existent policy and assert a `ClientError` with `Error.Code == "NoSuchEntity"`.
- Optionally, for a newly created group with no inline policies, assert that `list_group_policies()` returns `PolicyNames == []`.
This will verify the negative paths and empty-list handling for inline group policies.
Suggested implementation:
```python
def test_group_inline_policies(iam_client):
iam_client.create_group(GroupName="ops")
# newly created group has no inline policies
empty = iam_client.list_group_policies(GroupName="ops")["PolicyNames"]
assert empty == []
doc = '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}'
iam_client.put_group_policy(
GroupName="ops", PolicyName="inline1", PolicyDocument=doc
)
names = iam_client.list_group_policies(GroupName="ops")["PolicyNames"]
assert "inline1" in names
got = iam_client.get_group_policy(GroupName="ops", PolicyName="inline1")
assert got["GroupName"] == "ops"
assert got["PolicyName"] == "inline1"
iam_client.delete_group_policy(GroupName="ops", PolicyName="inline1")
names2 = iam_client.list_group_policies(GroupName="ops")["PolicyNames"]
assert "inline1" not in names2
# get_group_policy on non-existent inline policy
with pytest.raises(ClientError) as excinfo_get:
iam_client.get_group_policy(GroupName="ops", PolicyName="inline1")
assert excinfo_get.value.response["Error"]["Code"] == "NoSuchEntity"
# delete_group_policy on non-existent inline policy
with pytest.raises(ClientError) as excinfo_del:
iam_client.delete_group_policy(GroupName="ops", PolicyName="inline1")
assert excinfo_del.value.response["Error"]["Code"] == "NoSuchEntity"
```
To compile and run these tests, ensure the following at the top of `tests/compatibility/test_iam_sts.py` (or in the shared test utilities if that's where they live):
1. `import pytest`
2. `from botocore.exceptions import ClientError`
If these imports are already present in the file, no further changes are needed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+405
to
+414
| for i := 1; ; i++ { | ||
| id := form.Get(fmt.Sprintf("InstanceId.%d", i)) | ||
| if id == "" { | ||
| break | ||
| } | ||
| ids = append(ids, id) | ||
| } | ||
| return ids | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
issue (bug_risk): Port parsing in parseSGRules silently treats invalid ports as 0, which may hide malformed inputs.
In parseSGRules, both structured and legacy parsing use strconv.Atoi but ignore its error, treating invalid ports as 0. This silently accepts malformed/non-numeric ports and diverges from AWS behavior. Please validate the conversion and either return an error or drop those rules so callers are aware of invalid input.
Comment on lines
+611
to
+620
| // DeleteSecurityGroup removes a security group by group ID or group name. | ||
| func (s *EC2Store) DeleteSecurityGroup(accountID, identifier string) error { | ||
| res, err := s.db().Exec( | ||
| `DELETE FROM security_groups WHERE account_id=? AND (group_id=? OR group_name=?)`, | ||
| accountID, identifier, identifier, | ||
| ) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| n, _ := res.RowsAffected() |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Deleting a security group does not clean up its rules, leaving potentially orphaned security_group_rules rows.
DeleteSecurityGroup deletes from security_groups but leaves matching security_group_rules rows in place, causing orphaned data and potential query/maintenance issues. Consider adding a cascading delete (via code or FK) or explicitly deleting related security_group_rules entries in this operation.
Comment on lines
+700
to
+709
| func (p *Provider) handleAssociateAddress(form url.Values) (*plugin.Response, error) { | ||
| allocationID := form.Get("AllocationId") | ||
| instanceID := form.Get("InstanceId") | ||
| if allocationID == "" || instanceID == "" { | ||
| return ec2XMLError("MissingParameter", "AllocationId and InstanceId are required", http.StatusBadRequest), nil | ||
| } | ||
| assocID, err := p.store.AssociateAddress(defaultAccountID, allocationID, instanceID) | ||
| if err != nil { | ||
| if errors.Is(err, ErrAddressNotFound) { | ||
| return ec2XMLError("InvalidAllocationID.NotFound", err.Error(), http.StatusBadRequest), nil |
Contributor
There was a problem hiding this comment.
question (bug_risk): AssociateAddress does not validate that the target instance exists before associating.
In handleAssociateAddress, only the allocation is validated; InstanceId is not checked. This means associations can be created for nonexistent instances, causing inconsistent address state. Please reuse InstancesExist (or similar) to validate the instance and return InvalidInstanceID.NotFound (or equivalent) when it doesn’t exist.
Comment on lines
+276
to
+285
| def test_elastic_ip_association(ec2_client): | ||
| iid = ec2_client.run_instances(ImageId="ami-eip", MinCount=1, MaxCount=1)[ | ||
| "Instances" | ||
| ][0]["InstanceId"] | ||
| alloc = ec2_client.allocate_address(Domain="vpc") | ||
| alloc_id = alloc["AllocationId"] | ||
|
|
||
| assoc = ec2_client.associate_address(AllocationId=alloc_id, InstanceId=iid) | ||
| assoc_id = assoc["AssociationId"] | ||
| assert assoc_id.startswith("eipassoc-") |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add tests for Elastic IP error cases (invalid allocation/association IDs) and DescribeAddresses shape
The success path is well covered. To fully exercise the new store methods and error types, please also add tests that:
- Call
associate_addresswith an invalidAllocationIdand assert the expected error code. - Call
disassociate_addresswith an unknownAssociationIdand assertInvalidAssociationID.NotFound. - Call
release_addresswith an unknownAllocationIdand assertInvalidAllocationID.NotFound. - Optionally, verify
describe_addresses()includes the expected keys (AssociationId,InstanceId,Domain) for both associated and unassociated addresses.
These will cover the negative paths and help prevent regressions in error handling.
Suggested implementation:
def test_elastic_ip_association(ec2_client):
iid = ec2_client.run_instances(ImageId="ami-eip", MinCount=1, MaxCount=1)[
"Instances"
][0]["InstanceId"]
alloc = ec2_client.allocate_address(Domain="vpc")
alloc_id = alloc["AllocationId"]
assoc = ec2_client.associate_address(AllocationId=alloc_id, InstanceId=iid)
assoc_id = assoc["AssociationId"]
assert assoc_id.startswith("eipassoc-")
addrs = ec2_client.describe_addresses()["Addresses"]
mine = [a for a in addrs if a["AllocationId"] == alloc_id][0]
assert mine["InstanceId"] == iid
ec2_client.disassociate_address(AssociationId=assoc_id)
ec2_client.release_address(AllocationId=alloc_id)
remaining = [
a["AllocationId"] for a in ec2_client.describe_addresses()["Addresses"]
]
assert alloc_id not in remaining
def test_elastic_ip_associate_invalid_allocation(ec2_client):
iid = ec2_client.run_instances(ImageId="ami-eip", MinCount=1, MaxCount=1)[
"Instances"
][0]["InstanceId"]
invalid_alloc_id = "eipalloc-00000000"
with pytest.raises(ClientError) as excinfo:
ec2_client.associate_address(AllocationId=invalid_alloc_id, InstanceId=iid)
err = excinfo.value.response["Error"]
assert err["Code"] == "InvalidAllocationID.NotFound"
assert invalid_alloc_id in err.get("Message", "")
def test_elastic_ip_disassociate_invalid_association(ec2_client):
invalid_assoc_id = "eipassoc-00000000"
with pytest.raises(ClientError) as excinfo:
ec2_client.disassociate_address(AssociationId=invalid_assoc_id)
err = excinfo.value.response["Error"]
assert err["Code"] == "InvalidAssociationID.NotFound"
assert invalid_assoc_id in err.get("Message", "")
def test_elastic_ip_release_invalid_allocation(ec2_client):
invalid_alloc_id = "eipalloc-00000000"
with pytest.raises(ClientError) as excinfo:
ec2_client.release_address(AllocationId=invalid_alloc_id)
err = excinfo.value.response["Error"]
assert err["Code"] == "InvalidAllocationID.NotFound"
assert invalid_alloc_id in err.get("Message", "")
def test_elastic_ip_describe_addresses_shape(ec2_client):
iid = ec2_client.run_instances(ImageId="ami-eip", MinCount=1, MaxCount=1)[
"Instances"
][0]["InstanceId"]
associated = ec2_client.allocate_address(Domain="vpc")
unassociated = ec2_client.allocate_address(Domain="vpc")
assoc_resp = ec2_client.associate_address(
AllocationId=associated["AllocationId"],
InstanceId=iid,
)
assoc_id = assoc_resp["AssociationId"]
addrs = ec2_client.describe_addresses()["Addresses"]
assoc_entry = next(
a for a in addrs if a["AllocationId"] == associated["AllocationId"]
)
unassoc_entry = next(
a for a in addrs if a["AllocationId"] == unassociated["AllocationId"]
)
# Associated address has association + instance
assert assoc_entry["Domain"] == "vpc"
assert assoc_entry["AssociationId"] == assoc_id
assert assoc_entry["InstanceId"] == iid
# Unassociated address still has expected base keys
assert unassoc_entry["Domain"] == "vpc"
assert "AllocationId" in unassoc_entry
# And it should not be associated to any instance
assert "AssociationId" not in unassoc_entry
assert "InstanceId" not in unassoc_entry
# Cleanup
ec2_client.disassociate_address(AssociationId=assoc_id)
ec2_client.release_address(AllocationId=associated["AllocationId"])
ec2_client.release_address(AllocationId=unassociated["AllocationId"])
- These new tests use
pytest.raisesandClientError. If not already present at the top oftests/compatibility/test_ec2.py, add:import pytestfrom botocore.exceptions import ClientError
- If your implementation returns different error codes/messages for these cases, adjust the asserted
"Code"strings (e.g.,"InvalidAllocationID.NotFound","InvalidAssociationID.NotFound") to match your backend’s behavior.
Comment on lines
+299
to
+307
| def test_delete_tags(ec2_client): | ||
| iid = ec2_client.run_instances(ImageId="ami-tagdel", MinCount=1, MaxCount=1)[ | ||
| "Instances" | ||
| ][0]["InstanceId"] | ||
| ec2_client.create_tags(Resources=[iid], Tags=[{"Key": "team", "Value": "x"}]) | ||
| ec2_client.delete_tags(Resources=[iid], Tags=[{"Key": "team"}]) | ||
| desc = ec2_client.describe_instances(InstanceIds=[iid]) | ||
| tags = desc["Reservations"][0]["Instances"][0].get("Tags", []) | ||
| assert "team" not in {t["Key"] for t in tags} |
Contributor
There was a problem hiding this comment.
suggestion (testing): Extend tag tests to exercise DescribeTags and delete-all-tags semantics
Since the backing store and handlers now also support DescribeTags and additional DeleteTags semantics, it would be good to add tests that:
- Call
describe_tags()and verify tags created viacreate_tagsare returned with the correctResourceId,Key, andValue. - Use
delete_tags(Resources=[...], Tags=[])to confirm that all tags are removed from one or more resources in a single call.
This will exercise the new query logic and protect handleDescribeTags / handleDeleteTags from regressions.
Comment on lines
+287
to
+296
| def test_group_managed_policies(iam_client): | ||
| iam_client.create_group(GroupName="devs") | ||
| arn = "arn:aws:iam::aws:policy/AmazonS3ReadOnlyAccess" | ||
| iam_client.attach_group_policy(GroupName="devs", PolicyArn=arn) | ||
| attached = iam_client.list_attached_group_policies(GroupName="devs")[ | ||
| "AttachedPolicies" | ||
| ] | ||
| assert any(p["PolicyArn"] == arn for p in attached) | ||
|
|
||
| iam_client.detach_group_policy(GroupName="devs", PolicyArn=arn) |
Contributor
There was a problem hiding this comment.
suggestion (testing): Consider adding IAM group managed policy tests for error conditions (detach non-attached, missing parameters)
Since detach now returns NoSuchEntity for non-attached policies and validates required parameters, it would be useful to add tests that:
- Call
detach_group_policyon a group where the policy is not attached and assert aClientErrorwith codeNoSuchEntity. - Optionally assert that
list_attached_group_policiesfor a group with no policies returns an emptyAttachedPolicieslist.
This will exercise the new ErrPolicyNotFound mapping and align error handling with the user/role policy APIs.
Suggested implementation:
def test_group_managed_policies(iam_client):
iam_client.create_group(GroupName="devs")
arn = "arn:aws:iam::aws:policy/AmazonS3ReadOnlyAccess"
# attach and verify
iam_client.attach_group_policy(GroupName="devs", PolicyArn=arn)
attached = iam_client.list_attached_group_policies(GroupName="devs")[
"AttachedPolicies"
]
assert any(p["PolicyArn"] == arn for p in attached)
# detach and verify no longer attached
iam_client.detach_group_policy(GroupName="devs", PolicyArn=arn)
attached_after_detach = iam_client.list_attached_group_policies(GroupName="devs")[
"AttachedPolicies"
]
assert not any(p["PolicyArn"] == arn for p in attached_after_detach)
assert attached_after_detach == []
# detaching a non-attached policy should raise NoSuchEntity
with pytest.raises(ClientError) as excinfo:
iam_client.detach_group_policy(GroupName="devs", PolicyArn=arn)
err = excinfo.value.response["Error"]
assert err["Code"] == "NoSuchEntity"
assert "policy" in err.get("Message", "").lower()If pytest and ClientError are not already imported in tests/compatibility/test_iam_sts.py, you’ll need to:
- Add
import pytest. - Add
from botocore.exceptions import ClientError.
Comment on lines
+303
to
+312
| def test_group_inline_policies(iam_client): | ||
| iam_client.create_group(GroupName="ops") | ||
| doc = '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}' | ||
| iam_client.put_group_policy( | ||
| GroupName="ops", PolicyName="inline1", PolicyDocument=doc | ||
| ) | ||
|
|
||
| names = iam_client.list_group_policies(GroupName="ops")["PolicyNames"] | ||
| assert "inline1" in names | ||
|
|
Contributor
There was a problem hiding this comment.
suggestion (testing): Add coverage for Get/DeleteGroupPolicy on non-existent policies and list-empty behaviour
The inline group policy tests cover the happy path well. To fully exercise the new APIs, please also add tests that:
- Call
get_group_policyanddelete_group_policyfor a non-existent policy and assert aClientErrorwithError.Code == "NoSuchEntity". - Optionally, for a newly created group with no inline policies, assert that
list_group_policies()returnsPolicyNames == [].
This will verify the negative paths and empty-list handling for inline group policies.
Suggested implementation:
def test_group_inline_policies(iam_client):
iam_client.create_group(GroupName="ops")
# newly created group has no inline policies
empty = iam_client.list_group_policies(GroupName="ops")["PolicyNames"]
assert empty == []
doc = '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}'
iam_client.put_group_policy(
GroupName="ops", PolicyName="inline1", PolicyDocument=doc
)
names = iam_client.list_group_policies(GroupName="ops")["PolicyNames"]
assert "inline1" in names
got = iam_client.get_group_policy(GroupName="ops", PolicyName="inline1")
assert got["GroupName"] == "ops"
assert got["PolicyName"] == "inline1"
iam_client.delete_group_policy(GroupName="ops", PolicyName="inline1")
names2 = iam_client.list_group_policies(GroupName="ops")["PolicyNames"]
assert "inline1" not in names2
# get_group_policy on non-existent inline policy
with pytest.raises(ClientError) as excinfo_get:
iam_client.get_group_policy(GroupName="ops", PolicyName="inline1")
assert excinfo_get.value.response["Error"]["Code"] == "NoSuchEntity"
# delete_group_policy on non-existent inline policy
with pytest.raises(ClientError) as excinfo_del:
iam_client.delete_group_policy(GroupName="ops", PolicyName="inline1")
assert excinfo_del.value.response["Error"]["Code"] == "NoSuchEntity"To compile and run these tests, ensure the following at the top of tests/compatibility/test_iam_sts.py (or in the shared test utilities if that's where they live):
import pytestfrom botocore.exceptions import ClientError
If these imports are already present in the file, no further changes are needed.
5 tasks done
skyoo2003
changed the title
Add 24 commonly-used operations that were previously unimplemented, all reusing existing SQLite-backed stores. EC2 (17): - Instance lifecycle: StartInstances, StopInstances, RebootInstances - Discovery: DescribeAvailabilityZones, DescribeRegions - Security group rules: AuthorizeSecurityGroupIngress/Egress, RevokeSecurityGroupIngress/Egress (new security_group_rules table), surfaced in DescribeSecurityGroups - Elastic IP: AssociateAddress, DisassociateAddress, ReleaseAddress, DescribeAddresses (adds an association_id column) - Cleanup: DeleteSecurityGroup, DeleteSubnet, DescribeTags, DeleteTags IAM (7) — group policy management mirroring the existing user/role code: - AttachGroupPolicy, DetachGroupPolicy, ListAttachedGroupPolicies, PutGroupPolicy, GetGroupPolicy, DeleteGroupPolicy, ListGroupPolicies (new group_policies + group_inline_policies tables) Covered by boto3 compatibility tests (suite: 729 -> 740).
skyoo2003
force-pushed
the
feat/phase1-op-depth
branch
from
July 19, 2026 13:57
0da5241 to
5330d51
Compare
skyoo2003
commented
Jul 19, 2026
| // DeleteSecurityGroup removes a security group by group ID or group name. | ||
| func (s *EC2Store) DeleteSecurityGroup(accountID, identifier string) error { | ||
| res, err := s.db().Exec( | ||
| `DELETE FROM security_groups WHERE account_id=? AND (group_id=? OR group_name=?)`, |
Owner
Author
There was a problem hiding this comment.
correctness — data loss: DeleteSecurityGroup deletes with WHERE account_id=? AND (group_id=? OR group_name=?), but group_name has no UNIQUE constraint (only the random group_id PK does). A delete-by-name removes every group sharing that name.
Trigger: create two groups named web (each gets a distinct random group_id), then delete_security_group(GroupName="web") deletes both rows in one call.
skyoo2003
commented
Jul 19, 2026
| @@ -495,19 +599,50 @@ func (p *Provider) handleDescribeSecurityGroups(_ url.Values) (*plugin.Response, | |||
| if err != nil { | |||
Owner
Author
There was a problem hiding this comment.
correctness: handleDescribeSecurityGroups(_ url.Values) discards the form, so GroupIds/Filters are ignored and every group in the account is returned. (Anchored here; the discarded call is on line 598.)
Trigger: describe_security_groups(GroupIds=["sg-missing"]) returns HTTP 200 with all groups instead of raising InvalidGroup.NotFound; a caller doing resp["SecurityGroups"][0] operates on an unrelated group. The new test even filters client-side to compensate.
skyoo2003
commented
Jul 19, 2026
| } | ||
|
|
||
| func (p *Provider) handleDescribeTags(_ url.Values) (*plugin.Response, error) { | ||
| tags, err := p.store.DescribeTags(defaultAccountID) |
Owner
Author
There was a problem hiding this comment.
correctness: handleDescribeTags(_ url.Values) drops all Filter.N args and returns every tag in the account.
Trigger: describe_tags(Filters=[{"Name":"resource-id","Values":["i-abc"]}]) returns tags for every resource, so a caller attributes other resources' tags to i-abc. Any key/value/resource-type filter is silently disregarded.
skyoo2003
commented
Jul 19, 2026
| } | ||
|
|
||
| func (p *Provider) handleDescribeAddresses(_ url.Values) (*plugin.Response, error) { | ||
| addrs, err := p.store.DescribeAddresses(defaultAccountID) |
Owner
Author
There was a problem hiding this comment.
correctness: handleDescribeAddresses(_ url.Values) ignores AllocationIds/PublicIps/Filters and returns all Elastic IPs.
Trigger: describe_addresses(AllocationIds=["eipalloc-missing"]) returns all addresses with HTTP 200 instead of raising InvalidAllocationID.NotFound; describe_addresses(PublicIps=[ip]) returns unrelated addresses.
skyoo2003
commented
Jul 19, 2026
|
|
||
| // RevokeSecurityGroupRule removes rules from a group matching the given | ||
| // direction and (protocol, fromPort, toPort, cidr) tuple. | ||
| func (s *EC2Store) RevokeSecurityGroupRule(accountID, groupID string, egress bool, protocol string, fromPort, toPort int, cidr string) error { |
Owner
Author
There was a problem hiding this comment.
correctness — false success: RevokeSecurityGroupRule never checks group or rule existence and discards the DELETE's RowsAffected, so it always returns nil and the handler returns Return: true.
Trigger: revoke_security_group_ingress(GroupId="sg-missing", ...) or revoking a never-authorized rule reports success; real AWS raises InvalidGroup.NotFound / InvalidPermission.NotFound. Note the asymmetry — handleAuthorizeSecurityGroup validates the group, Revoke does not.
skyoo2003
commented
Jul 19, 2026
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if _, err := s.db().Exec(`UPDATE instances SET state=? WHERE instance_id=? AND account_id=?`, state, id, accountID); err != nil { |
Owner
Author
There was a problem hiding this comment.
correctness — non-atomic batch: SetInstanceState updates each instance before validating the next (per-id SELECT+UPDATE, no transaction), so a batch containing a later invalid ID leaves earlier instances already transitioned while the call returns an error. Real EC2 is all-or-nothing.
Trigger: stop_instances(InstanceIds=['i-good','i-bad']) → i-good becomes stopped, then i-bad hits sql.ErrNoRows and the handler returns HTTP 400; a caller treating the 400 as "nothing changed" is wrong. Validate all IDs first (or wrap in a tx).
skyoo2003
commented
Jul 19, 2026
|
|
||
| // AssociateAddress binds an allocation to an instance and returns the new | ||
| // association ID. | ||
| func (s *EC2Store) AssociateAddress(accountID, allocationID, instanceID string) (string, error) { |
Owner
Author
There was a problem hiding this comment.
correctness: AssociateAddress binds the EIP without verifying the target instance exists.
Trigger: associate_address(AllocationId=alloc, InstanceId="i-missing") succeeds and returns an association id; a later describe_addresses shows the EIP bound to a nonexistent instance. Real AWS raises InvalidInstanceID.NotFound.
skyoo2003
commented
Jul 19, 2026
| egressInt = 1 | ||
| } | ||
| if _, err := s.db().Exec( | ||
| `INSERT INTO security_group_rules (rule_id, group_id, egress, protocol, from_port, to_port, cidr, account_id) |
Owner
Author
There was a problem hiding this comment.
correctness: AuthorizeSecurityGroupRule INSERTs a fresh rule_id on every call with no uniqueness on (group_id, egress, protocol, from_port, to_port, cidr), so re-authorizing an identical rule stores a duplicate row that handleDescribeSecurityGroups then emits twice.
Trigger: call authorize_security_group_ingress twice with {tcp,22,22,0.0.0.0/0} (client retry / idempotent reconcile). describe_security_groups returns the identical IpPermission twice; real EC2 rejects the second with InvalidPermission.Duplicate.
skyoo2003
commented
Jul 19, 2026
| } | ||
|
|
||
| func (p *Provider) handleAuthorizeSecurityGroup(form url.Values, egress bool) (*plugin.Response, error) { | ||
| groupID := form.Get("GroupId") |
Owner
Author
There was a problem hiding this comment.
correctness: handleAuthorizeSecurityGroup (and handleRevokeSecurityGroup at ~989) read only GroupId and reject GroupName, so rule changes addressed by group name fail with HTTP 400 — inconsistent with the new handleDeleteSecurityGroup (~845) which accepts either.
Trigger: in a default VPC, authorize_security_group_ingress(GroupName="web", IpPermissions=[...]) sends no GroupId, so this returns MissingParameter: GroupId is required instead of adding the rule. Same for the Egress and Revoke variants.
skyoo2003
commented
Jul 19, 2026
| if proto == "" { | ||
| break | ||
| } | ||
| from, _ := strconv.Atoi(form.Get(prefix + "FromPort")) |
Owner
Author
There was a problem hiding this comment.
correctness: parseSGRules discards strconv.Atoi errors, so a missing/invalid FromPort/ToPort silently becomes 0. Combined with the always-non-nil *int port fields in DescribeSecurityGroups (omitempty never fires), port-less rules report port 0 instead of being omitted.
Trigger: an all-traffic rule (IpProtocol '-1') sends no ports → stored as 0/0 → describe_security_groups returns FromPort=0, ToPort=0. A boto3 client asserting "FromPort" not in perm fails. Same silent-zero at the legacy flat-form parse (~953).
skyoo2003
commented
Jul 19, 2026
|
|
||
| // DeleteTags removes tags from the given resources. If keys is empty, all tags | ||
| // on each resource are removed. | ||
| func (s *EC2Store) DeleteTags(accountID string, resourceIDs, keys []string) error { |
Owner
Author
There was a problem hiding this comment.
correctness: DeleteTags matches by key only, ignoring the Tag.N.Value boto3 sends, so a value-qualified delete removes tags whose value doesn't match.
Trigger: delete_tags(Resources=[iid], Tags=[{"Key":"team","Value":"x"}]) intends to delete only if the value is x, but the handler never parses Tag.N.Value and the DELETE keys on tag_key alone — so team=y is also removed. AWS would leave it.
skyoo2003
commented
Jul 19, 2026
| for _, sg := range sgs { | ||
| items = append(items, sgItemXML{GroupId: sg.GroupID, GroupName: sg.GroupName, VpcId: sg.VpcID, Description: sg.Description}) | ||
| item := sgItemXML{GroupId: sg.GroupID, GroupName: sg.GroupName, VpcId: sg.VpcID, Description: sg.Description} | ||
| rules, err := p.store.SecurityGroupRules(defaultAccountID, sg.GroupID) |
Owner
Author
There was a problem hiding this comment.
efficiency (N+1): handleDescribeSecurityGroups runs one SecurityGroupRules query per group — 1 query for the groups plus N per-group rule queries on a hot describe path. Cheaper: one SELECT ... FROM security_group_rules WHERE account_id=? bucketed by group_id in a map.
skyoo2003
commented
Jul 19, 2026
| // AuthorizeSecurityGroupRule adds an ingress/egress rule to a security group | ||
| // and returns the new rule ID. Returns ErrSecurityGroupNotFound if the group | ||
| // does not exist. | ||
| func (s *EC2Store) AuthorizeSecurityGroupRule(accountID, groupID string, egress bool, protocol string, fromPort, toPort int, cidr string) (string, error) { |
Owner
Author
There was a problem hiding this comment.
efficiency: AuthorizeSecurityGroupRule re-runs the group-existence SELECT for every rule, so a multi-rule authorize does 2M queries (SELECT-exists + INSERT per rule) and re-validates the same group_id M times. Validate the group once in the provider before the loop (or accept all rules in one store call).
skyoo2003
commented
Jul 19, 2026
| } | ||
|
|
||
| // instanceIDsFromForm collects InstanceId.1, InstanceId.2, … from a Query form. | ||
| func instanceIDsFromForm(form url.Values) []string { |
Owner
Author
There was a problem hiding this comment.
cleanup (plausible): instanceIDsFromForm reimplements the generic collectMembers(form, prefix) helper already in the ses/autoscaling providers, and doesn't dedupe the two existing InstanceId.N loops in handleDescribeInstances (~300) and handleTerminateInstances (~361). The same indexed-param loop now lives in three places; a future change (bare InstanceId, count cap) must be made in each. An ec2 collectMembers(form, "InstanceId") would replace all three.
…ation
Addresses a systemic pattern of Describe handlers discarding client input
and store methods skipping existence/validation checks:
- Describe{SecurityGroups,Tags,Addresses} now honor GroupId/AllocationId/
PublicIp selectors and Filter.N, and raise InvalidGroup.NotFound /
InvalidAllocationID.NotFound for unknown IDs (were returning every
resource in the account)
- DeleteSecurityGroup resolves to a single group (group_name is not unique)
and removes its rules, instead of deleting every same-named group
- SetInstanceState validates all IDs first inside a transaction, so a bad
ID in a batch Start/Stop no longer half-applies (EC2 is all-or-nothing)
- AssociateAddress verifies the instance exists (InvalidInstanceID.NotFound)
- Authorize rejects duplicate rules (InvalidPermission.Duplicate); Revoke
reports unknown group/rule (InvalidGroup.NotFound / InvalidPermission.NotFound)
- Authorize/Revoke accept GroupName, resolving the group once per request
- All-traffic ('-1') rules omit FromPort/ToPort instead of reporting 0
- DeleteTags honors Tag.N.Value (value-qualified delete)
- DescribeSecurityGroups fetches rules in one batched query (was N+1)
Adds 12 boto3 regression tests covering each behavior.
…ation
orderByColumn used CAST AS REAL, which loses precision past 2^53 and on
high-precision decimals, so numeric ('N') sort/range keys could sort wrong.
Register a NUMTEXT SQLite collation (big.Rat comparison, exact across
DynamoDB's full 38-digit precision) and order by COLLATE NUMTEXT instead.
Also annotates two deliberate mock-appropriate ceilings as accepted (no
behaviour change): SQS synchronous redrive and EC2 lenient filter names.
Adds a comparator unit test and a collation-wiring test.
skyoo2003
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds operation depth for core services as part of Phase 1 (AWS Depth & Stabilization), together with correctness fixes surfaced by a follow-up review of the new EC2 code and a DynamoDB sort-key precision fix. Base branch
main.All work reuses the existing SQLite-backed stores and is covered by boto3 compatibility tests. Full suite: 760 passing.
New operations (24)
EC2 (17)
StartInstances,StopInstances,RebootInstancesDescribeAvailabilityZones,DescribeRegionsAuthorizeSecurityGroupIngress/Egress,RevokeSecurityGroupIngress/Egress(newsecurity_group_rulestable), surfaced inDescribeSecurityGroupsAssociateAddress,DisassociateAddress,ReleaseAddress,DescribeAddresses(adds anassociation_idcolumn)DeleteSecurityGroup,DeleteSubnet,DescribeTags,DeleteTagsIAM (7)
Group policy management, mirroring the existing user/role policy code:
AttachGroupPolicy,DetachGroupPolicy,ListAttachedGroupPolicies,PutGroupPolicy,GetGroupPolicy,DeleteGroupPolicy,ListGroupPolicies(newgroup_policies+group_inline_policiestables).Correctness fixes — EC2
A max-effort review of the new EC2 handlers found a systemic pattern of
Describehandlers discarding client input and store methods skipping validation. All fixed, each with a boto3 regression test (12 added):Describe{SecurityGroups,Tags,Addresses}now honorGroupId/GroupName/AllocationId/PublicIpselectors andFilter.N, and raiseInvalidGroup.NotFound/InvalidAllocationID.NotFoundfor unknown IDs — previously they ignored all input and returned every resource in the account.DeleteSecurityGroupresolves the identifier to a single group and removes its rules;group_nameis not unique, so a delete-by-name no longer wipes every same-named group (data-loss fix).StartInstances/StopInstancesvalidate every ID first inside a transaction — a bad ID in a batch no longer leaves earlier instances half-transitioned (EC2 is all-or-nothing).AssociateAddressverifies the target instance exists (InvalidInstanceID.NotFound).InvalidPermission.Duplicate); revoke reports unknown group/rule (InvalidGroup.NotFound/InvalidPermission.NotFound) instead of silently succeeding.Authorize/RevokeacceptGroupName(not justGroupId), resolving the group once per request.-1) rules omitFromPort/ToPortinstead of reporting0.DeleteTagshonorsTag.N.Value(value-qualified delete).DescribeSecurityGroupsfetches rules in one batched query instead of N+1.Correctness fixes — DynamoDB
N) sort/range keys now order by true numeric value across DynamoDB's full 38-digit precision, via aNUMTEXTSQLite collation (big.Ratcomparison). Previously aCAST AS REALlost precision past 2^53 and on high-precision decimals. Guarded by a comparator unit test and a collation-wiring test.Deliberate non-changes
Two shortcuts were reviewed and kept as correct for a mock (annotated in code):
tag:*,resource-type).Verification
go build ./...,go vet ./...,go test ./..., and the full boto3 compatibility suite (760 passed) all green.