Repository navigation
fix: report unedited saves as unchanged, follow Chef precedence in node ssh, surface export config errors - #260
Merged
Conversation
…de ssh, surface export config errors
Edit commands compared the object before and after the editor with
reflect.DeepEqual. The server sends empty attribute maps as {}, and the
editor's round trip through omitempty turns them into nil, so saving a
role or environment without touching it still sent a PUT and printed
"Updated". Compare the JSON each side would send instead.
node ssh --attribute looked a dotted path up only at the top of the
search row, so cloud.public_hostname (which lives under automatic) never
resolved, and a plain name tried default before override. Decode the row
as a node and use cinc-api's precedence-aware lookup.
policy export discarded every error from resolving the server client, so
a broken config surfaced as a vague "requires a configured server
connection" and a fresh machine could start first-run setup mid-export.
Resolve the client only when the lock has a chef_server source, and
return its error when it fails.
Signed-off-by: Tim Smith <tsmith84@proton.me>
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.
Three bugs from an audit of the command layer. None needs new cinc-api surface.
Edit commands reported "Updated" for a save that changed nothing. Every
editcompared before/after withreflect.DeepEqual. The server sends empty attribute maps as{}, and the editor's round trip throughomitemptyturns them intonil, sorole edit/environment editalways sent a PUT. The existing tests stubbed the editor with a hand-built struct, so they never saw this. The fix adds anunchangedhelper that compares the JSON each side would send, used at all nine edit sites.node ssh --attributeignored Chef precedence. A dotted path was looked up only at the top of the search row, socloud.public_hostname(underautomatic) never resolved. A plain name also trieddefaultbeforeoverride. The fix decodes the row as acinc.Nodeand uses its precedence-awareAttributeString(automatic, override, normal, default).namestill resolves to the node name.policy exportswallowed config errors. It calledchef, _ := resolveClient(cmd), so a broken config showed up as "chef_server source requires a configured server connection", and on a fresh machine with a terminal it could start first-run setup in the middle of an offline export. The fix resolves the client only when the lock has achef_serversource, and returns its error.Test plan
TestEditWithoutChangesSendsNothing(role, environment) serves the raw server body and stubs the editor with a real marshal/unmarshal round trip. It fails before the change and passes after.TestSearchRowAttributeFollowsChefPrecedence: dotted path and override-over-default cases fail before the change and pass after.TestPolicyExportResolvesTheServerOnlyWhenTheLockNeedsIt: a path lock exports with a broken config, and a chef_server lock reports the config error.go test ./apps/cinc/cmd/ ./cli/policyfile/,go vet,gofmtclean