feat(sdk): Add per-service SDK tag fallback for model fetching by sapphirew · Pull Request #690 · aws-controllers-k8s/code-generator

sapphirew · 2026-04-16T19:01:55Z

Issue #, if available:
aws-controllers-k8s/community#2859
Description of changes:

When a new AWS service is released, its model JSON is published under a per-service tag (e.g. service/s3files/v1.0.0) before the next core SDK tag is cut. This adds automatic fallback to per-service tags when the core tag returns HTTP 404.

Changes:

Extend EnsureModel with per-service URL fallback and separate cache
Add --aws-service-sdk-version CLI flag to ack-generate
Add version resolution priority chain (flag > metadata > empty)
Record aws_service_sdk_version in ack-generate-metadata.yaml
Add AWS_SERVICE_SDK_VERSION passthrough in build-controller.sh
Add unit tests for fallback, cache separation, URL construction, metadata round-trip, and EnsureSemverPrefix idempotence

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

sapphirew · 2026-04-16T21:29:33Z

/retest

knottnt

Thanks @sapphirew. Left some comments. Mostly questions about how we want to use the new flag.

knottnt · 2026-04-16T23:58:42Z

+
+	// Core tag returned non-200 (likely 404)
+	if coreStatus == http.StatusNotFound && serviceSDKVersion != "" {
+		// Fallback to per-service tag


Q: If the per-service-tag is able to represent a newer version of the api model file should it take precedence?

Agreed. The per-service API models are always more up to date until the core sdk catches up in a few weeks. It should take precedence when set

knottnt · 2026-04-17T00:01:33Z

+//
 // The returned string is the base path to use with NewHelper — it mirrors the
 // SDK repo directory structure so that ModelAndDocsPath works unchanged.
 func EnsureModel(


p: having the logic for checking both the core and per-service versions is making this function quite a bit more complex. Wondering if it would be possible to make the decision on whether or not to use the core vs per-service version outside of this function and base the parameters we pass into this function on that decision.

I think we can always use per-service version when set. If core sdk version is set at the same time, return an error or warn

knottnt · 2026-04-17T00:03:45Z

 	rootCmd.PersistentFlags().StringVar(
 		&optAWSSDKGoVersion, "aws-sdk-go-version", "", "Version of github.com/aws/aws-sdk-go-v2 used to generate apis and controllers files",
 	)
+	rootCmd.PersistentFlags().StringVar(


Q: Should this flag be mutually exclusive with aws-sdk-go-version? What are the scenarios where we'd want both to be set?

Makes sense. If core sdk version is set at the same time, we can return an error

When a new AWS service is released, its model JSON is published under a per-service tag (e.g. service/s3files/v1.0.0) before the next core SDK tag is cut. This adds automatic fallback to per-service tags when the core tag returns HTTP 404. Changes: - Extend EnsureModel with per-service URL fallback and separate cache - Add --aws-service-sdk-version CLI flag to ack-generate - Add version resolution priority chain (flag > metadata > empty) - Record aws_service_sdk_version in ack-generate-metadata.yaml - Add AWS_SERVICE_SDK_VERSION passthrough in build-controller.sh - Add unit tests for fallback, cache separation, URL construction, metadata round-trip, and EnsureSemverPrefix idempotence

knottnt

Thanks @sapphirew! Left a few more comments.

knottnt · 2026-04-17T23:22:57Z

+
+// TestGenerationMetadata_BackwardCompatibility verifies that YAML without the
+// aws_service_sdk_version field deserializes without error and the field
+// defaults to empty string (Requirement 4.3).


nit: (Requirement 4.3). doesn't refer to anything.

Suggested change

// defaults to empty string (Requirement 4.3).

// defaults to empty string.

knottnt · 2026-04-17T23:24:40Z

+				LastModification: lastModificationInfo{
+					Reason: UpdateReasonAPIGeneration,
+				},
+				AWSSDKGoVersion:      "v1.41.5",


It would be good to also test that AWSSDKGoVersion is emitted as expected.

knottnt · 2026-04-17T23:44:19Z

+	totalStart time.Time,
+) (string, error) {
+	normalizedSvcVer := EnsureSemverPrefix(serviceSDKVersion)
+	basePath := filepath.Join(cacheDir, "models", "service", modelName, normalizedSvcVer)


Doing some testing modelName doesn't appear to work as-is for all controllers. For example the route53-controller uses route-53 as the model name, but the service package directory uses route53. Using svcAlias here instead may be a better general fit. Going forward we may also need to add a new generator.yaml sdk_names:pkg_name config similar to the sdk_names:model_name config that's causing route53's issue.

https://github.com/aws-controllers-k8s/route53-controller/blob/6b66359fac93b613ef69b68ac6e185137b9247f6/generator.yaml#L23

knottnt · 2026-04-17T23:55:47Z

 		optGenVersion,
 		filepath.Join(optOutputPath, "apis"),
 		ackmetadata.UpdateReasonAPIGeneration,
-		optAWSSDKGoVersion,


Testing this PR against the s3-controller I see that after calling make build-controller SERVICE=s3 AWS_SERVICE_SDK_VERSION=v1.99.1 the ack-generate-metadata.yaml file looks like the below and includes both aws_sdk_go_version and aws_service_sdk_version. Should only one of these fields be emitted?

ack_generate_info: build_date: "2026-03-31T19:59:08Z" build_hash: a9e2ceaadfc00a742e2ea2b6d6c68348f03e52a5 build_date: "2026-04-17T23:51:02Z" build_hash: 2f10e7b367cd01230a7916e4956c61f97be8cf13 go_version: go1.26.1 version: v0.58.0-3-ga9e2cea api_directory_checksum: 379f9aa07359a58fabb591e795271b44a678b20c version: v0.58.0-5-g2f10e7b api_directory_checksum: 29a8845f7c3bf83c6d20107a582b35d7a912d93a api_version: v1alpha1 aws_sdk_go_version: v1.41.5 aws_service_sdk_version: v1.99.1 generator_config_info: file_checksum: 035386df2e486fac01a2dbedf247c36bbaeec7f3 original_file_name: generator.yaml

…k-tags

- Use the service package name (not the model name) as the per-service tag path segment so controllers whose model name differs from their SDK package dir (e.g. route53: model 'route-53' vs package 'route53') resolve correctly. Adds resolveServicePackageName helper backed by sdk_names.package_name. - Record exactly one version field in ack-generate-metadata.yaml: when a per-service SDK version drives the fetch, the core aws_sdk_go_version is no longer resolved or emitted (and vice versa). - Drop stale '(Requirement 4.3)' reference in metadata test comment. - Assert aws_sdk_go_version is emitted in the metadata round-trip test. - Cover route53-style package/model name mismatch and empty-package fallback in the per-service URL construction test.

knottnt · 2026-06-24T17:39:27Z

/retest

knottnt · 2026-06-24T19:06:21Z

/retest

knottnt · 2026-06-24T23:49:48Z

/lgtm

knottnt · 2026-06-24T23:50:14Z

/approve

ack-prow · 2026-06-24T23:50:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: knottnt, sapphirew

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~~ [knottnt]

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

ack-prow · 2026-06-25T00:17:50Z

@sapphirew: 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
dynamodb-controller-test	`88ed930`	link	unknown	`/test dynamodb-controller-test`

Full PR test history. Your PR dashboard.

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

knottnt · 2026-06-25T00:38:07Z

/retest

ack-prow Bot requested review from jlbutler and knottnt April 16, 2026 19:02

knottnt reviewed Apr 17, 2026

View reviewed changes

sapphirew force-pushed the feat/per-service-sdk-tags branch from 74e60d6 to 2f10e7b Compare April 17, 2026 17:22

knottnt self-requested a review April 17, 2026 22:41

knottnt reviewed Apr 17, 2026

View reviewed changes

knottnt added 2 commits June 23, 2026 10:45

Merge remote-tracking branch 'upstream/main' into feat/per-service-sd…

eabfd9d

…k-tags

ack-prow Bot assigned knottnt Jun 24, 2026

ack-prow Bot added lgtm Indicates that a PR is ready to be merged. approved labels Jun 24, 2026

	// defaults to empty string (Requirement 4.3).
	// defaults to empty string.

Uh oh!

Conversation

sapphirew commented Apr 16, 2026

Uh oh!

sapphirew commented Apr 16, 2026

Uh oh!

knottnt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knottnt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knottnt commented Jun 24, 2026

Uh oh!

knottnt commented Jun 24, 2026

Uh oh!

knottnt commented Jun 24, 2026

Uh oh!

knottnt commented Jun 24, 2026

Uh oh!

ack-prow Bot commented Jun 24, 2026

Uh oh!

ack-prow Bot commented Jun 25, 2026

Uh oh!

knottnt commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants