Add support for ManagedPrefixLists in EC2 controller #295

alexcristi · 2025-10-29T10:04:49Z

Issue #, if available: aws-controllers-k8s/community#2665

Description of changes:

Added support for ManagedPrefixLists. This was validated with a KIND local cluster, and here are the results:

Controller logs

$ kubectl -n ack-system logs ack-ec2-controller-6f9ff4897-nxlpb | grep prefix
{"level":"info","ts":"2025-10-28T10:56:18.991Z","msg":"Starting EventSource","controller":"managedprefixlist","controllerGroup":"ec2.services.k8s.aws","controllerKind":"ManagedPrefixList","source":"kind source: *v1alpha1.ManagedPrefixList"}
{"level":"info","ts":"2025-10-28T10:56:19.193Z","msg":"Starting Controller","controller":"managedprefixlist","controllerGroup":"ec2.services.k8s.aws","controllerKind":"ManagedPrefixList"}
{"level":"info","ts":"2025-10-28T10:56:19.193Z","msg":"Starting workers","controller":"managedprefixlist","controllerGroup":"ec2.services.k8s.aws","controllerKind":"ManagedPrefixList","worker count":1}

ManagedPrefixList status

$ kubectl get ManagedPrefixLists
NAME                      ID                     NAME                   STATE             VERSION
ack-managed-prefix-list   pl-0e02e7c2410655ae5   ack-final-test-11184   create-complete   1

apiVersion: ec2.services.k8s.aws/v1alpha1
kind: ManagedPrefixList
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"ec2.services.k8s.aws/v1alpha1","kind":"ManagedPrefixList","metadata":{"annotations":{},"name":"ack-managed-prefix-list","namespace":"default"},"spec":{"addressFamily":"IPv4","entries":[{"cidr":"10.250.0.0/24","description":"ACK Created - Network 1"},{"cidr":"10.250.1.0/24","description":"ACK Created - Network 2"},{"cidr":"10.250.2.0/24","description":"ACK Created - Network 3"}],"maxEntries":10,"prefixListName":"ack-final-test-11184","tags":[{"key":"Environment","value":"production"},{"key":"ManagedBy","value":"ACK-Controller"},{"key":"Purpose","value":"Demo"}]}}
  creationTimestamp: "2025-10-28T10:54:38Z"
  finalizers:
  - finalizers.ec2.services.k8s.aws/ManagedPrefixList
  generation: 1
  name: ack-managed-prefix-list
  namespace: default
  resourceVersion: "3836"
  uid: 16681e81-40cd-4137-ad0f-ff06ed0ec060
spec:
  addressFamily: IPv4
  entries:
  - cidr: 10.250.0.0/24
    description: ACK Created - Network 1
  - cidr: 10.250.1.0/24
    description: ACK Created - Network 2
  - cidr: 10.250.2.0/24
    description: ACK Created - Network 3
  maxEntries: 10
  prefixListName: ack-final-test-11184
  tags:
  - key: ManagedBy
    value: ACK-Controller
  - key: Purpose
    value: Demo
status:
  ackResourceMetadata:
    ownerAccountID: "258057316678"
    region: us-west-2
  conditions:
  - lastTransitionTime: "2025-10-28T10:56:20Z"
    message: Resource synced successfully
    reason: ""
    status: "True"
    type: ACK.ResourceSynced
  ownerID: "258057316678"
  prefixListARN: arn:aws:ec2:us-west-2:258057316678:prefix-list/pl-0e02e7c2410655ae5
  prefixListID: pl-0e02e7c2410655ae5
  state: create-complete
  version: 1

ManagedPrefixList created in AWS

ManagedPrefixList updated

ManagedPrefixList deleted

ack-prow · 2025-10-29T10:05:00Z

Hi @alexcristi. 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.

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.

ack-prow · 2025-10-29T10:05:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: alexcristi
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.

Needs approval from an approver in each of these files:

OWNERS

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

a-hilaly · 2025-10-29T17:32:55Z

/ok-to-test

knottnt · 2025-10-29T17:57:43Z

@alexcristi: It looks like there's some compilation issues with these changes. Any chance there's some un-pushed commits?

 32.93 pkg/resource/managed_prefix_list/sdk.go:179:23: invalid operation: elem.State != nil (mismatched types "github.com/aws/aws-sdk-go-v2/service/ec2/types".PrefixListState and untyped nil)
32.93 pkg/resource/managed_prefix_list/sdk.go:180:25: cannot use elem.State (variable of string type "github.com/aws/aws-sdk-go-v2/service/ec2/types".PrefixListState) as *string value in assignment
32.93 pkg/resource/managed_prefix_list/sdk.go:195:28: cannot use elem.MaxEntries (variable of type *int32) as *int64 value in assignment
32.93 pkg/resource/managed_prefix_list/sdk.go:380:31: invalid operation: resp.PrefixList.State != nil (mismatched types "github.com/aws/aws-sdk-go-v2/service/ec2/types".PrefixListState and untyped nil)
32.93 pkg/resource/managed_prefix_list/sdk.go:381:22: cannot use resp.PrefixList.State (variable of string type "github.com/aws/aws-sdk-go-v2/service/ec2/types".PrefixListState) as *string value in assignment
32.93 pkg/resource/managed_prefix_list/sdk.go:665:22: cannot use desired.ko.Spec.MaxEntries (variable of type *int64) as *int32 value in assignment
32.93 pkg/resource/managed_prefix_list/sdk.go:746:32: invalid operation: resp.PrefixList.State != nil (mismatched types "github.com/aws/aws-sdk-go-v2/service/ec2/types".PrefixListState and untyped nil)
32.93 pkg/resource/managed_prefix_list/sdk.go:747:31: cannot use resp.PrefixList.State (variable of string type "github.com/aws/aws-sdk-go-v2/service/ec2/types".PrefixListState) as *string value in assignment
32.93 pkg/resource/managed_prefix_list/sdk.go:757:16: rm.syncTags undefined (type *resourceManager has no field or method syncTags)
32.93 pkg/resource/managed_prefix_list/sdk.go:757:16: too many errors

alexcristi · 2025-10-29T19:16:32Z

Thank you for taking time to look at this @knottnt, really appreciate it!
I see what happened, when I built it locally for testing, I fixed the errors directly in the sdk.go file, and when I ran the generator again of course my changes were overwritten. Now fixed it in the template files.

alexcristi · 2025-10-29T20:09:06Z

Hold on, I need to test it again

apis/v1alpha1/generator.yaml

test/e2e/tests/test_managed_prefix_list.py

templates/hooks/managed_prefix_list/sdk_file_end.go.tpl

templates/hooks/managed_prefix_list/sdk_update_pre_build_request.go.tpl

knottnt · 2025-10-30T18:46:20Z

templates/hooks/managed_prefix_list/sdk_read_many_post_build_request.go.tpl

+		// If we don't have an ID yet, filter by both owner ID and prefix list name
+		// This prevents matching against AWS-managed prefix lists
+		// and other user-owned prefix lists with different names
+		filters := []svcsdktypes.Filter{


Q: Is this combination guaranteed to be unique?

No, it is not, the only primary key is prefixListID. If there is already a prefix list in the target account, we should either adopt it or change the prefix list name in our manifest - I know, this is a "new contract" we artificially introduce so that the prefix list names must be unique; and yes, the controller will error out with the "resource already exists" message.

However, if we rely only on prefix list ID as the filter, how do we prevent resource leaks/resource storm in case of AWS issues and we don't get a response back, or the response is malformed/missing the ID? Would you point me on how you guys solved it for different resources, please? Can we use UUID tags, for example?

Looking at the other EC2 resources we check are currently checking if the resource has an ID set in the status and if that hasn't been set yet we are assuming it is a new resource. You can see an example of this in the DHCP Options. Generally, we don't want to enforce ACK specific contracts on top of the AWS Service API.

For the case of never hearing back from AWS I don't think we currently have a good solution for that if it happens during the Create operation. Right now, the controller will see a timeout failure of the Create api call and will retry on the next reconciliation loop. Solving the issue will likely require one or more improvements to the ACK runtime since we'll need to store information across reconciliation loops and potentially handle recovery when a "leaked" create is detected. Coming up with a general solution across a service/resource is also difficult since support for tagging, filtering by tag, and idempotency tokens varies between services and by resources within services.

In this case storing a UUID as a tag and then filtering the DescribeManagedPrefixLists response by that tag could work since that API does return tags for the resource. However, we'd need to add a way to store/retrieve this the UUID across reconciliation loops which seems out of scope for this PR.

cc: @a-hilaly @rushmash91 @michaelhtm

templates/hooks/managed_prefix_list/sdk_create_post_set_output.go.tpl

test/e2e/tests/test_managed_prefix_list.py

pkg/resource/managed_prefix_list/hooks.go

alexcristi · 2025-10-31T15:33:55Z

/retest

ack-prow · 2025-10-31T18:50:30Z

@alexcristi: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ec2-verify-code-gen	`3c199d8`	link	false	`/test ec2-verify-code-gen`

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

alexcristi · 2025-11-03T09:28:53Z

@knottnt - Also, what do you want me to do with ec2-verify-code-gen? Do you want me to rebuild with v0.19.0? If I do so, are we okay with committing changes on other resources than the one this PR is purposed for?

knottnt · 2025-11-03T18:58:17Z

@knottnt - Also, what do you want me to do with ec2-verify-code-gen? Do you want me to rebuild with v0.19.0? If I do so, are we okay with committing changes on other resources than the one this PR is purposed for?

@alexcristi That's a new optional PR check that we're testing out. Right now it's just informational for the reviewer. Since, the ec2-controller hasn't been rebuilt with v0.19.0 yet I'm fine with keeping it as-is and applying that change in a future PR. But no there wouldn't be an issue with you using v0.19.0 as long as the change to the other CRDs is limited to the controller-gen.kubebuilder.io/version annotation.

Add support for ManagedPrefixLists in EC2 controller

a4aedfe

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 Oct 29, 2025

ack-prow bot requested review from jlbutler and rushmash91 October 29, 2025 10:05

ack-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 29, 2025

Fix templates

c935c52

alexcristi added 3 commits October 30, 2025 11:15

Fix filter logic

b0d3a73

Update integration tests

9a98f17

Update integration tests - fix status assertion too early

b79b571

knottnt reviewed Oct 30, 2025

View reviewed changes

alexcristi added 2 commits October 31, 2025 01:01

Integrate CR feedback - integration tests update

3485247

Wait on condition ACK.ResourceSynced on update test

b470b10

knottnt reviewed Oct 31, 2025

View reviewed changes

pkg/resource/managed_prefix_list/hooks.go Outdated Show resolved Hide resolved

Integrate CR feedback - code cleanup

f551a86

Integrate CR feedback - tags

3c199d8

Add support for ManagedPrefixLists in EC2 controller #295

Are you sure you want to change the base?

Add support for ManagedPrefixLists in EC2 controller #295

Uh oh!

Conversation

alexcristi commented Oct 29, 2025

Uh oh!

ack-prow bot commented Oct 29, 2025

Uh oh!

ack-prow bot commented Oct 29, 2025

Uh oh!

a-hilaly commented Oct 29, 2025

Uh oh!

knottnt commented Oct 29, 2025

Uh oh!

alexcristi commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexcristi commented Oct 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knottnt Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

alexcristi Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knottnt Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexcristi commented Oct 31, 2025

Uh oh!

ack-prow bot commented Oct 31, 2025

Uh oh!

alexcristi commented Nov 3, 2025

Uh oh!

knottnt commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alexcristi commented Oct 29, 2025 •

edited

Loading

alexcristi Oct 31, 2025 •

edited

Loading

knottnt commented Nov 3, 2025 •

edited

Loading