fix: handle optional authConfig in mechanizeDockerContainer function#4016
Merged
Siumauricio merged 1 commit intocanaryfrom Mar 17, 2026
Conversation
- Updated the mechanizeDockerContainer function to conditionally use authConfig when creating a Docker service, ensuring proper service creation based on authentication settings.
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.
What is this PR about?
Please describe in a short paragraph what this PR is about.
Checklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
closes #4003
Screenshots (if applicable)
Greptile Summary
This PR fixes Docker service creation for applications using private registry images by conditionally passing
authConfigas the first argument to Dockerode'sdocker.createService(auth, opts)overload, ensuring the auth token is correctly sent as theX-Registry-AuthHTTP header.authconfiginsettings:settingsstill containsauthconfig: authConfig(line 129), so auth is now passed twice in the truthy branch — once correctly as the first arg and once harmlessly in the body. Removing it fromsettingswould clean up the intent.service.updatenot addressed: The existing-service update path (lines 175–182) spreadssettings(includingauthconfig) but does not apply the same(auth, opts)pattern. If Dockerode'sService.updatealso requires auth as a separate argument for private registry re-pulls, this path could have the same latent authentication issue for service updates.Confidence Score: 4/5
createService(auth, opts)overload. The only concerns are a minor redundancy insettings.authconfigand a pre-existing inconsistency in theservice.updatepath that is not made worse by this PR.service.updatepath (lines 175–182) for the same auth-header concern.Comments Outside Diff (2)
packages/server/src/utils/builders/index.ts, line 175-182 (link)service.updatehas same auth issue ascreateServiceThe fix correctly passes
authConfigas a separate first argument todocker.createService, which maps to theX-Registry-Authheader in the Docker API. However, theservice.updatecall on this path uses...settingswhich spreadsauthconfiginto the request body — but Docker's service update API also requiresX-Registry-Authas a header (not a body field) for private registry images.Depending on how Dockerode's
Service.updatehandles theauthconfigfield within the options object, updates of existing services that use private registry images may still silently fail to re-authenticate. If Dockerode'sService.updatesupports an(auth, opts)overload similar tocreateService, the same conditional auth pattern should be applied here for consistency.packages/server/src/utils/builders/index.ts, line 128-129 (link)authconfiginsettingsis now redundant for thecreateServicepathsettingsstill includesauthconfig: authConfigat this line. After the fix, whenauthConfigis truthy,docker.createService(authConfig, settings)passes auth both as the dedicated first argument (correct, maps toX-Registry-Authheader) and inside thesettingsbody asauthconfig(which Docker ignores for service creation). This is harmless but redundant.Consider removing
authconfigfrom thesettingsobject and relying solely on the explicit first-argument form to keep the intent clear:Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Last reviewed commit: e7af2c0