Skip to content

OCPBUGS-99695: Add TC 88940- Apply password only if changes exist #6328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Jul 24, 2026
Merged

OCPBUGS-99695: Add TC 88940- Apply password only if changes exist #6328

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions test/extended-priv/mco_password.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,108 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati
checkAuthorizedKeyInNode(mcp.GetCoreOsNodesOrFail()[0], append(initialKeys, key))
logger.Infof("OK!\n")
})

// https://issues.redhat.com/browse/OCPBUGS-83830
g.It("[PolarionID:88940][OTP] Apply password only if changes exist [Disruptive]", func() {
var (
mcName = "tc-88940-core-password-hash-change"
silentOC = oc.AsAdmin()

password = exutil.GetRandomString()
passwordHash = OrFail[string](getHashPasswd(password))
updatedPassword = password + "-2"
updatedPasswdHash = OrFail[string](getHashPasswd(updatedPassword))

_, sshPubKey = GenerateSSHKeyPairOrFail()

pwdConfiguredLogMsg = "Password has been configured"
expectedReboot = false
)
silentOC.NotShowInfo()
sshPubKey = strings.TrimSpace(sshPubKey)

node := mcp.GetSortedNodesOrFail()[0]
mcc := NewController(oc.AsAdmin()).IgnoreLogsBeforeNowOrFail()
startTime := node.GetDateOrFail()
o.Expect(node.IgnoreEventsBeforeNow()).NotTo(o.HaveOccurred(),
"Error getting the latest event in node %s", node.GetName())
Comment thread
coderabbitai[bot] marked this conversation as resolved.

exutil.By("Apply MC with passwordHash for core user")
pwdUser := ign32PaswdUser{Name: user, PasswordHash: passwordHash}
mc := NewMachineConfig(silentOC, mcName, mcp.GetName())
mc.parameters = []string{fmt.Sprintf(`PWDUSERS=[%s]`, MarshalOrFail(pwdUser))}
mc.skipWaitForMcp = true

defer mc.DeleteWithWait()
mc.create()
mcp.waitForComplete()
logger.Infof("OK!\n")

exutil.By("Check MCD logs to verify 'Password has been configured' is logged")
mcdLogs, err := node.GetMCDaemonLogs("")
o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MCD logs for node %s", node.GetName())
initialPasswordCount := strings.Count(mcdLogs, pwdConfiguredLogMsg)
o.Expect(initialPasswordCount).To(o.BeNumerically(">", 0),
"Expected '%s' in MCD logs after initial password application on node %s", pwdConfiguredLogMsg, node.GetName())
Comment on lines +462 to +467

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Capture the pre-apply log count.

initialPasswordCount > 0 passes when the node already has this message from prior activity, so it does not prove the initial MachineConfig emitted it. Snapshot the count before mc.create() and compare against that baseline.

Proposed fix 
+		mcdLogsBeforeApply, err := node.GetMCDaemonLogs("")
+		o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MCD logs")
+		passwordCountBeforeApply := strings.Count(mcdLogsBeforeApply, pwdConfiguredLogMsg)
+
 		exutil.By("Apply MC with passwordHash for core user")
 ...
-		o.Expect(initialPasswordCount).To(o.BeNumerically(">", 0),
+		o.Expect(initialPasswordCount).To(o.BeNumerically(">", passwordCountBeforeApply),

Based on the PR objective to verify initial password-configuration logging.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/extended-priv/mco_password.go` around lines 462 - 467, Update the
password-configuration verification around mc.create() to capture the
pwdConfiguredLogMsg count from node.GetMCDaemonLogs("") before applying the
MachineConfig, then retrieve the logs afterward and assert the count increased.
Replace the current initialPasswordCount > 0 check while preserving the existing
error handling and node-specific assertion context.

logger.Infof("OK!\n")

exutil.By("Check that drain and reboot are skipped for password-only change")
checkDrainAction(expectedReboot, node, mcc)
checkRebootAction(expectedReboot, node, startTime)
logger.Infof("OK!\n")

exutil.By("Update the passwordHash in MC")
patchErr := mc.Patch("json",
fmt.Sprintf(`[{ "op": "replace", "path": "/spec/config/passwd/users/0/passwordHash", "value": "%s"}]`, updatedPasswdHash))
o.Expect(patchErr).NotTo(o.HaveOccurred(),
"Error patching mc %s to update the 'core' user password", mc.GetName())
mcp.waitForComplete()
logger.Infof("OK!\n")

exutil.By("Check MCD logs to verify 'Password has been configured' is logged again after hash change")
mcdLogs, err = node.GetMCDaemonLogs("")
o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MCD logs for node %s", node.GetName())
updatedPasswordCount := strings.Count(mcdLogs, pwdConfiguredLogMsg)
o.Expect(updatedPasswordCount).To(o.BeNumerically(">", initialPasswordCount),
"Expected a new '%s' log entry after password hash change on node %s", pwdConfiguredLogMsg, node.GetName())
logger.Infof("OK!\n")

exutil.By("Get currently configured authorized keys before adding new SSH key")
currentMc, err := mcp.GetConfiguredMachineConfig()
o.Expect(err).NotTo(o.HaveOccurred(), "Error getting the current configuration for %s pool", mcp.GetName())
initialKeys, err := currentMc.GetAuthorizedKeysByUserAsList("core")
o.Expect(err).NotTo(o.HaveOccurred(), "Error getting the current authorizedkeys for user 'core' in %s pool", mcp.GetName())
logger.Infof("OK!\n")

exutil.By("Add SSH key to MC without changing the password hash")
pwdUserWithSSH := ign32PaswdUser{Name: user, PasswordHash: updatedPasswdHash, SSHAuthorizedKeys: []string{sshPubKey}}
patchErr = mc.Patch("json",
fmt.Sprintf(`[{ "op": "replace", "path": "/spec/config/passwd/users", "value": [%s]}]`, MarshalOrFail(pwdUserWithSSH)))
o.Expect(patchErr).NotTo(o.HaveOccurred(),
"Error patching mc %s to add SSH key", mc.GetName())
mcp.waitForComplete()
logger.Infof("OK!\n")

exutil.By("Verify that the SSH key was correctly applied to the node")
checkAuthorizedKeyInNode(node, append(initialKeys, sshPubKey))
logger.Infof("OK!\n")

exutil.By("Check MCD logs to verify 'Password has been configured' is NOT logged for SSH-only change")
mcdLogs, err = node.GetMCDaemonLogs("")
o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MCD logs for node %s", node.GetName())
finalPasswordCount := strings.Count(mcdLogs, pwdConfiguredLogMsg)
o.Expect(finalPasswordCount).To(o.Equal(updatedPasswordCount),
"'%s' should NOT appear in MCD logs when only SSH keys are added (no password hash change) on node %s",
pwdConfiguredLogMsg, node.GetName())
logger.Infof("OK!\n")

exutil.By("Check events to make sure that drain and reboot events were not triggered")
nodeEvents, eErr := node.GetEvents()
o.Expect(eErr).ShouldNot(o.HaveOccurred(), "Error getting events for node %s", node.GetName())
o.Expect(nodeEvents).NotTo(HaveEventsSequence("Drain"), "Error, a Drain event was triggered but it shouldn't")
o.Expect(nodeEvents).NotTo(HaveEventsSequence("Reboot"), "Error, a Reboot event was triggered but it shouldn't")
logger.Infof("OK!\n")
})
})

// getPasswdValidator returns the commands that need to be executed in an interactive expect shell to validate that a user can login
Expand Down