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

templates/hooks/managed_prefix_list/sdk_read_many_post_build_request.go.tpl

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

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.

alexcristi · 2025-11-05T14:27:50Z

/retest

alexcristi · 2025-11-05T14:58:50Z

/retest

alexcristi · 2025-11-05T18:17:27Z

/retest

knottnt · 2025-11-05T19:12:10Z

/test ec2-kind-e2e

ack-prow · 2025-11-05T21:35:53Z

@alexcristi: The following tests 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	`49339bd`	link	false	`/test ec2-verify-code-gen`
ec2-kind-e2e	`49339bd`	link	true	`/test ec2-kind-e2e`

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.

knottnt · 2025-11-05T20:46:50Z

helm/templates/_helpers.tpl

Sorry I missed this earlier, but checking the git diff from the ec2-verify-code-gen CI job it looks like the Helmrole-writer.yaml, role-reader.yaml, and helm/values.yalm files haven't been updated to include the managedprefixlists in resources list. These files should normally be updated when running make build-controller SERVICE=ec2 in the code-generator repo.

diff --git a/helm/templates/role-reader.yaml b/helm/templates/role-reader.yaml index 1bab6b2..cfe2b49 100644 --- a/helm/templates/role-reader.yaml +++ b/helm/templates/role-reader.yaml @@ -23,6 +23,7 @@ rules: - instances - internetgateways - launchtemplates + - managedprefixlists - natgateways - networkacls - routetables diff --git a/helm/templates/role-writer.yaml b/helm/templates/role-writer.yaml index 162cc85..ba9465f 100644 --- a/helm/templates/role-writer.yaml +++ b/helm/templates/role-writer.yaml @@ -23,6 +23,7 @@ rules: - instances - internetgateways - launchtemplates + - managedprefixlists - natgateways - networkacls - routetables @@ -52,6 +53,7 @@ rules: - instances - internetgateways - launchtemplates + - managedprefixlists - natgateways - networkacls - routetables diff --git a/helm/values.yaml b/helm/values.yaml index beac9dc..ad35f4c 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -152,6 +152,7 @@ reconcile: - Instance - InternetGateway - LaunchTemplate + - ManagedPrefixList - NATGateway - NetworkACL - RouteTable

knottnt · 2025-11-05T21:42:23Z

helm/crds/ec2.services.k8s.aws_managedprefixlists.yaml

+      type: string
+    - jsonPath: .status.version
+      name: VERSION
+      type: string


Double checking the verify-code-gen job I noticed that there are some differences in the ManagedPrefixList CRD being generated by that ProwJob. Tested building locally with controller-gen v0.16.2 and I still see these diffs (minus the controller-gen.kubebuilder.io/version annotation). Normally, changes to CRD field descriptions come from using a different version of the AWS SDK. Are you overriding this value when building the controller?

diff --git a/helm/crds/ec2.services.k8s.aws_managedprefixlists.yaml b/helm/crds/ec2.services.k8s.aws_managedprefixlists.yaml index ce097b8..c6b3b53 100644 --- a/helm/crds/ec2.services.k8s.aws_managedprefixlists.yaml +++ b/helm/crds/ec2.services.k8s.aws_managedprefixlists.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.16.2 + controller-gen.kubebuilder.io/version: v0.19.0 name: managedprefixlists.ec2.services.k8s.aws spec: group: ec2.services.k8s.aws @@ -26,7 +26,7 @@ spec: type: string - jsonPath: .status.version name: VERSION - type: string + type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -57,12 +57,11 @@ spec: properties: addressFamily: description: |- - The IP address version. + The IP address type. Valid Values: IPv4 | IPv6 type: string entries: - description: One or more entries for the prefix list. items: description: An entry for a prefix list. properties: @@ -80,8 +79,7 @@ spec: description: |- A name for the prefix list. - Constraints: Up to 255 characters in length. The name cannot start with - com.amazonaws. + Constraints: Up to 255 characters in length. The name cannot start with com.amazonaws. type: string tags: description: |- @@ -173,13 +171,13 @@ spec: description: The ID of the owner of the prefix list. type: string prefixListARN: - description: The Amazon Resource Name (ARN) of the prefix list. + description: The Amazon Resource Name (ARN) for the prefix list. type: string prefixListID: description: The ID of the prefix list. type: string state: - description: The state of the prefix list. + description: The current state of the prefix list. type: string stateMessage: description: The state message.

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

Integrate CR feedback - use prefixListID as the only filter

49339bd

knottnt reviewed Nov 5, 2025

View reviewed changes

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexcristi 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!

alexcristi commented Nov 5, 2025

Uh oh!

alexcristi commented Nov 5, 2025

Uh oh!

alexcristi commented Nov 5, 2025

Uh oh!

knottnt commented Nov 5, 2025

Uh oh!

ack-prow bot commented Nov 5, 2025

Uh oh!

knottnt Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

knottnt Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alexcristi commented Oct 29, 2025 •

edited

Loading

knottnt commented Nov 3, 2025 •

edited

Loading