feat: adds a DescribeSecurityGroups queryType - #12
Merged
Merged
Conversation
…y types Adds TestEc2APIError (403 UnauthorizedOperation surfaces wrapped with the operation name and AWS error code) and table-drives TestEc2RegionGuard over every EC2 describe, including DescribeSecurityGroups. respStub gains an optional HTTP status.
ytsarev
approved these changes
Sep 25, 2026
ytsarev
left a comment
Member
There was a problem hiding this comment.
LGTM - additive, no behaviour change to existing query types.
Verified on the CI-pinned go1.25.10:
go vet,go test ./...andgolangci-lint v2.8.0clean (0 issues)go generate ./...produces no diff, so the input CRD is in sync withinput.go- the
describeSecurityGroupshandler follows the sharedec2Clientguard / paginator pattern; the projection test pins that inlineipPermissionsstay dropped
Pushed 8b9353d to close test gaps:
TestEc2APIError: a 403UnauthorizedOperationsurfaces as<Op> failed ... UnauthorizedOperationfor all four EC2 describes (the previously uncovered error branch)TestEc2RegionGuardnow table-driven over every EC2 describe and asserts the actualrequires a regionmessage, sinceSGsNoRegionin the guards table only assertederr != nil- a shared
ec2QueryTypeslist, so the next EC2 describe picks up the region, filter and error tests from one place
The four EC2 handlers other than subnets are now at 100% coverage.
Nits, non-blocking:
- The docs say
DescribeSecurityGroupRuleserrors onvpc-idonly where the region holds rules and otherwise returns[]. That is observed AWS behaviour, not documented behaviour, so it could change without notice. - The example XRD no longer uses preserve-unknown-fields on
status, so anyone who copies it must declare each newtarget. The inline comment covers this.
stevendborrelli
approved these changes
Sep 25, 2026
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
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.
Description of your changes
Adds a
DescribeSecurityGroupsqueryType.Why
DescribeSecurityGroupRulescannot filter by VPC:A rules query therefore needs a
group-idthat nothing produced from a VPC.Tags do not help for the canonical case, a VPC's default group: AWS creates it untagged and unreferenced.
What
ec2Clientguard (region and filters required). ProjectsgroupId,groupName,securityGroupArn,description,vpcId,ownerId,tags{}.IpPermissions/IpPermissionsEgressare dropped on purpose.x-kubernetes-preserve-unknown-fieldsfrom the XRDstatusobject, so everytarget: status.<field>inexample/fails apply withfield not declared in schema. All 11 status targets and 3*Refspec fields now declared.Additive: a new enum value plus docs, no existing queryType changes behaviour.
Tests
on the CI-pinned
go1.25.10:go test=>ok 0.836sgolangci-lint@v2.8.0=>0 issues.Verified live on a throwaway VPC with 56 groups, through the built package on kind running
crossplane:v2.4.1-up.1: all 56 cross-checked field by field againstaws ec2 describe-security-groupswith zero mismatches.Chaining
groupIdintoDescribeSecurityGroupRulesreturned 6 rules withsecurityGroupRuleIdpopulated.The empty-filter, bad-filter-name and missing-
ec2:DescribeSecurityGroupsguards each surface on the XR.c9f954eA/B'd on Crossplanev1.20.13andv2.4.1-up.1: both drop the flag and fail identically before, both land all 11 status fields after