Skip to content

Fix incorrect shape name in bad default filtering#671

Merged
ack-prow[bot] merged 4 commits intoaws-controllers-k8s:mainfrom
gustavodiaz7722:main
Mar 4, 2026
Merged

Fix incorrect shape name in bad default filtering#671
ack-prow[bot] merged 4 commits intoaws-controllers-k8s:mainfrom
gustavodiaz7722:main

Conversation

@gustavodiaz7722
Copy link
Contributor

Issue #, if available:

Description of changes:

Fix incorrect shape name in bad default filtering

Issue

The EMR Serverless controller was failing to build with type errors like:

cannot use &f9valiter.WorkerCount (value of type **int64) as *int64 value in assignment


Root Cause

AWS SDK Go v2 has a RemoveDefaults customization that removes default values from certain shapes where the default is incompatible with the shape's range constraints. This makes those fields nillable (pointers) in the generated SDK code.

The ACK code generator had the wrong shape name in its BadDefaultsAssignment map:

  • Before: "WorkerCounts": true (the Smithy shape name)
  • After: "WorkerCount": true (the actual member name used in Go structs)

Reference: AWS SDK Go v2 Source


Changes

  • pkg/apiv2/remove_defaults.go: Fixed the member name from WorkerCounts to WorkerCount to match the actual struct field name in the SDK.
  • pkg/generate/code/set_sdk.go: Fixed map value type generation to use the original shape name for SDK struct types when available.

Testing

  • EMR Serverless controller now builds successfully.
  • Generated code correctly handles WorkerCount as a pointer type (*int64).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ack-prow ack-prow bot requested review from jlbutler and michaelhtm February 27, 2026 07:14
@knottnt
Copy link
Contributor

knottnt commented Feb 27, 2026

/retest

@gustavodiaz7722
Copy link
Contributor Author

/test ec2-controller-test

1 similar comment
@gustavodiaz7722
Copy link
Contributor Author

/test ec2-controller-test

Copy link
Member

@michaelhtm michaelhtm left a comment

Choose a reason for hiding this comment

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

nice work @gustavodiaz7722
left one nit below

@ack-prow ack-prow bot added the approved label Mar 2, 2026
@gustavodiaz7722
Copy link
Contributor Author

/test ec2-controller-test

2 similar comments
@gustavodiaz7722
Copy link
Contributor Author

/test ec2-controller-test

@gustavodiaz7722
Copy link
Contributor Author

/test ec2-controller-test

Copy link
Contributor

@knottnt knottnt left a comment

Choose a reason for hiding this comment

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

@gustavodiaz7722 Functional change and tests look good, just need to move them to be with the existing set_sdk test cases.

Copy link
Contributor

@knottnt knottnt left a comment

Choose a reason for hiding this comment

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

/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 2, 2026
@knottnt
Copy link
Contributor

knottnt commented Mar 2, 2026

/retest

@gustavodiaz7722
Copy link
Contributor Author

Test are failing due to known issue. Pending fix here #669

@knottnt
Copy link
Contributor

knottnt commented Mar 3, 2026

/test s3-controller-test

@knottnt
Copy link
Contributor

knottnt commented Mar 3, 2026

/test ec2-controller-test

Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

/lgtm
/retest

@ack-prow
Copy link

ack-prow bot commented Mar 3, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, gustavodiaz7722, knottnt, michaelhtm

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

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [a-hilaly,knottnt,michaelhtm]

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

@knottnt
Copy link
Contributor

knottnt commented Mar 3, 2026

/test s3-controller-test

@gustavodiaz7722
Copy link
Contributor Author

/retest

3 similar comments
@knottnt
Copy link
Contributor

knottnt commented Mar 3, 2026

/retest

@gustavodiaz7722
Copy link
Contributor Author

/retest

@a-hilaly
Copy link
Member

a-hilaly commented Mar 4, 2026

/retest

@ack-prow ack-prow bot merged commit 35caa24 into aws-controllers-k8s:main Mar 4, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants