feat: Add ResourceShareAccepter CRD for accepting RAM share invita…#43
feat: Add ResourceShareAccepter CRD for accepting RAM share invita…#43rhysxevans wants to merge 2 commits into
Conversation
Implements a custom ACK resource to accept AWS RAM (Resource Access Manager)
share invitations. This addresses the limitation that the generated controller
only supports creating outgoing shares, not accepting incoming invitations.
Implementation details:
- Created ResourceShareAccepter CRD following Terraform's aws_ram_resource_share_accepter pattern
- Maps non-standard AWS RAM operations to Kubernetes resource lifecycle:
* Create: Find pending invitation by ShareARN and accept it
* Read: Get invitation status from AWS
* Update: No-op (invitations are immutable)
* Delete: Disassociate from the share (leave it)
- Implements full ACK resource manager interface with custom SDK operations
- Uses finalizer-based resource management for proper lifecycle handling
- Includes Helm chart integration for easy deployment
New CRD: ResourceShareAccepter (ram.services.k8s.aws/v1alpha1)
Spec:
- shareARN: ARN of the resource share to accept
Status:
- invitationARN: ARN of the accepted invitation
- invitationStatus: Current status (PENDING, ACCEPTED, REJECTED, EXPIRED)
- senderAccountID: Account that sent the invitation
- receiverAccountID: Account receiving the invitation
- shareName: Name of the resource share
Files changed:
- apis/v1alpha1/resource_share_invitation.go: CRD type definitions
- pkg/resource/resource_share_invitation/: Complete resource implementation
- config/crd/bases/ram.services.k8s.aws_resourceshareaccepters.yaml: Generated CRD
- helm/crds/ram.services.k8s.aws_resourceshareaccepters.yaml: Helm CRD
- cmd/controller/main.go: Register ResourceShareAccepter resource
- helm/values.yaml: Add ResourceShareAccepter to reconcile resources
- config/crd/kustomization.yaml: Include new CRD in kustomization
Testing:
- Verified controller builds successfully
- Tested locally with AWS profile authentication
- Confirmed resource reconciliation and error handling
- Validated CRD generation and Helm integration
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rhysxevans The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @rhysxevans. Thanks for your PR. I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
…eAccepter
This commit fixes two critical issues with the ResourceShareAccepter controller:
1. Terminal Error Handling
- Wrap "no pending invitation" error with ackerr.NewTerminalError() to prevent
infinite retry loops when no invitation exists
- Add updateConditions() method to properly detect and handle Terminal/Recoverable errors
- Add terminalAWSError() method to identify AWS errors that should be terminal
(e.g., MalformedArnException)
- Update onError() in manager.go to call updateConditions() and return ackerr.Terminal
when Terminal condition is set
2. Status Population
- Add setStatusDefaults() method to initialize resource status metadata
- Call setStatusDefaults() in sdkCreate() before setting invitation-specific fields
- Call setStatusDefaults() in sdkFind() before processing invitations
- Ensures ackResourceMetadata (region, ownerAccountID, ARN) and conditions array
are properly initialized
3. Test Infrastructure
- Add e2e integration test for ResourceShareAccepter
- Test validates Terminal condition is set when no pending invitation exists
- Add helper functions for working with RAM share invitations
- Add test documentation and resource templates
Changes to pkg/resource/resource_share_invitation/sdk.go:
- Import corev1 for condition handling
- Wrap "no pending invitation" error with ackerr.NewTerminalError() (line 127)
- Add setStatusDefaults() method (lines 254-267)
- Add updateConditions() method (lines 270-346)
- Add terminalAWSError() method (lines 349-364)
- Call setStatusDefaults() in sdkFind() (line 72)
- Call setStatusDefaults() in sdkCreate() (line 147)
Changes to pkg/resource/resource_share_invitation/manager.go:
- Import ackerr package
- Replace onError() method to properly handle Terminal conditions (lines 185-207)
Test files added:
- test/e2e/tests/test_resource_share_accepter.py
- test/e2e/ram_resource_share_accepter.py
- test/e2e/resources/ram_resource_share_accepter.yaml
- test/e2e/README.md
Fixes:
- Controller no longer retries indefinitely when no invitation exists
- Kubernetes resource status is now fully populated with all fields
- Terminal errors are properly detected and stop reconciliation
- Integration tests validate expected behavior
michaelhtm
left a comment
There was a problem hiding this comment.
Thanks for the contribution @rhysxevans
Should we consider making ResourceShareAccepter a functionality of the ResourceShare CRD?
Similar to what we do with EKS AccessPolicies
|
Hi Thanks for reviewing, I am ultimately happy to take direction from you. I do however feel that there may be a subtle difference
I don't know if my interpretation is in any way relevant though. Let me know your thoughts Thanks |
This commit integrates resource share invitation acceptance functionality into the existing ResourceShare CRD, enabling dual-mode operation for cross-account AWS RAM sharing scenarios. ## Key Changes ### 1. Dual-Mode ResourceShare CRD - Added acceptInvitation field to enable Accept Mode - Made name field optional (required in Create Mode, not needed in Accept Mode) - Added invitation-specific status fields (invitationARN, invitationStatus, etc.) - Added resources field to status for tracking shared resources ### 2. Accept Mode Implementation - Implemented sdkAcceptInvitation() for accepting pending invitations - Implemented sdkFindAcceptedInvitation() for finding existing accepted invitations - Modified sdkCreate() to route to acceptance logic when acceptInvitation is set - Modified sdkFind() to route to invitation lookup in Accept Mode - Added validation to ensure name is provided in Create Mode ### 3. Deletion Handling - Implemented sdkRejectOrLeaveShare() for proper cleanup in Accept Mode - PENDING invitations: Rejected using RejectResourceShareInvitation - ACCEPTED invitations: Left using DisassociateResourceShare - Added graceful handling for OperationNotPermittedException (some resource types like Network Firewall don't support self-disassociation) ### 4. Deprecated API Replacement - Removed usage of deprecated ResourceShareAssociations field - Implemented populateInvitationResources() using ListPendingInvitationResources API - Added logging and metrics for resource population - Handles invitations with no resources gracefully ### 5. Documentation - Added comprehensive function-level documentation for Accept Mode - Documented AWS resource type limitations (Network Firewall, etc.) - Explained deletion behavior and OperationNotPermittedException handling - Added inline comments for error handling logic ### 6. E2E Tests - Created test_resource_share_accept_mode.py with Accept Mode tests - Added test for name validation in Create Mode - Added test templates for Accept Mode scenarios - Updated ram_resource_share.py with invitation helper functions ### 7. Examples - Added resource-share-integrated-modes.yaml with usage examples - Documented Create Mode and Accept Mode workflows - Included cross-account sharing examples ## Testing - ✅ Accept invitation with resources (Network Firewall) - ✅ Resources populated in status using new API - ✅ Deletion with OperationNotPermittedException handling - ✅ Name validation in Create Mode - ✅ All status fields populated correctly ## AWS Limitations Handled Some AWS resource types (e.g., Network Firewall) do not support self-disassociation. For these resources, the Kubernetes resource is deleted successfully, but the AWS invitation remains ACCEPTED until the share owner removes the principal. This is expected AWS behavior and is handled gracefully with appropriate logging. Addresses reviewer feedback on PR aws-controllers-k8s#43 regarding integrated vs separate CRD approach.
|
FYI: @michaelhtm I have created #44 as a response to your comment #43 (review) |
You are right, thanks for clarifying the differences. Another quirk i see is, an Accepter is not necessarily a resource, so it feels weird to have a CRD for it, although i do see Terraform does support it |
|
Hi, yep, apologies around not using the code generator. Once we have a direction of travel, I will look at re-implementing via the code generator |
|
Hi @michaelhtm @a-hilaly @knottnt any thoughts ? |
|
Hey @rhysxevans |
…tions
NOTE: This code was AI generated
Implements a custom ACK resource to accept AWS RAM (Resource Access Manager)
share invitations. This addresses the limitation that the generated controller
only supports creating outgoing shares, not accepting incoming invitations.
Implementation details:
Issue #, if available: aws-controllers-k8s/community#2786
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.