✨ Migrate ssm code to AWS SDK v2 #5529

miyadav · 2025-06-06T06:38:10Z

What type of PR is this?

/kind feature
/kind cleanup

What this PR does / why we need it:

Migrates ssm code to AWS SDK v2
Which issue(s) this PR fixes (optional, in fixes #(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #5412

Special notes for your reviewer:

Checklist:

squashed commits
includes documentation
includes emoji in title
adds unit tests
adds or updates e2e tests

Release note:

Migrate ssm code to AWS SDK v2

k8s-ci-robot · 2025-06-06T06:38:21Z

Hi @miyadav. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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-sigs/prow repository.

miyadav · 2025-06-11T11:24:57Z

/test ?

k8s-ci-robot · 2025-06-11T11:25:12Z

@miyadav: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test ?

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-sigs/prow repository.

punkwalker · 2025-06-12T14:45:43Z

@miyadav The client implementation does not look correct. Please refer to #5497 or #5498 for examples.

nrb

Thanks for taking this on! I've left a few comments. As Punkaj suggested, review the PRs that have merged before and try to follow their structure.

Clients should go in pkg/cloud/scope/clients.go and the interfaces should go in pkg/cloud/services/<service name>/service.go.

pkg/cloud/services/ssm/ssmiface/ssm_client.go

pkg/cloud/services/ssm/ssmiface/interface.go

miyadav · 2025-06-13T06:22:31Z

updated , thanks @punkwalker , @nrb .

punkwalker

Requesting another round of changes.

punkwalker · 2025-06-25T18:39:23Z

pkg/cloud/services/ssm/service.go

+type SSMAPI interface {
+	PutParameter(ctx context.Context, input *ssm.PutParameterInput, optFns ...func(*ssm.Options)) (*ssm.PutParameterOutput, error)
+	DeleteParameter(ctx context.Context, input *ssm.DeleteParameterInput, optFns ...func(*ssm.Options)) (*ssm.DeleteParameterOutput, error)
+	GetParameter(ctx context.Context, input *ssm.GetParameterInput, optFns ...func(*ssm.Options)) (*ssm.GetParameterOutput, error)
+	// Add more methods as needed
 }

-// NewService returns a new service given the api clients.
+// SSMClientV2 is a concrete implementation of the SSMAPI interface using AWS SDK v2.
+// It wraps PutParameter for potential custom logic while using the AWS SDK directly for other methods.
+type SSMClientV2 struct {
+	Client *ssm.Client
+}
+
+// PutParameter adds or overwrites a parameter in AWS SSM Parameter Store.
+// This method is wrapped to allow for custom error handling or retry logic if needed.
+func (c *SSMClientV2) PutParameter(ctx context.Context, input *ssm.PutParameterInput, optFns ...func(*ssm.Options)) (*ssm.PutParameterOutput, error) {
+	if c.Client == nil {
+		return nil, errors.New("SSM client is not initialized")
+	}
+	return c.Client.PutParameter(ctx, input, optFns...)
+}
+
+// DeleteParameter deletes a parameter from AWS SSM Parameter Store.
+func (c *SSMClientV2) DeleteParameter(ctx context.Context, input *ssm.DeleteParameterInput, optFns ...func(*ssm.Options)) (*ssm.DeleteParameterOutput, error) {
+	return c.Client.DeleteParameter(ctx, input, optFns...)
+}
+
+// GetParameter retrieves a parameter from AWS SSM Parameter Store.
+func (c *SSMClientV2) GetParameter(ctx context.Context, input *ssm.GetParameterInput, optFns ...func(*ssm.Options)) (*ssm.GetParameterOutput, error) {
+	return c.Client.GetParameter(ctx, input, optFns...)
+}


@miyadav Any reason you still want to wrap the AWS SDK ssm client into custom struct? As I see, we do not have any custom logic per se in PutParameter , DeleteParameter or GetParameter. I feel we can do this in future if really required.

I might have missed to push the change properly , after making changes earlier , some ec2 tests were failing , reason might be incomplete changes , so this got reverted , I am working on it now .

miyadav · 2025-06-26T04:03:12Z

pkg/cloud/services/ssm/secret.go

+func isErrorRetryable(err error, retryableCodes []string) bool {
+	// Use the ParseSmithyError utility to parse the error
+	smithyErr := awserrors.ParseSmithyError(err)
+	if smithyErr == nil {
+		return false
+	}
+
+	// Get the error code from the parsed error
+	codeToCheck := smithyErr.ErrorCode()
+
+	// Compare the extracted string with your list
+	for _, code := range retryableCodes {
+		if codeToCheck == code {
+			return true // It's a match!
+		}
+	}
+	return false
+}
+


Thanks @punkwalker , but , I am using ParseSmithyError from the awserrors package in the isErrorRetryable function, my intent was to rely on the centralized logic in ParseSmithyError , let me know if I have misunderstood , your comment.

I guess I was looking at wrong commit!

1.Since ssm interface provided earlier is removed , new is now at pkg/cloud/services/ssm/service.go. 2.Modified client code for ssm client pkg/cloud/scope/clients.go. 3.In the ec2 package updated references of aws-sdk-go/ssm to point to aws-sdk-go-v2/ssm. 4.Types provided by aws-sdk-go-v2/ssm/types pacakage updated in ec2 (launchtemplate_test.go,ami.go,ami_test.go).

…dle in ssm/service as well)

… failed ,updated method signatures for tests to pass

miyadav · 2025-06-26T04:03:12Z

pkg/cloud/services/ssm/secret.go

+func isErrorRetryable(err error, retryableCodes []string) bool {
+	// Use the ParseSmithyError utility to parse the error
+	smithyErr := awserrors.ParseSmithyError(err)
+	if smithyErr == nil {
+		return false
+	}
+
+	// Get the error code from the parsed error
+	codeToCheck := smithyErr.ErrorCode()
+
+	// Compare the extracted string with your list
+	for _, code := range retryableCodes {
+		if codeToCheck == code {
+			return true // It's a match!
+		}
+	}
+	return false
+}
+


Thanks @punkwalker , but , I am using ParseSmithyError from the awserrors package in the isErrorRetryable function, my intent was to rely on the centralized logic in ParseSmithyError , let me know if I have misunderstood , your comment.

miyadav · 2025-06-26T04:56:44Z

/test pull-cluster-api-provider-aws-e2e-blocking

damdo · 2025-06-26T09:14:19Z

/test pull-cluster-api-provider-aws-build-docker

punkwalker

LGTM 🚀

damdo · 2025-06-27T06:03:30Z

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-eks

damdo · 2025-06-27T07:57:27Z

/test pull-cluster-api-provider-aws-e2e

damdo

Thanks a LOT @miyadav 🎉

All tests passed and approvers/reviewers are happy.

/lgtm

/unhold

k8s-ci-robot · 2025-06-27T12:32:10Z

LGTM label has been added.

Git tree hash: 30f2438df768fe37b39adcfe226249799ec0a67c

damdo · 2025-06-27T13:30:05Z

/test pull-cluster-api-provider-aws-build-docker

damdo · 2025-06-27T14:26:08Z

/test pull-cluster-api-provider-aws-build-docker

damdo · 2025-06-27T16:39:02Z

/test pull-cluster-api-provider-aws-build-docker

k8s-ci-robot requested review from dlipovetsky and fiunchinho June 6, 2025 06:38

miyadav force-pushed the updateawssdktov2 branch from 3173077 to a6aed6e Compare June 6, 2025 06:41

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 6, 2025

miyadav force-pushed the updateawssdktov2 branch 5 times, most recently from acf1453 to 0082989 Compare June 11, 2025 11:06

miyadav changed the title ~~WIP Migrate ssm code to AWS SDK v2~~ ✨ Migrate ssm code to AWS SDK v2 Jun 11, 2025

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 11, 2025

miyadav force-pushed the updateawssdktov2 branch from 0082989 to 4b07844 Compare June 12, 2025 04:34

nrb reviewed Jun 12, 2025

View reviewed changes

pkg/cloud/services/ssm/ssmiface/ssm_client.go Outdated Show resolved Hide resolved

pkg/cloud/services/ssm/ssmiface/interface.go Outdated Show resolved Hide resolved

miyadav force-pushed the updateawssdktov2 branch 2 times, most recently from a5f6060 to 124624c Compare June 13, 2025 06:21

miyadav force-pushed the updateawssdktov2 branch from 124624c to 6d18363 Compare June 13, 2025 07:42

punkwalker suggested changes Jun 25, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 25, 2025

miyadav added 10 commits June 26, 2025 08:47

Parameter Not found updated as per sdk-v2(smithy.APIError)

1215dba

Move the MapToSSMTag to central location,add unit test for it

534295d

making consistent in error return in tests

1e42f19

Added missing doc.go in mock_ssmiface pkg

6aec633

initialize the SSMClient properly in the EC2 service ( update to han…

e9cd900

…dle in ssm/service as well)

add custom endpoint resolver for ssm

a8ccb1f

parse smithyError code using existing code pattern

8b12956

unwrap GetParameter and DeleteParameter for ssmClient

2528914

updated ctx to be propagated , but that resulted in some tests being…

d3b8408

… failed ,updated method signatures for tests to pass

miyadav commented Jun 26, 2025

View reviewed changes

miyadav force-pushed the updateawssdktov2 branch from a01df82 to 9b0502a Compare June 26, 2025 04:05

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2025

ssm client wrapper removed

ab9a661

miyadav force-pushed the updateawssdktov2 branch from 9b0502a to ab9a661 Compare June 26, 2025 04:31

damdo requested a review from punkwalker June 26, 2025 12:31

punkwalker approved these changes Jun 26, 2025

View reviewed changes

damdo reviewed Jun 27, 2025

View reviewed changes

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 27, 2025

✨ Migrate ssm code to AWS SDK v2 #5529

Are you sure you want to change the base?

✨ Migrate ssm code to AWS SDK v2 #5529

Conversation

miyadav commented Jun 6, 2025 • edited by damdo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jun 6, 2025

Uh oh!

miyadav commented Jun 11, 2025

Uh oh!

k8s-ci-robot commented Jun 11, 2025

Uh oh!

punkwalker commented Jun 12, 2025

Uh oh!

nrb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

miyadav commented Jun 13, 2025

Uh oh!

punkwalker left a comment

Choose a reason for hiding this comment

Uh oh!

punkwalker Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

miyadav Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

miyadav Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

punkwalker Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

miyadav Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

miyadav commented Jun 26, 2025

Uh oh!

damdo commented Jun 26, 2025

Uh oh!

punkwalker left a comment

Choose a reason for hiding this comment

Uh oh!

damdo commented Jun 27, 2025

Uh oh!

damdo commented Jun 27, 2025

Uh oh!

damdo left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jun 27, 2025

Uh oh!

damdo commented Jun 27, 2025

Uh oh!

damdo commented Jun 27, 2025

Uh oh!

damdo commented Jun 27, 2025

Uh oh!

Uh oh!

miyadav commented Jun 6, 2025 •

edited by damdo

Loading