fix(node): validate the profile on a bootstrap dry run - #188
Merged
Merged
Conversation
resolveClient is the only thing that calls Profile.Validate, and a dry run skips it. The server URL is then concatenated unconditionally into the client.rb the script writes, so a profile naming no Cinc Server, such as a Supermarket-only one, produced a script pointing nowhere and exited zero: chef_server_url '/organizations/' Dry run exists to show what a real run would do, so it has to apply the same requirement rather than print something that could never work. Validate the resolved profile before building the command, on both paths. Signed-off-by: Tim Smith <tim@mondoo.com>
tas50
force-pushed
the
fix/bootstrap-dry-run-validates-profile
branch
from
September 8, 2026 15:16
62eadd1 to
76c60c4
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
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.
The defect
resolveClientis the only thing that callsProfile.Validate(), and--dry-runskips it. The server URL is then concatenated unconditionally into theclient.rbthe script writes:With a profile that names no Cinc Server — a Supermarket-only one, say, which is perfectly valid on disk and which
cinc config createwill happily write — the dry run prints a script pointing nowhere and exits 0:A dry run exists to show what a real run would do. Printing a script that could never work, and reporting success, is the one thing it must not do — especially since the natural next step is to copy that script and run it by hand.
The fix
Validate the resolved profile before building the command, on both paths. A real run reaches the same check a moment later in
resolveClient, so this only changes the dry-run path.Tests
TestNodeBootstrapDryRunRejectsProfileWithoutServeruses a Supermarket-only profile and asserts the command errors and prints no script. Verified red before the fix by stashing it — onmainit emits the/organizations/script above and returns nil.go test ./...,go vet ./..., andgofmt -l .are clean.Note
Same function as the
--formatordering fix in the sibling PR. Independent changes, so separate PRs as requested, but whichever lands second will want a trivial rebase.