Skip to content

feat: Add ResourceShareAccepter CRD for accepting RAM share invita…#43

Open
rhysxevans wants to merge 2 commits into
aws-controllers-k8s:mainfrom
rhysxevans:ram_share_acceptor
Open

feat: Add ResourceShareAccepter CRD for accepting RAM share invita…#43
rhysxevans wants to merge 2 commits into
aws-controllers-k8s:mainfrom
rhysxevans:ram_share_acceptor

Conversation

@rhysxevans

Copy link
Copy Markdown

…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:

  • 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

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.

   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
@ack-prow ack-prow Bot requested review from a-hilaly and michaelhtm February 15, 2026 18:51
@ack-prow

ack-prow Bot commented Feb 15, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rhysxevans
Once this PR has been reviewed and has the lgtm label, please assign a-hilaly for approval by writing /assign @a-hilaly in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-prow ack-prow Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 15, 2026
@ack-prow

ack-prow Bot commented Feb 15, 2026

Copy link
Copy Markdown

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions 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 michaelhtm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @rhysxevans
Should we consider making ResourceShareAccepter a functionality of the ResourceShare CRD?
Similar to what we do with EKS AccessPolicies

@rhysxevans

Copy link
Copy Markdown
Author

Hi

Thanks for reviewing, I am ultimately happy to take direction from you.

I do however feel that there may be a subtle difference

  1. AccessPolicies are attached to AccessEntries, in the same account.
  2. ResourceShare's are created in one account and shared to another (assuming not in the same org), and the ResourceShareAccepter is done in the "other" account. This is particularly relevant where the "sharer" is a 3rd party saas provider.

I don't know if my interpretation is in any way relevant though.

Let me know your thoughts

Thanks

rhysxevans added a commit to rhysxevans/ram-controller that referenced this pull request Feb 17, 2026
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.
@rhysxevans

Copy link
Copy Markdown
Author

FYI: @michaelhtm I have created #44 as a response to your comment #43 (review)

@michaelhtm

Copy link
Copy Markdown
Member
  1. AccessPolicies are attached to AccessEntries, in the same account.
  2. ResourceShare's are created in one account and shared to another (assuming not in the same org), and the ResourceShareAccepter is done in the "other" account. This is particularly relevant where the "sharer" is a 3rd party saas provider.

You are right, thanks for clarifying the differences.
After a closer look, i do see the need for an accepter, but i wonder if ACK is equipped to support this.
I noticed you have not used the code-generator.

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

@a-hilaly @knottnt any thoughts?

@rhysxevans

Copy link
Copy Markdown
Author

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

@rhysxevans

Copy link
Copy Markdown
Author

Hi

@michaelhtm @a-hilaly @knottnt any thoughts ?

@michaelhtm

Copy link
Copy Markdown
Member

Hey @rhysxevans
All ACK controllers are generated by code-generator. This one seems like it wasn't. Can you try using generator.yaml to introduce the resource? You can use this PR as an example: aws-controllers-k8s/route53resolver-controller#65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants