Use catalog API to resolve latest emulator version#101
Open
anisaoshafi wants to merge 3 commits intomainfrom
Open
Use catalog API to resolve latest emulator version#101anisaoshafi wants to merge 3 commits intomainfrom
anisaoshafi wants to merge 3 commits intomainfrom
Conversation
anisaoshafi
force-pushed
the
drg-504
branch
2 times, most recently
from
2148708 to
845bbfd
Compare
anisaoshafi
requested review from
carole-lavillonniere and
silv-io
as code owners
📝 WalkthroughWalkthroughAdds PlatformClient.GetLatestCatalogVersion and its tests, a generated MockPlatformAPI, and container start logic to resolve "latest"/empty image tags via the catalog API with a Docker-inspection fallback; integrates resolved versions into existing license/start flows and updates related tests. Changes
Sequence DiagramsequenceDiagram
participant Client as License Validator
participant Resolver as resolveVersion()
participant CatalogAPI as Platform Catalog API
participant Docker as Docker Runtime
Client->>Resolver: resolveVersion(ctx, emulatorType, tag)
alt Pinned tag (non-"latest" & non-empty)
Resolver-->>Client: return pinned tag
else tag empty or "latest"
Resolver->>CatalogAPI: GET /v1/license/catalog/{emulatorType}/version
alt API returns 200 with valid {emulator_type, version}
CatalogAPI-->>Resolver: { emulator_type, version }
Resolver-->>Client: return catalog version
else API timeout/failure or invalid response
Resolver->>Docker: inspect local image for version
alt Docker returns version
Docker-->>Resolver: local image version
Resolver-->>Client: return local version
else Docker failure
Resolver-->>Client: return error
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/api/client.go`:
- Around line 277-279: The code currently only checks for empty fields in
versionResp; change the guard in the function handling catalog responses to also
compare versionResp.EmulatorType with the requested emulatorType and return an
error on mismatch (e.g., "unexpected emulator_type: got=%q want=%q") so a
response for the wrong emulator is rejected; update the test file
catalog_version_test.go to add a regression test that simulates a response with
a non-empty but non-matching versionResp.EmulatorType and asserts that the
function returns an error.
In `@internal/container/start.go`:
- Around line 254-255: The fallback log uses log.Printf and bypasses the output
sink; update resolveVersion to accept an output.Sink parameter (thread the sink
from Start into resolveVersion and any callers like resolveVersion(...)),
replace the log.Printf call with the appropriate typed event (e.g.
output.EmitWarning or output.EmitNote) using the passed sink and include the
error in the message, and add the necessary import of internal/output; ensure
callers are updated to pass the existing sink so the message respects
non‑TTY/sink behavior.
- Line 249: The call to platformClient.GetLatestCatalogVersion incorrectly
passes containerConfig.ProductName (Docker product name) instead of the emulator
key; change the argument to containerConfig.Type so
GetLatestCatalogVersion(apiCtx, containerConfig.Type) is used. Locate the call
to GetLatestCatalogVersion in start.go and replace the ProductName parameter
with Type, keeping the apiCtx and error handling intact; ensure tests that mock
GetLatestCatalogVersion expect the emulator key value (e.g., "aws") rather than
the Docker product name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 9eeef742-c5ca-47fa-99e1-67552444b31b
📒 Files selected for processing (5)
internal/api/catalog_version_test.gointernal/api/client.gointernal/api/mock_platform_api.gointernal/container/start.gointernal/container/start_test.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
internal/container/start.go (1)
248-248:⚠️ Potential issue | 🔴 CriticalUse
containerConfig.Typeinstead ofcontainerConfig.ProductNamefor the catalog API call.The
GetLatestCatalogVersionmethod expects an emulator type parameter (e.g.,"aws") to construct the URL/v1/license/catalog/{emulatorType}/version. However,ProductNamereturns the Docker image product name ("localstack-pro"), causing the API call to fail and always fall back to Docker image inspection.🐛 Proposed fix
- v, err := platformClient.GetLatestCatalogVersion(apiCtx, containerConfig.ProductName) + v, err := platformClient.GetLatestCatalogVersion(apiCtx, containerConfig.Type)Note: This requires adding a
Typefield toruntime.ContainerConfigif it doesn't already exist, or passing the type separately toresolveVersion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/container/start.go` at line 248, The call to platformClient.GetLatestCatalogVersion in resolveVersion is using containerConfig.ProductName but the API expects an emulator type; change the argument to containerConfig.Type (i.e., use the emulator type) when calling GetLatestCatalogVersion; if runtime.ContainerConfig lacks a Type field, add it (or pass the type into resolveVersion) and ensure any call sites that construct ContainerConfig populate Type accordingly so the URL /v1/license/catalog/{emulatorType}/version is built correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@internal/container/start.go`:
- Line 248: The call to platformClient.GetLatestCatalogVersion in resolveVersion
is using containerConfig.ProductName but the API expects an emulator type;
change the argument to containerConfig.Type (i.e., use the emulator type) when
calling GetLatestCatalogVersion; if runtime.ContainerConfig lacks a Type field,
add it (or pass the type into resolveVersion) and ensure any call sites that
construct ContainerConfig populate Type accordingly so the URL
/v1/license/catalog/{emulatorType}/version is built correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e78c2fc7-dbd7-4fdb-9848-bf5ce33ad31b
📒 Files selected for processing (1)
internal/container/start.go
anisaoshafi
force-pushed
the
drg-504
branch
2 times, most recently
from
1fcbccc to
0a8f759
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently to resolve latest version, we inspect the local Docker image.
This PR changes it so that we rely on the version set in the catalog API response.
If platform API is down or response changes falls back to inspecting the local Docker image.