Draft
Conversation
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
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.
https://linear.app/keboola/issue/PAT-1571/3-docker-bundle-remove-definitionname-fallback-and-transformimageurl
Description
Removes the
definition.namefallback path that was added during the replicated-registry rollout. When the replicated registry was enabled and a component was missingdefinition.name,ReplicatedRegistryImage::getImageId()previously logged a warning and rewrote the original ECR image id viaReplicatedRegistry::transformImageUrl(). All components now havedefinition.nameset, so the fallback (and the URI-rewrite path that backed it) is deleted; missingdefinition.namenow hard-fails with anApplicationException.Counterpart to https://github.com/keboola/job-queue-daemon/pull/861 — same strict-mode cleanup, adapted to docker-bundle's exception conventions.
Changes:
ReplicatedRegistryImage::getImageId()— drop theimageName === nullwarning branch, throwApplicationExceptioninstead.ReplicatedRegistry— deletetransformImageUrl()method andECR_REGISTRY_URLconstant.testTransformImageUrl()+ provider andtestTransformImageUrlWhenDisabledThrowsException()+ provider; replacetestGetImageIdFallsBackToUriTransformationWhenDefinitionNameMissing()withtestGetImageIdThrowsWhenDefinitionNameMissing()covering the new throw.Release Notes
Plans for Customer Communication
None.
Impact Analysis
definition.name, image resolution will now fail withApplicationExceptioninstead of silently rewriting the ECR URI. Audit beforehand confirms all components havedefinition.name.ReplicatedRegistryImageis not constructed at all on these stacks.job-runnerwill be bumped in a follow-up PR (composer update keboola/dockerbundle --with-dependencies) once this lands.Justification
The fallback was a temporary safety net during the replicated-registry rollout. Now that rollout is complete and every component has
definition.name, the silent URI rewrite obscures real misconfiguration; failing loudly is preferred.Deployment Plan
Merge & Deploy.
Rollback Plan
Revert this PR.
Post-Release Support Plan
Watch image-pull error rates on GCP/Azure stacks for
ApplicationException: Component "…" is missing definition.name…. If a genuine component surfaces missingdefinition.name, fix the component definition rather than reverting this PR.