diff --git a/cmd/ack-generate/command/apis.go b/cmd/ack-generate/command/apis.go index dace0a91..4ef110e0 100644 --- a/cmd/ack-generate/command/apis.go +++ b/cmd/ack-generate/command/apis.go @@ -64,7 +64,8 @@ func saveGeneratedMetadata(cmd *cobra.Command, args []string) error { optGenVersion, filepath.Join(optOutputPath, "apis"), ackmetadata.UpdateReasonAPIGeneration, - optAWSSDKGoVersion, + sdkVersion, + svcSDKVersion, optGeneratorConfigPath, ) if err != nil { diff --git a/cmd/ack-generate/command/common.go b/cmd/ack-generate/command/common.go index b98f1f13..8b3a3ae5 100644 --- a/cmd/ack-generate/command/common.go +++ b/cmd/ack-generate/command/common.go @@ -16,6 +16,7 @@ package command import ( "context" "fmt" + "path/filepath" "sort" "strings" "time" @@ -31,6 +32,10 @@ import ( "github.com/aws-controllers-k8s/code-generator/pkg/util" ) +// svcSDKVersion holds the resolved per-service SDK version for use by +// saveGeneratedMetadata. +var svcSDKVersion string + // resolveModelName returns the SDK model name for a service, checking the // generator config for an override. func resolveModelName(svcAlias string, cfg ackgenconfig.Config) string { @@ -115,24 +120,65 @@ func getServiceAccountName() (string, error) { // setupGenerator loads the generator configuration, resolves the SDK version and fetches the // model file func setupGenerator(svcAlias string) (ackgenconfig.Config, error) { + var cfg ackgenconfig.Config + + // Mutual exclusivity: both explicit CLI flags is an error + if optAWSSDKGoVersion != "" && optAWSServiceSDKVersion != "" { + return cfg, fmt.Errorf( + "--aws-sdk-go-version and --aws-service-sdk-version are mutually exclusive; provide only one", + ) + } + // Load generator config to resolve model name before fetching cfg, err := ackgenconfig.New(optGeneratorConfigPath, ackgenerate.DefaultConfig) if err != nil { return cfg, err } + // Load existing generation metadata (used for both per-service and core + // version resolution fallbacks). + var metadataSvcSDKVersion string + var metadataCoreSDKVersion string + existingMetadata, err := ackmetadata.LoadGenerationMetadata( + filepath.Join(optOutputPath, "apis"), optGenVersion, + ) + if err != nil { + return cfg, fmt.Errorf("cannot load existing generation metadata: %v", err) + } + if existingMetadata != nil { + metadataSvcSDKVersion = existingMetadata.AWSServiceSDKVersion + metadataCoreSDKVersion = existingMetadata.AWSSDKGoVersion + } + + // Resolve per-service SDK version from priority chain: + // CLI flag → metadata YAML → empty + svcSDKVersion = sdk.GetServiceSDKVersion(optAWSServiceSDKVersion, metadataSvcSDKVersion) + // Resolve SDK version and fetch the model file fetchStart := time.Now() - resolvedVersion, err := sdk.GetSDKVersion(optAWSSDKGoVersion, "", optOutputPath) + + // When a per-service SDK version is set, the core version is still needed + // for the sdkVersion variable (used by metadata saving and other callers), + // but it is resolved from metadata/go.mod as a fallback — not as the + // primary fetch source. A resolution failure is non-fatal when the + // per-service version drives the EnsureModel fetch strategy. + resolvedVersion, err := sdk.GetSDKVersion(optAWSSDKGoVersion, metadataCoreSDKVersion, optOutputPath) if err != nil { - return cfg, err + if svcSDKVersion == "" { + return cfg, err + } + // Per-service version is set; core version is best-effort. + resolvedVersion = "" + } + if resolvedVersion != "" { + resolvedVersion = sdk.EnsureSemverPrefix(resolvedVersion) } - resolvedVersion = sdk.EnsureSemverPrefix(resolvedVersion) modelName := resolveModelName(svcAlias, cfg) + ctx, cancel := sdk.ContextWithSigterm(context.Background()) defer cancel() - basePath, err := sdk.EnsureModel(ctx, optCacheDir, resolvedVersion, modelName) + basePath, err := sdk.EnsureModel(ctx, optCacheDir, resolvedVersion, modelName, svcSDKVersion) if err != nil { return cfg, err } diff --git a/cmd/ack-generate/command/crossplane.go b/cmd/ack-generate/command/crossplane.go index 555baa77..cce7367b 100644 --- a/cmd/ack-generate/command/crossplane.go +++ b/cmd/ack-generate/command/crossplane.go @@ -69,7 +69,7 @@ func generateCrossplane(_ *cobra.Command, args []string) error { modelName := resolveModelName(svcAlias, cfg) ctx, cancel := sdk.ContextWithSigterm(context.Background()) defer cancel() - basePath, err := sdk.EnsureModel(ctx, optCacheDir, resolvedVersion, modelName) + basePath, err := sdk.EnsureModel(ctx, optCacheDir, resolvedVersion, modelName, "") if err != nil { return err } diff --git a/cmd/ack-generate/command/root.go b/cmd/ack-generate/command/root.go index 5013a6c8..1ffe512b 100644 --- a/cmd/ack-generate/command/root.go +++ b/cmd/ack-generate/command/root.go @@ -33,6 +33,7 @@ var ( defaultCacheDir string optCacheDir string optAWSSDKGoVersion string + optAWSServiceSDKVersion string defaultTemplateDirs []string optTemplateDirs []string defaultServicesDir string @@ -124,6 +125,9 @@ func init() { 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( + &optAWSServiceSDKVersion, "aws-service-sdk-version", "", "Per-service SDK version (e.g. v1.0.0) for fetching model from per-service tag", + ) rootCmd.PersistentFlags().StringVar( &optServiceAccountName, "service-account-name", "", "The name of the ServiceAccount used for ACK service controller", ) diff --git a/pkg/metadata/generation_metadata.go b/pkg/metadata/generation_metadata.go index 5d6dc7f1..4e35b9ec 100644 --- a/pkg/metadata/generation_metadata.go +++ b/pkg/metadata/generation_metadata.go @@ -59,7 +59,9 @@ type GenerationMetadata struct { // Last modification reason LastModification lastModificationInfo `json:"last_modification"` // AWS SDK Go version used generate the APIs - AWSSDKGoVersion string `json:"aws_sdk_go_version"` + AWSSDKGoVersion string `json:"aws_sdk_go_version,omitempty"` + // Per-service AWS SDK version used to fetch the model from a per-service tag + AWSServiceSDKVersion string `json:"aws_service_sdk_version,omitempty"` // Information about the ack-generate binary used to generate the APIs ACKGenerateInfo ackGenerateInfo `json:"ack_generate_info"` // Information about the generator config file used to generate the APIs @@ -86,6 +88,25 @@ type lastModificationInfo struct { Reason UpdateReason `json:"reason"` } +// LoadGenerationMetadata reads and parses an ack-generate-metadata.yaml file +// from the given API version directory. Returns nil (not error) when the file +// does not exist, since metadata may not yet have been generated. +func LoadGenerationMetadata(apisPath string, apiVersion string) (*GenerationMetadata, error) { + metadataPath := filepath.Join(apisPath, apiVersion, outputFileName) + content, err := ioutil.ReadFile(metadataPath) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + gm := &GenerationMetadata{} + if err = yaml.Unmarshal(content, gm); err != nil { + return nil, err + } + return gm, nil +} + // CreateGenerationMetadata gathers information about the generated code and save // a yaml version in the API version directory func CreateGenerationMetadata( @@ -93,6 +114,7 @@ func CreateGenerationMetadata( apisPath string, modificationReason UpdateReason, awsSDKGo string, + awsServiceSDKVersion string, generatorFileName string, ) error { filesDirectory := filepath.Join(apisPath, apiVersion) @@ -112,7 +134,8 @@ func CreateGenerationMetadata( LastModification: lastModificationInfo{ Reason: modificationReason, }, - AWSSDKGoVersion: awsSDKGo, + AWSSDKGoVersion: awsSDKGo, + AWSServiceSDKVersion: awsServiceSDKVersion, ACKGenerateInfo: ackGenerateInfo{ Version: version.Version, BuildDate: version.BuildDate, diff --git a/pkg/metadata/generation_metadata_test.go b/pkg/metadata/generation_metadata_test.go new file mode 100644 index 00000000..fa432f86 --- /dev/null +++ b/pkg/metadata/generation_metadata_test.go @@ -0,0 +1,124 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package metadata + +import ( + "strings" + "testing" + + "github.com/ghodss/yaml" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestGenerationMetadata_RoundTrip validates that serializing GenerationMetadata +// to YAML and deserializing back produces identical field values, including the +// aws_service_sdk_version field when present and its absence when empty. +func TestGenerationMetadata_RoundTrip(t *testing.T) { + tests := []struct { + name string + awsServiceSDKVersion string + expectFieldInYAML bool + }{ + { + name: "with per-service SDK version", + awsServiceSDKVersion: "v1.0.0", + expectFieldInYAML: true, + }, + { + name: "without per-service SDK version", + awsServiceSDKVersion: "", + expectFieldInYAML: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + original := GenerationMetadata{ + APIVersion: "v1alpha1", + APIDirectoryChecksum: "abc123", + LastModification: lastModificationInfo{ + Reason: UpdateReasonAPIGeneration, + }, + AWSSDKGoVersion: "v1.41.5", + AWSServiceSDKVersion: tc.awsServiceSDKVersion, + ACKGenerateInfo: ackGenerateInfo{ + Version: "v0.58.0", + GoVersion: "go1.25.3", + BuildDate: "2026-04-14T18:02:39Z", + BuildHash: "a9e2cea", + }, + GeneratorConfigInfo: generatorConfigInfo{ + OriginalFileName: "generator.yaml", + FileChecksum: "5b522171", + }, + } + + data, err := yaml.Marshal(original) + require.NoError(t, err) + + // Verify the aws_service_sdk_version field presence/absence in raw YAML + yamlStr := string(data) + if tc.expectFieldInYAML { + assert.True(t, strings.Contains(yamlStr, "aws_service_sdk_version"), + "expected aws_service_sdk_version in YAML output") + } else { + assert.False(t, strings.Contains(yamlStr, "aws_service_sdk_version"), + "expected aws_service_sdk_version to be omitted from YAML output") + } + + // Unmarshal back and verify round-trip equality + var restored GenerationMetadata + err = yaml.Unmarshal(data, &restored) + require.NoError(t, err) + + assert.Equal(t, original.APIVersion, restored.APIVersion) + assert.Equal(t, original.APIDirectoryChecksum, restored.APIDirectoryChecksum) + assert.Equal(t, original.LastModification, restored.LastModification) + assert.Equal(t, original.AWSSDKGoVersion, restored.AWSSDKGoVersion) + assert.Equal(t, original.AWSServiceSDKVersion, restored.AWSServiceSDKVersion) + assert.Equal(t, original.ACKGenerateInfo, restored.ACKGenerateInfo) + assert.Equal(t, original.GeneratorConfigInfo, restored.GeneratorConfigInfo) + }) + } +} + +// 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). +func TestGenerationMetadata_BackwardCompatibility(t *testing.T) { + // YAML that predates the aws_service_sdk_version field + oldYAML := ` +api_version: v1alpha1 +api_directory_checksum: abc123 +aws_sdk_go_version: v1.41.5 +last_modification: + reason: API generation +ack_generate_info: + version: v0.58.0 + go_version: go1.25.3 + build_date: "2026-04-14T18:02:39Z" + build_hash: a9e2cea +generator_config_info: + original_file_name: generator.yaml + file_checksum: "5b522171" +` + var gm GenerationMetadata + err := yaml.Unmarshal([]byte(oldYAML), &gm) + require.NoError(t, err) + + assert.Equal(t, "v1alpha1", gm.APIVersion) + assert.Equal(t, "v1.41.5", gm.AWSSDKGoVersion) + assert.Equal(t, "", gm.AWSServiceSDKVersion, "missing field should default to empty string") +} diff --git a/pkg/model/multiversion/manager.go b/pkg/model/multiversion/manager.go index 313eed4b..2dd3359a 100644 --- a/pkg/model/multiversion/manager.go +++ b/pkg/model/multiversion/manager.go @@ -93,7 +93,7 @@ func NewAPIVersionManager( } basePath, err := acksdk.EnsureModel( context.Background(), sdkCacheDir, - acksdk.EnsureSemverPrefix(apiInfo.AWSSDKVersion), modelName, + acksdk.EnsureSemverPrefix(apiInfo.AWSSDKVersion), modelName, "", ) if err != nil { return nil, err diff --git a/pkg/sdk/fetch.go b/pkg/sdk/fetch.go index 4c99b804..26f93153 100644 --- a/pkg/sdk/fetch.go +++ b/pkg/sdk/fetch.go @@ -26,14 +26,22 @@ import ( ) const ( - sdkModelURLTemplate = "https://raw.githubusercontent.com/aws/aws-sdk-go-v2/%s/codegen/sdk-codegen/aws-models/%s.json" - defaultHTTPFetchTimeout = 60 * time.Second + sdkModelURLTemplate = "https://raw.githubusercontent.com/aws/aws-sdk-go-v2/%s/codegen/sdk-codegen/aws-models/%s.json" + perServiceModelURLTemplate = "https://raw.githubusercontent.com/aws/aws-sdk-go-v2/service/%s/%s/codegen/sdk-codegen/aws-models/%s.json" + defaultHTTPFetchTimeout = 60 * time.Second ) // EnsureModel ensures that we have a locally-cached copy of the AWS SDK model // JSON file for a given service and SDK version. If the file is already cached, // it returns immediately. Otherwise, it fetches the file from GitHub. // +// The fetch strategy is determined by serviceSDKVersion: +// - When serviceSDKVersion is non-empty: check per-service cache, then fetch +// per-service tag URL. On non-200, return an error — no core fallback. +// - When serviceSDKVersion is empty: check core cache, then fetch core tag +// URL. On 404, suggest --aws-service-sdk-version. On other non-200, return +// an error with the URL and status code. +// // 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( @@ -41,71 +49,171 @@ func EnsureModel( cacheDir string, sdkVersion string, modelName string, + serviceSDKVersion string, ) (string, error) { totalStart := time.Now() + if serviceSDKVersion != "" { + return ensureModelPerService(ctx, cacheDir, modelName, serviceSDKVersion, totalStart) + } + return ensureModelCore(ctx, cacheDir, sdkVersion, modelName, totalStart) +} + +// ensureModelPerService handles the per-service-only fetch path. +// It checks the per-service cache first, then fetches from the per-service tag +// URL. It never falls back to the core SDK tag. +func ensureModelPerService( + ctx context.Context, + cacheDir string, + modelName string, + serviceSDKVersion string, + totalStart time.Time, +) (string, error) { + normalizedSvcVer := EnsureSemverPrefix(serviceSDKVersion) + basePath := filepath.Join(cacheDir, "models", "service", modelName, normalizedSvcVer) + modelDir := filepath.Join(basePath, "codegen", "sdk-codegen", "aws-models") + modelPath := filepath.Join(modelDir, fmt.Sprintf("%s.json", modelName)) + + // Check per-service cache + if _, err := os.Stat(modelPath); err == nil { + util.Tracef("EnsureModel: per-service cache hit for %s@%s\n", modelName, normalizedSvcVer) + return basePath, nil + } + + // Create the per-service cache directory + if err := os.MkdirAll(modelDir, os.ModePerm); err != nil { + return "", fmt.Errorf("cannot create per-service model cache directory %s: %v", modelDir, err) + } + + // Fetch from GitHub (per-service tag) + perSvcURL := fmt.Sprintf(perServiceModelURLTemplate, modelName, normalizedSvcVer, modelName) + util.Tracef("EnsureModel: fetching %s\n", perSvcURL) + + fetchStart := time.Now() + status, body, err := httpGet(ctx, perSvcURL) + if err != nil { + return "", fmt.Errorf("cannot fetch model file from %s: %v", perSvcURL, err) + } + + if status == http.StatusOK { + if err := writeModelToCache(modelDir, modelName, body); err != nil { + return "", err + } + util.Tracef("EnsureModel: fetched %s from per-service tag (%s)\n", modelName, time.Since(fetchStart)) + util.Tracef("EnsureModel total: %s\n", time.Since(totalStart)) + return basePath, nil + } + + // Non-200 — return error with URL and status code, no core fallback + return "", fmt.Errorf("failed to fetch model file from %s: HTTP %d", perSvcURL, status) +} + +// ensureModelCore handles the core-only fetch path. +// It checks the core cache first, then fetches from the core tag URL. +// On 404 it suggests --aws-service-sdk-version; on other non-200 it returns +// an error with the URL and status code. +func ensureModelCore( + ctx context.Context, + cacheDir string, + sdkVersion string, + modelName string, + totalStart time.Time, +) (string, error) { basePath := filepath.Join(cacheDir, "models", sdkVersion) modelDir := filepath.Join(basePath, "codegen", "sdk-codegen", "aws-models") modelPath := filepath.Join(modelDir, fmt.Sprintf("%s.json", modelName)) - // Check cache first + // Check core cache if _, err := os.Stat(modelPath); err == nil { util.Tracef("EnsureModel: cache hit for %s@%s\n", modelName, sdkVersion) return basePath, nil } - // Create the cache directory + // Create the core cache directory if err := os.MkdirAll(modelDir, os.ModePerm); err != nil { return "", fmt.Errorf("cannot create model cache directory %s: %v", modelDir, err) } - // Fetch from GitHub - url := fmt.Sprintf(sdkModelURLTemplate, sdkVersion, modelName) - util.Tracef("EnsureModel: fetching %s\n", url) + // Fetch from GitHub (core tag) + coreURL := fmt.Sprintf(sdkModelURLTemplate, sdkVersion, modelName) + util.Tracef("EnsureModel: fetching %s\n", coreURL) fetchStart := time.Now() + status, body, err := httpGet(ctx, coreURL) + if err != nil { + return "", fmt.Errorf("cannot fetch model file from %s: %v", coreURL, err) + } + + if status == http.StatusOK { + if err := writeModelToCache(modelDir, modelName, body); err != nil { + return "", err + } + util.Tracef("EnsureModel: fetched %s (%s)\n", modelName, time.Since(fetchStart)) + util.Tracef("EnsureModel total: %s\n", time.Since(totalStart)) + return basePath, nil + } + + if status == http.StatusNotFound { + return "", fmt.Errorf( + "model %s not found at core tag %s; to use a per-service tag, provide --aws-service-sdk-version", + modelName, sdkVersion, + ) + } + + // Non-404 error from core URL + return "", fmt.Errorf("failed to fetch model file from %s: HTTP %d", coreURL, status) +} + +// httpGet performs an HTTP GET request and returns the status code, response body, +// and any error. The caller is responsible for the body bytes. +func httpGet(ctx context.Context, url string) (int, []byte, error) { ctx, cancel := context.WithTimeout(ctx, defaultHTTPFetchTimeout) defer cancel() req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { - return "", fmt.Errorf("cannot create HTTP request: %v", err) + return 0, nil, fmt.Errorf("cannot create HTTP request: %v", err) } resp, err := http.DefaultClient.Do(req) if err != nil { - return "", fmt.Errorf("cannot fetch model file from %s: %v", url, err) + return 0, nil, err } defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("failed to fetch model file from %s: HTTP %d", url, resp.StatusCode) + body, err := io.ReadAll(resp.Body) + if err != nil { + return resp.StatusCode, nil, fmt.Errorf("cannot read response body: %v", err) } - // Write to a temp file, then rename atomically into the cache path + return resp.StatusCode, body, nil +} + +// writeModelToCache writes model data to the cache directory atomically. +func writeModelToCache(modelDir string, modelName string, data []byte) error { + modelPath := filepath.Join(modelDir, fmt.Sprintf("%s.json", modelName)) + tmpFile, err := os.CreateTemp(modelDir, ".tmp-"+modelName+"-*.json") if err != nil { - return "", fmt.Errorf("cannot create temp file: %v", err) + return fmt.Errorf("cannot create temp file: %v", err) } tmpPath := tmpFile.Name() - _, err = io.Copy(tmpFile, resp.Body) + _, err = tmpFile.Write(data) closeErr := tmpFile.Close() if err != nil { os.Remove(tmpPath) - return "", fmt.Errorf("cannot write model file: %v", err) + return fmt.Errorf("cannot write model file: %v", err) } if closeErr != nil { os.Remove(tmpPath) - return "", fmt.Errorf("cannot close temp file: %v", closeErr) + return fmt.Errorf("cannot close temp file: %v", closeErr) } if err := os.Rename(tmpPath, modelPath); err != nil { os.Remove(tmpPath) - return "", fmt.Errorf("cannot rename temp file to cache path: %v", err) + return fmt.Errorf("cannot rename temp file to cache path: %v", err) } - util.Tracef("EnsureModel: fetched %s (%s)\n", modelName, time.Since(fetchStart)) - util.Tracef("EnsureModel total: %s\n", time.Since(totalStart)) - return basePath, nil + return nil } diff --git a/pkg/sdk/fetch_test.go b/pkg/sdk/fetch_test.go new file mode 100644 index 00000000..d2b477be --- /dev/null +++ b/pkg/sdk/fetch_test.go @@ -0,0 +1,420 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package sdk_test + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/aws-controllers-k8s/code-generator/pkg/sdk" +) + +// newTestServer creates an httptest server that routes requests based on path. +// The handler map keys are URL paths; values are (statusCode, body) pairs. +func newTestServer(routes map[string]struct { + status int + body string +}) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if route, ok := routes[r.URL.Path]; ok { + w.WriteHeader(route.status) + fmt.Fprint(w, route.body) + return + } + w.WriteHeader(http.StatusNotFound) + fmt.Fprint(w, "not found") + })) +} + +// patchURLTemplates temporarily overrides the URL templates used by EnsureModel +// by replacing http.DefaultClient's Transport to rewrite requests to the test +// server. This approach avoids modifying the production code's constants. +// +// Returns a cleanup function that restores the original transport. +func patchTransport(testServerURL string) func() { + original := http.DefaultClient.Transport + http.DefaultClient.Transport = &rewriteTransport{ + base: original, + testServerURL: testServerURL, + } + return func() { + http.DefaultClient.Transport = original + } +} + +// rewriteTransport rewrites GitHub raw content URLs to point to the test server. +type rewriteTransport struct { + base http.RoundTripper + testServerURL string +} + +func (t *rewriteTransport) RoundTrip(req *http.Request) (*http.Response, error) { + // Rewrite any request to raw.githubusercontent.com to our test server + if strings.Contains(req.URL.Host, "raw.githubusercontent.com") { + // Keep the path, but change the scheme+host to the test server + newURL := t.testServerURL + req.URL.Path + newReq, err := http.NewRequestWithContext(req.Context(), req.Method, newURL, req.Body) + if err != nil { + return nil, err + } + base := t.base + if base == nil { + base = http.DefaultTransport + } + return base.RoundTrip(newReq) + } + base := t.base + if base == nil { + base = http.DefaultTransport + } + return base.RoundTrip(req) +} + +// TestEnsureModel_CoreSuccess verifies that when no serviceSDKVersion is +// provided (empty string), the core tag URL is used and the model is cached +// at the core path. +func TestEnsureModel_CoreSuccess(t *testing.T) { + modelBody := `{"metadata":{"apiVersion":"2023-01-01"}}` + + routes := map[string]struct { + status int + body string + }{ + "/aws/aws-sdk-go-v2/v1.41.5/codegen/sdk-codegen/aws-models/s3files.json": { + status: http.StatusOK, + body: modelBody, + }, + } + ts := newTestServer(routes) + defer ts.Close() + cleanup := patchTransport(ts.URL) + defer cleanup() + + cacheDir := t.TempDir() + // Pass empty serviceSDKVersion to exercise the core-only branch + basePath, err := sdk.EnsureModel(context.Background(), cacheDir, "v1.41.5", "s3files", "") + require.NoError(t, err) + + // Verify cached at core path + expectedBase := filepath.Join(cacheDir, "models", "v1.41.5") + assert.Equal(t, expectedBase, basePath) + + cachedFile := filepath.Join(basePath, "codegen", "sdk-codegen", "aws-models", "s3files.json") + data, err := os.ReadFile(cachedFile) + require.NoError(t, err) + assert.Equal(t, modelBody, string(data)) +} + +// TestEnsureModel_PerServiceSuccess verifies that when serviceSDKVersion is +// set, the per-service tag URL is tried first (not as a fallback from core +// 404) and no core request is made. The model is cached at the per-service +// path. +func TestEnsureModel_PerServiceSuccess(t *testing.T) { + modelBody := `{"metadata":{"apiVersion":"2024-06-01"}}` + + coreRequested := false + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Track whether the core URL was ever requested + if strings.Contains(r.URL.Path, "/aws/aws-sdk-go-v2/v1.41.5/") { + coreRequested = true + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, `{"core":"should not be used"}`) + return + } + // Per-service URL returns 200 + if strings.Contains(r.URL.Path, "/service/s3files/v1.0.0/") { + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, modelBody) + return + } + w.WriteHeader(http.StatusNotFound) + })) + defer ts.Close() + cleanup := patchTransport(ts.URL) + defer cleanup() + + cacheDir := t.TempDir() + basePath, err := sdk.EnsureModel(context.Background(), cacheDir, "v1.41.5", "s3files", "v1.0.0") + require.NoError(t, err) + + // Verify no core request was made — per-service is the sole path + assert.False(t, coreRequested, "core URL should not be requested when serviceSDKVersion is set") + + // Verify cached at per-service path + expectedBase := filepath.Join(cacheDir, "models", "service", "s3files", "v1.0.0") + assert.Equal(t, expectedBase, basePath) + + cachedFile := filepath.Join(basePath, "codegen", "sdk-codegen", "aws-models", "s3files.json") + data, err := os.ReadFile(cachedFile) + require.NoError(t, err) + assert.Equal(t, modelBody, string(data)) +} + +// TestEnsureModel_PerServiceFailure verifies that when serviceSDKVersion is +// set and the per-service URL returns a non-200 status, the error contains +// the per-service URL and the HTTP status code. No core fallback is attempted. +func TestEnsureModel_PerServiceFailure(t *testing.T) { + coreRequested := false + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/aws/aws-sdk-go-v2/v1.41.5/") { + coreRequested = true + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, `{"core":"should not be used"}`) + return + } + if strings.Contains(r.URL.Path, "/service/s3files/v1.0.0/") { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, "server error") + return + } + w.WriteHeader(http.StatusNotFound) + })) + defer ts.Close() + cleanup := patchTransport(ts.URL) + defer cleanup() + + cacheDir := t.TempDir() + _, err := sdk.EnsureModel(context.Background(), cacheDir, "v1.41.5", "s3files", "v1.0.0") + require.Error(t, err) + + // Verify no core fallback was attempted + assert.False(t, coreRequested, "core URL should not be requested when serviceSDKVersion is set") + + errMsg := err.Error() + // Error should contain the per-service URL and the status code + assert.Contains(t, errMsg, "/service/s3files/v1.0.0/") + assert.Contains(t, errMsg, "500") +} + +// TestEnsureModel_NoServiceVersionOn404 verifies that when the core URL returns +// 404 and no service SDK version is provided, the error suggests --aws-service-sdk-version. +func TestEnsureModel_NoServiceVersionOn404(t *testing.T) { + routes := map[string]struct { + status int + body string + }{ + "/aws/aws-sdk-go-v2/v1.41.5/codegen/sdk-codegen/aws-models/s3files.json": { + status: http.StatusNotFound, + body: "not found", + }, + } + ts := newTestServer(routes) + defer ts.Close() + cleanup := patchTransport(ts.URL) + defer cleanup() + + cacheDir := t.TempDir() + _, err := sdk.EnsureModel(context.Background(), cacheDir, "v1.41.5", "s3files", "") + require.Error(t, err) + + errMsg := err.Error() + assert.Contains(t, errMsg, "--aws-service-sdk-version") + assert.Contains(t, errMsg, "s3files") +} + +// TestEnsureModel_CachePathSeparation verifies that for any model name +// and SDK version pair, core and per-service cache paths are distinct. +func TestEnsureModel_CachePathSeparation(t *testing.T) { + tests := []struct { + name string + modelName string + coreVersion string + serviceVersion string + }{ + { + name: "s3files basic", + modelName: "s3files", + coreVersion: "v1.41.5", + serviceVersion: "v1.0.0", + }, + { + name: "sns different versions", + modelName: "sns", + coreVersion: "v1.40.0", + serviceVersion: "v2.1.0", + }, + { + name: "custom model name", + modelName: "my-custom-model", + coreVersion: "v1.50.0", + serviceVersion: "v3.0.0", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cacheDir := "/tmp/test-cache" + corePath := filepath.Join(cacheDir, "models", tc.coreVersion) + perServicePath := filepath.Join(cacheDir, "models", "service", tc.modelName, tc.serviceVersion) + + assert.NotEqual(t, corePath, perServicePath, + "core and per-service cache paths must be distinct") + + // Also verify they don't share a prefix that could cause file collisions + coreModelFile := filepath.Join(corePath, "codegen", "sdk-codegen", "aws-models", tc.modelName+".json") + perSvcModelFile := filepath.Join(perServicePath, "codegen", "sdk-codegen", "aws-models", tc.modelName+".json") + assert.NotEqual(t, coreModelFile, perSvcModelFile, + "core and per-service model file paths must be distinct") + }) + } +} + +// TestEnsureModel_PerServiceURLConstruction verifies that the per-service +// tag URL uses the model name in both the tag path segment and the file name, +// and contains the normalized (v-prefixed) version. +func TestEnsureModel_PerServiceURLConstruction(t *testing.T) { + tests := []struct { + name string + modelName string + serviceVersion string + expectedPath string + }{ + { + name: "basic service", + modelName: "s3files", + serviceVersion: "v1.0.0", + expectedPath: "/aws/aws-sdk-go-v2/service/s3files/v1.0.0/codegen/sdk-codegen/aws-models/s3files.json", + }, + { + name: "model name override", + modelName: "monitoring", + serviceVersion: "v2.3.1", + expectedPath: "/aws/aws-sdk-go-v2/service/monitoring/v2.3.1/codegen/sdk-codegen/aws-models/monitoring.json", + }, + { + name: "version without v prefix gets normalized", + modelName: "sqs", + serviceVersion: "1.5.0", + // EnsureSemverPrefix will add the v prefix + expectedPath: "/aws/aws-sdk-go-v2/service/sqs/v1.5.0/codegen/sdk-codegen/aws-models/sqs.json", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // We verify URL construction by setting up a test server that + // returns 404 for core and captures the per-service request path. + var capturedPath string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/service/") { + capturedPath = r.URL.Path + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, `{"test":"data"}`) + return + } + // Core URL returns 404 + w.WriteHeader(http.StatusNotFound) + })) + defer ts.Close() + cleanup := patchTransport(ts.URL) + defer cleanup() + + cacheDir := t.TempDir() + _, err := sdk.EnsureModel(context.Background(), cacheDir, "v1.41.5", tc.modelName, tc.serviceVersion) + require.NoError(t, err) + + assert.Equal(t, tc.expectedPath, capturedPath, + "per-service URL path should match expected format") + }) + } +} + +// TestEnsureSemverPrefix_Idempotence verifies that applying +// EnsureSemverPrefix twice produces the same result as applying it once. +func TestEnsureSemverPrefix_Idempotence(t *testing.T) { + tests := []struct { + name string + input string + }{ + {name: "no prefix", input: "1.0.0"}, + {name: "single v prefix", input: "v1.0.0"}, + {name: "double v prefix", input: "vv1.0.0"}, + {name: "triple v prefix", input: "vvv1.0.0"}, + {name: "complex version", input: "1.2.3-beta.1"}, + {name: "v-prefixed complex", input: "v1.2.3-beta.1"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + once := sdk.EnsureSemverPrefix(tc.input) + twice := sdk.EnsureSemverPrefix(once) + assert.Equal(t, once, twice, + "EnsureSemverPrefix should be idempotent: f(f(x)) == f(x)") + // Also verify the result starts with exactly one 'v' + assert.True(t, strings.HasPrefix(once, "v"), "result should start with 'v'") + assert.False(t, strings.HasPrefix(once, "vv"), "result should not start with 'vv'") + }) + } +} + +// TestEnsureModel_CoreCacheHit verifies that when serviceSDKVersion is empty, +// a cached core model is returned without making any HTTP requests. +func TestEnsureModel_CoreCacheHit(t *testing.T) { + cacheDir := t.TempDir() + modelBody := `{"cached":"true"}` + + // Pre-populate the core cache + modelDir := filepath.Join(cacheDir, "models", "v1.41.5", "codegen", "sdk-codegen", "aws-models") + require.NoError(t, os.MkdirAll(modelDir, os.ModePerm)) + require.NoError(t, os.WriteFile(filepath.Join(modelDir, "s3files.json"), []byte(modelBody), 0644)) + + // Pass empty serviceSDKVersion to exercise the core-only branch + // No test server needed — if HTTP is attempted, it will fail + basePath, err := sdk.EnsureModel(context.Background(), cacheDir, "v1.41.5", "s3files", "") + require.NoError(t, err) + + expectedBase := filepath.Join(cacheDir, "models", "v1.41.5") + assert.Equal(t, expectedBase, basePath) +} + +// TestEnsureModel_PerServiceCacheHit verifies that when serviceSDKVersion is +// set, the per-service cache is checked first and returned on hit. No core +// cache check or HTTP request is made. +func TestEnsureModel_PerServiceCacheHit(t *testing.T) { + cacheDir := t.TempDir() + modelBody := `{"cached":"per-service"}` + + // Pre-populate the per-service cache + modelDir := filepath.Join(cacheDir, "models", "service", "s3files", "v1.0.0", "codegen", "sdk-codegen", "aws-models") + require.NoError(t, os.MkdirAll(modelDir, os.ModePerm)) + require.NoError(t, os.WriteFile(filepath.Join(modelDir, "s3files.json"), []byte(modelBody), 0644)) + + // Also pre-populate a core cache with different content to prove it's not used + coreModelDir := filepath.Join(cacheDir, "models", "v1.41.5", "codegen", "sdk-codegen", "aws-models") + require.NoError(t, os.MkdirAll(coreModelDir, os.ModePerm)) + require.NoError(t, os.WriteFile(filepath.Join(coreModelDir, "s3files.json"), []byte(`{"cached":"core-should-not-be-used"}`), 0644)) + + // No test server needed — if HTTP is attempted, it will fail + basePath, err := sdk.EnsureModel(context.Background(), cacheDir, "v1.41.5", "s3files", "v1.0.0") + require.NoError(t, err) + + // Verify per-service cache path is returned, not core + expectedBase := filepath.Join(cacheDir, "models", "service", "s3files", "v1.0.0") + assert.Equal(t, expectedBase, basePath) + + // Verify the returned path contains the per-service cached content + cachedFile := filepath.Join(basePath, "codegen", "sdk-codegen", "aws-models", "s3files.json") + data, err := os.ReadFile(cachedFile) + require.NoError(t, err) + assert.Equal(t, modelBody, string(data)) +} diff --git a/pkg/sdk/repo.go b/pkg/sdk/repo.go index 73494ef5..b8e261b1 100644 --- a/pkg/sdk/repo.go +++ b/pkg/sdk/repo.go @@ -121,6 +121,28 @@ func GetSDKVersion( return "", err } +// GetServiceSDKVersion returns the per-service SDK version to use. It first +// tries to get the version from the --aws-service-sdk-version flag, then from +// the ack-generate-metadata.yaml aws_service_sdk_version field. Unlike +// GetSDKVersion, this returns empty string (not error) when no version is +// available, since per-service version is optional. +func GetServiceSDKVersion( + awsServiceSDKVersion string, + lastGenerationVersion string, +) string { + // First try to get the version from --aws-service-sdk-version flag + if awsServiceSDKVersion != "" { + return awsServiceSDKVersion + } + + // then, try to use last generation version (from ack-generate-metadata.yaml) + if lastGenerationVersion != "" { + return lastGenerationVersion + } + + return "" +} + // getSDKVersionFromGoMod parses a given go.mod file and returns // the aws-sdk-go version in the required modules. func getSDKVersionFromGoMod(goModPath string) (string, error) { diff --git a/pkg/sdk/repo_test.go b/pkg/sdk/repo_test.go new file mode 100644 index 00000000..1050e97b --- /dev/null +++ b/pkg/sdk/repo_test.go @@ -0,0 +1,66 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package sdk_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/aws-controllers-k8s/code-generator/pkg/sdk" +) + +// TestGetServiceSDKVersion validates the version resolution priority chain: +// CLI flag if non-empty, else metadata YAML value if non-empty, else empty +// string. +func TestGetServiceSDKVersion(t *testing.T) { + tests := []struct { + name string + awsServiceSDKVersion string + lastGenerationVersion string + expected string + }{ + { + name: "flag only", + awsServiceSDKVersion: "v1.2.0", + lastGenerationVersion: "", + expected: "v1.2.0", + }, + { + name: "metadata only", + awsServiceSDKVersion: "", + lastGenerationVersion: "v1.0.0", + expected: "v1.0.0", + }, + { + name: "both provided, flag wins", + awsServiceSDKVersion: "v2.0.0", + lastGenerationVersion: "v1.0.0", + expected: "v2.0.0", + }, + { + name: "neither provided, returns empty", + awsServiceSDKVersion: "", + lastGenerationVersion: "", + expected: "", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := sdk.GetServiceSDKVersion(tc.awsServiceSDKVersion, tc.lastGenerationVersion) + assert.Equal(t, tc.expected, got) + }) + } +} diff --git a/scripts/build-controller-release.sh b/scripts/build-controller-release.sh index 4cb0cee9..8e43e3bd 100755 --- a/scripts/build-controller-release.sh +++ b/scripts/build-controller-release.sh @@ -172,6 +172,13 @@ if [ -z "$AWS_SDK_GO_VERSION" ]; then AWS_SDK_GO_VERSION=$(cat "$SERVICE_CONTROLLER_SOURCE_PATH/apis/$ACK_GENERATE_API_VERSION/ack-generate-metadata.yaml" | yq ".aws_sdk_go_version" -r) fi +if [ -z "$AWS_SERVICE_SDK_VERSION" ]; then + AWS_SERVICE_SDK_VERSION=$(cat "$SERVICE_CONTROLLER_SOURCE_PATH/apis/$ACK_GENERATE_API_VERSION/ack-generate-metadata.yaml" | yq ".aws_service_sdk_version" -r) + if [ "$AWS_SERVICE_SDK_VERSION" = "null" ] || [ "$AWS_SERVICE_SDK_VERSION" = "" ]; then + AWS_SERVICE_SDK_VERSION="" + fi +fi + # If there's a generator.yaml in the service's directory and the caller hasn't # specified an override, use that. if [ -z "$ACK_GENERATE_CONFIG_PATH" ]; then @@ -197,7 +204,13 @@ if [ -z "$ACK_DOCUMENTATION_CONFIG_PATH" ]; then fi helm_output_dir="$SERVICE_CONTROLLER_SOURCE_PATH/helm" -ag_args=("$SERVICE" "$RELEASE_VERSION" -o "$SERVICE_CONTROLLER_SOURCE_PATH" --template-dirs "$TEMPLATES_DIR" --aws-sdk-go-version "$AWS_SDK_GO_VERSION") +# When a per-service SDK version is set, it takes precedence — don't pass +# --aws-sdk-go-version since the two flags are mutually exclusive. +if [ -n "$AWS_SERVICE_SDK_VERSION" ]; then + ag_args=("$SERVICE" "$RELEASE_VERSION" -o "$SERVICE_CONTROLLER_SOURCE_PATH" --template-dirs "$TEMPLATES_DIR" --aws-service-sdk-version "$AWS_SERVICE_SDK_VERSION") +else + ag_args=("$SERVICE" "$RELEASE_VERSION" -o "$SERVICE_CONTROLLER_SOURCE_PATH" --template-dirs "$TEMPLATES_DIR" --aws-sdk-go-version "$AWS_SDK_GO_VERSION") +fi if [ -n "$ACK_GENERATE_CACHE_DIR" ]; then ag_args=("${ag_args[@]}" --cache-dir "$ACK_GENERATE_CACHE_DIR") fi @@ -276,7 +289,13 @@ if [[ $ACK_GENERATE_OLM == "true" ]]; then DEFAULT_ACK_GENERATE_OLMCONFIG_PATH="$SERVICE_CONTROLLER_SOURCE_PATH/olm/olmconfig.yaml" ACK_GENERATE_OLMCONFIG_PATH=${ACK_GENERATE_OLMCONFIG_PATH:-$DEFAULT_ACK_GENERATE_OLMCONFIG_PATH} - ag_olm_args=("$SERVICE" "$RELEASE_VERSION" -o "$SERVICE_CONTROLLER_SOURCE_PATH" --template-dirs "$TEMPLATES_DIR" --olm-config "$ACK_GENERATE_OLMCONFIG_PATH" --aws-sdk-go-version "$AWS_SDK_GO_VERSION") + # When a per-service SDK version is set, it takes precedence — don't pass + # --aws-sdk-go-version since the two flags are mutually exclusive. + if [ -n "$AWS_SERVICE_SDK_VERSION" ]; then + ag_olm_args=("$SERVICE" "$RELEASE_VERSION" -o "$SERVICE_CONTROLLER_SOURCE_PATH" --template-dirs "$TEMPLATES_DIR" --olm-config "$ACK_GENERATE_OLMCONFIG_PATH" --aws-service-sdk-version "$AWS_SERVICE_SDK_VERSION") + else + ag_olm_args=("$SERVICE" "$RELEASE_VERSION" -o "$SERVICE_CONTROLLER_SOURCE_PATH" --template-dirs "$TEMPLATES_DIR" --olm-config "$ACK_GENERATE_OLMCONFIG_PATH" --aws-sdk-go-version "$AWS_SDK_GO_VERSION") + fi if [ -n "$ACK_GENERATE_CONFIG_PATH" ]; then ag_olm_args=("${ag_olm_args[@]}" --generator-config-path "$ACK_GENERATE_CONFIG_PATH") diff --git a/scripts/build-controller.sh b/scripts/build-controller.sh index abdb7ee7..336082ca 100755 --- a/scripts/build-controller.sh +++ b/scripts/build-controller.sh @@ -66,6 +66,9 @@ Environment variables: AWS_SDK_GO_VERSION: Overrides the version of github.com/aws/aws-sdk-go used by 'ack-generate' to fetch the service API Specifications. Default: Version of aws/aws-sdk-go in service go.mod + AWS_SERVICE_SDK_VERSION: Overrides the per-service SDK version used by 'ack-generate' + to fetch the service API model from a per-service tag. + Default: Value of aws_service_sdk_version in ack-generate-metadata.yaml TEMPLATE_DIRS: Overrides the list of directories containing ack-generate templates. Default: $TEMPLATE_DIRS @@ -141,6 +144,13 @@ if [ -z "$AWS_SDK_GO_VERSION" ]; then AWS_SDK_GO_VERSION=$(cat "$SERVICE_CONTROLLER_SOURCE_PATH/apis/$ACK_GENERATE_API_VERSION/ack-generate-metadata.yaml" | yq ".aws_sdk_go_version" -r) fi +if [ -z "$AWS_SERVICE_SDK_VERSION" ]; then + AWS_SERVICE_SDK_VERSION=$(cat "$SERVICE_CONTROLLER_SOURCE_PATH/apis/$ACK_GENERATE_API_VERSION/ack-generate-metadata.yaml" | yq ".aws_service_sdk_version" -r) + if [ "$AWS_SERVICE_SDK_VERSION" = "null" ] || [ "$AWS_SERVICE_SDK_VERSION" = "" ]; then + AWS_SERVICE_SDK_VERSION="" + fi +fi + # If there's a generator.yaml in the service's directory and the caller hasn't # specified an override, use that. if [ -z "$ACK_GENERATE_CONFIG_PATH" ]; then @@ -190,9 +200,19 @@ if [ -n "$ACK_DOCUMENTATION_CONFIG_PATH" ]; then apis_args=("${apis_args[@]}" --documentation-config-path "$ACK_DOCUMENTATION_CONFIG_PATH") fi -if [ -n "$AWS_SDK_GO_VERSION" ]; then +# When a per-service SDK version is set, it takes precedence for model +# fetching. Don't pass --aws-sdk-go-version to ack-generate in that case +# since the two flags are mutually exclusive. The core version is still +# resolved internally from metadata/go.mod for other purposes. +if [ -n "$AWS_SERVICE_SDK_VERSION" ]; then + ag_args=("${ag_args[@]}" --aws-service-sdk-version "$AWS_SERVICE_SDK_VERSION") + apis_args=("${apis_args[@]}" --aws-service-sdk-version "$AWS_SERVICE_SDK_VERSION") +elif [ -n "$AWS_SDK_GO_VERSION" ]; then ag_args=("${ag_args[@]}" --aws-sdk-go-version "$AWS_SDK_GO_VERSION") apis_args=("${apis_args[@]}" --aws-sdk-go-version "$AWS_SDK_GO_VERSION") +else + echo "ERROR: at least one of AWS_SDK_GO_VERSION or AWS_SERVICE_SDK_VERSION must be set" 1>&2 + exit 1 fi if [ -n "$ACK_GENERATE_SERVICE_ACCOUNT_NAME" ]; then