Skip to content

NO-ISSUE: test MCC proxy. Refactor TC 52373 proxy test. #6355

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 31, 2026
Merged

NO-ISSUE: test MCC proxy. Refactor TC 52373 proxy test. #6355

Show file tree
Hide file tree
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
96 changes: 58 additions & 38 deletions test/extended-priv/mco_machineconfigpool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,25 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati

g.It("[PolarionID:52373][OTP] Modify proxy configuration in paused pools", func() {

proxyValue := "http://user:pass@proxy-fake:1111"
noProxyValue := "test.52373.no-proxy.com"
var (
proxyValue = "http://user:pass@proxy-fake:1111"
noProxyValue = "test.52373.no-proxy.com"
expectedDegradedMessage = `required MachineConfigPool master is paused and cannot sync until it is unpaused`
wmcp = NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolWorker)
mmcp = NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolMaster)
machineConfiguration = GetMachineConfiguration(oc.AsAdmin())
proxy = NewResource(oc.AsAdmin(), "proxy", "cluster")
mcoMCD = NewNamespacedResource(oc.AsAdmin(), "DaemonSet", MachineConfigNamespace, "machine-config-daemon")
mcoMCC = NewNamespacedResource(oc.AsAdmin(), "Deployment", MachineConfigNamespace, "machine-config-controller")
mco = NewResource(oc.AsAdmin(), "co", "machine-config")
kubeAPIServer = NewResource(oc.AsAdmin(), "co", "kube-apiserver")
)

// Disable boot images update and skew enforcement so that the boot image controller
// does not try to reconcile machinesets while the proxy is set to a fake value, which
// would cause the controller to fail and degrade the cluster operator
exutil.By("Disable boot images update and skew enforcement")
if IsBootImageUpdateSupported(oc.AsAdmin()) {
machineConfiguration := GetMachineConfiguration(oc.AsAdmin())
defer machineConfiguration.SetSpec(machineConfiguration.GetSpecOrFail())
DisableSkew(machineConfiguration)
o.Expect(
Expand All @@ -127,25 +137,31 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati
logger.Infof("OK!\n")

exutil.By("Get current proxy configuration")
proxy := NewResource(oc.AsAdmin(), "proxy", "cluster")
proxyInitialConfig := proxy.GetOrFail(`{.spec}`)
proxyInitialConfig := proxy.GetSpecOrFail()
logger.Infof("Initial proxy configuration: %s", proxyInitialConfig)
logger.Infof("OK!\n")

wmcp := NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolWorker)
mmcp := NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolMaster)

defer func() {
defer SafeCleanup(func() {
logger.Infof("Start TC defer block")

logger.Infof("Restore original proxy config %s", proxyInitialConfig)
Comment on lines +140 to 147

@coderabbitai coderabbitai Bot Jul 31, 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.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Remove or redact sensitive cleanup output.

proxyInitialConfig can contain HTTP proxy credentials. Lines 141 and 147 write it directly to CI logs. SafeCleanup also writes collected assertion text without redaction. A failed proxy restore can then report the same sensitive configuration.

  • test/extended-priv/mco_machineconfigpool.go#L140-L147: Log that the proxy configuration was captured or restored. Do not log the full specification.
  • test/extended-priv/util.go#L327-L331: Redact sensitive values before logging or including collected failures in the final assertion message.

As per coding guidelines, flag logging that may expose passwords, tokens, API keys, PII, session IDs, internal hostnames, or customer data.

📍 Affects 2 files
  • test/extended-priv/mco_machineconfigpool.go#L140-L147 (this comment)
  • test/extended-priv/util.go#L327-L331
🤖 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_machineconfigpool.go` around lines 140 - 147, Redact
sensitive proxy details in test/extended-priv/mco_machineconfigpool.go lines
140-147 by changing the capture and restore logs to report only that the
configuration was captured or restored, without logging proxyInitialConfig. In
test/extended-priv/util.go lines 327-331, update SafeCleanup to redact sensitive
values before logging or embedding collected assertion failures in the final
message.

Source: Coding guidelines

_ = proxy.Patch("json", `[{ "op": "add", "path": "/spec", "value": `+proxyInitialConfig+`}]`)
o.Expect(proxy.SetSpec(proxyInitialConfig)).To(o.Succeed())

logger.Infof("Wait for new machine configs to be rendered and paused pools to report updated status")
// We need to make sure that the config will NOT be applied, since the proxy is a fake one and if
// we dont make sure that the config proxy is reverted, the nodes will be broken and go into
// NotReady status
_ = wmcp.WaitForUpdatedStatus()
_ = mmcp.WaitForUpdatedStatus()
o.Expect(wmcp.WaitForUpdatedStatus()).To(o.Succeed())
o.Expect(mmcp.WaitForUpdatedStatus()).To(o.Succeed())

// Proxy changes trigger a kube-apiserver rollout. Wait for it to complete so that
// ValidatingAdmissionPolicy paramRef lookups don't fail while apiserver pods are replaced.
// Before modifying the MCP again, we need to make sure that the api server can handle the ValidatingAdmissionPolicy
exutil.By("Wait for kube-apiserver rollout to complete after proxy restore")
o.Eventually(kubeAPIServer, "20m", "30s").Should(
HaveConditionField("Progressing", "status", "False"),
"kube-apiserver is still progressing after proxy change")
logger.Infof("OK!\n")

logger.Infof("Unpause worker pool")
wmcp.pause(false)
Expand All @@ -154,60 +170,64 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati
mmcp.pause(false)

logger.Infof("End TC defer block")
}()
})

exutil.By("Pause MCPs")
wmcp.pause(true)
mmcp.pause(true)
logger.Infof("OK!\n")

exutil.By("Configure new proxy")
err := proxy.Patch("json",
`[{ "op": "add", "path": "/spec/httpProxy", "value": "`+proxyValue+`" }]`)
o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching http proxy")

err = proxy.Patch("json",
`[{ "op": "add", "path": "/spec/httpsProxy", "value": "`+proxyValue+`" }]`)
o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching https proxy")

err = proxy.Patch("json",
`[{ "op": "add", "path": "/spec/noProxy", "value": "`+noProxyValue+`" }]`)
o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching noproxy")
`[{ "op": "add", "path": "/spec/httpProxy", "value": "`+proxyValue+`" },`+
`{ "op": "add", "path": "/spec/httpsProxy", "value": "`+proxyValue+`" },`+
`{ "op": "add", "path": "/spec/noProxy", "value": "`+noProxyValue+`" }]`)
o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching proxy configuration")
logger.Infof("OK!\n")

exutil.By("Verify that the proxy configuration was applied to daemonsets")
mcoDs := NewNamespacedResource(oc.AsAdmin(), "DaemonSet", MachineConfigNamespace, "machine-config-daemon")
exutil.By("Verify that the proxy configuration was applied to the MCD daemonset and the MCC deployment")
// it should never take longer than 5 minutes to apply the proxy config under any circumstance,
// it should be considered a bug.
o.Eventually(mcoDs.Poll(`{.spec}`), "5m", "30s").Should(o.ContainSubstring(proxyValue),
o.Eventually(mcoMCD.GetSpec, "5m", "30s").Should(o.ContainSubstring(proxyValue),
"machine-config-daemon is not using the new proxy configuration: %s", proxyValue)
o.Eventually(mcoDs.Poll(`{.spec}`), "5m", "30s").Should(o.ContainSubstring(noProxyValue),
o.Eventually(mcoMCD.GetSpec, "5m", "30s").Should(o.ContainSubstring(noProxyValue),
"machine-config-daemon is not using the new no-proxy value: %s", noProxyValue)
o.Eventually(mcoMCC.GetSpec, "5m", "30s").Should(o.ContainSubstring(proxyValue),
"machine-config-controller is not using the new proxy configuration: %s", proxyValue)
o.Eventually(mcoMCC.GetSpec, "5m", "30s").Should(o.ContainSubstring(noProxyValue),
"machine-config-controller is not using the new no-proxy value: %s", noProxyValue)
logger.Infof("OK!\n")

exutil.By("Check that the operator has been marked as degraded")
mco := NewResource(oc.AsAdmin(), "co", "machine-config")
o.Eventually(mco.Poll(`{.status.conditions[?(@.type=="Degraded")].status}`),
"5m", "30s").Should(o.Equal("True"),
o.Eventually(mco, "5m", "30s").Should(BeDegraded(),
"machine-config Operator should report degraded status")

o.Eventually(mco.Poll(`{.status.conditions[?(@.type=="Degraded")].message}`),
"5m", "30s").Should(o.ContainSubstring(`required MachineConfigPool master is paused and cannot sync until it is unpaused`),
o.Eventually(mco, "5m", "30s").Should(HaveDegradedMessage(o.ContainSubstring(expectedDegradedMessage)),
"machine-config Operator is not reporting the right reason for degraded status")
logger.Infof("OK!\n")

exutil.By("Restore original proxy configuration")
err = proxy.Patch("json", `[{ "op": "add", "path": "/spec", "value": `+proxyInitialConfig+`}]`)
err = proxy.SetSpec(proxyInitialConfig)
o.Expect(err).ShouldNot(o.HaveOccurred(), "Error patching and restoring original proxy config")
logger.Infof("OK!\n")

exutil.By("Verify that the new configuration is applied to the daemonset")
exutil.By("Verify that the original configuration is restored in the MCD daemonset and the MCC deployment")
// it should never take longer than 5 minutes to apply the proxy config under any circumstance,
// it should be considered a bug.
o.Eventually(mcoDs.Poll(`{.spec}`), "5m", "30s").ShouldNot(o.ContainSubstring(proxyValue),
o.Eventually(mcoMCD.GetSpec, "5m", "30s").ShouldNot(o.ContainSubstring(proxyValue),
"machine-config-daemon has not restored the original proxy configuration")
o.Eventually(mcoDs.Poll(`{.spec}`), "5m", "30s").ShouldNot(o.ContainSubstring(noProxyValue),
o.Eventually(mcoMCD.GetSpec, "5m", "30s").ShouldNot(o.ContainSubstring(noProxyValue),
"machine-config-daemon has not restored the original proxy configuration for 'no-proxy'")
o.Eventually(mcoMCC.GetSpec, "5m", "30s").ShouldNot(o.ContainSubstring(proxyValue),
"machine-config-controller has not restored the original proxy configuration")
o.Eventually(mcoMCC.GetSpec, "5m", "30s").ShouldNot(o.ContainSubstring(noProxyValue),
"machine-config-controller has not restored the original proxy configuration for 'no-proxy'")
logger.Infof("OK!\n")

exutil.By("Check that the operator is not marked as degraded anymore")
o.Eventually(mco.Poll(`{.status.conditions[?(@.type=="Degraded")].status}`),
"5m", "30s").Should(o.Equal("False"),
o.Eventually(mco, "5m", "30s").ShouldNot(BeDegraded(),
"machine-config Operator should not report degraded status anymore")
logger.Infof("OK!\n")

})

Expand Down
19 changes: 3 additions & 16 deletions test/extended-priv/mco_scale.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,23 +472,10 @@ func SimpleScaleUPTest(oc *exutil.CLI, mcp *MachineConfigPool, imageVersion, ign
logger.Infof("OK!\n")
}

defer func() {
logger.Infof("Start TC defer block")
defer SafeCleanup(func() {
newMs := NewMachineSet(oc.AsAdmin(), MachineAPINamespace, newMsName)
errors := o.InterceptGomegaFailures(func() { removeClonedMachineSet(newMs, mcp, initialNumWorkers) }) // We don't want gomega to fail and stop the deferred cleanup process
if len(errors) != 0 {
logger.Infof("There were errors restoring the original MachineSet resources in the cluster")
for _, e := range errors {
logger.Errorf(e)
}
}

// We don't want the test to pass if there were errors while restoring the initial state
o.Expect(len(errors)).To(o.BeZero(),
"There were %d errors while recovering the cluster's initial state", len(errors))

logger.Infof("End TC defer block")
}()
removeClonedMachineSet(newMs, mcp, initialNumWorkers)
})

logger.Infof("Create a new MachineSet using the right base image")
allMs, err := NewMachineSetList(oc.AsAdmin(), MachineAPINamespace).GetAll()
Expand Down
12 changes: 10 additions & 2 deletions test/extended-priv/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,17 @@ func (r *Resource) DeleteOrFail(extraParams ...string) {
o.Expect(err).NotTo(o.HaveOccurred())
}

// GetSpecOrFail returns the resource's spec as a JSON string
// GetSpecOrFail returns the resource's spec as a JSON string and fails the test if any error happens
func (r Resource) GetSpecOrFail() string {
return r.GetOrFail(`{.spec}`)
spec, err := r.GetSpec()
o.Expect(err).NotTo(o.HaveOccurred())

return spec
}

// GetSpec returns the resource's spec as a JSON string
func (r Resource) GetSpec() (string, error) {
return r.Get(`{.spec}`)
}

// SetSpec replace the current resource's spec with the provided JSON string spec
Expand Down
12 changes: 12 additions & 0 deletions test/extended-priv/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,18 @@ func IsTrue(s string) bool {
return strings.EqualFold(s, TrueString)
}

// SafeCleanup executes the given cleanup function ensuring all statements run even if some assertions fail.
// It collects all gomega failures and reports them at the end, failing the test if any occurred.
// Use this in defer blocks instead of raw o.Expect calls to prevent cleanup from being interrupted.
func SafeCleanup(cleanup func()) {
errors := o.InterceptGomegaFailures(cleanup)
for _, e := range errors {
logger.Errorf("Cleanup error: %s", e)
}
o.Expect(len(errors)).To(o.BeZero(),
"There were %d errors during cleanup:\n%s", len(errors), strings.Join(errors, "\n"))
}

// IsSNOSafe returns true if the cluster is a SNO cluster. Instead of failing, it returns an error if we can't know if the cluster is SNO or not
func IsSNOSafe(oc *exutil.CLI) (bool, error) {
allNodes, err := NewNodeList(oc.AsAdmin()).GetAll()
Expand Down