[4.22] OCPBUGS-59514: redacted install config for baremetal by iurygregory · Pull Request #9972 · openshift/installer

iurygregory · 2025-09-29T01:43:40Z

No description provided.

openshift-ci-robot · 2025-09-29T01:43:46Z

@iurygregory: This pull request references Jira Issue OCPBUGS-59514, which is invalid:

expected the bug to target the "4.21.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

iurygregory · 2025-09-29T01:48:29Z

/jira refresh

openshift-ci-robot · 2025-09-29T01:48:38Z

@iurygregory: This pull request references Jira Issue OCPBUGS-59514, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.21.0) matches configured target version for branch (4.21.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jadhaj

Details

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

patrickdillon

/label backport-risk-assessed

Hm perhaps I'm missing something obvious, but could you simply do this:

	case newConfig.BareMetal != nil:
		for _, host := range newConfig.BareMetal.Hosts {
			host.BMC.Password = "REDACTED"
		}

This way you would not need to update this code for future fields. Not sure why other platforms don't do this, so perhaps I am missing something!

iurygregory · 2025-09-29T16:19:25Z

@patrickdillon thanks for the feedback, let me push a change with your idea and test via cluster-bot!

zaneb · 2025-09-29T23:08:24Z

It looks like another instance of this problem was already fixed by OCPBUGS-61353, but there is still one more that this doesn't solve.

Clearly this is going to keep happening though. I don't feel like this is fixed until we have code that uses reflect to go through the whole structure and redact any fields with a json tag that contains user or password.

This way you would not need to update this code for future fields. Not sure why other platforms don't do this, so perhaps I am missing something!

I think the problem is that this changes the original install-config, meaning the redacted version will get written to the asset store. The InstallConfig struct is passed by value, so it is safe to overwrite the pullSecret at the top level, but it contains references to nested structs (Platform struct is embedded by value, but it contains pointers to the actual platform implementation), and only the references get copied.

We need to either go through (a hopefully better version of) the same rigmarole as the other platforms or generate DeepCopy() functions for the InstallConfig. The latter would be vastly preferable, since the current code is extremely brittle if not already broken (e.g. Port field is missing from VCenters).

barbacbd

This seems ok. Is there any way you can add some commit notes maybe why this is needed? I also like Patrick's comment to add Redacted instead of an empty string; I think it gives an indication for why it was set the way that it was.

patrickdillon · 2025-09-30T18:02:24Z

Platform struct is embedded by value, but it contains pointers to the actual platform implementation

+1. I knew I was missing something and this point was exactly it. Ignore my suggestion!

patrickdillon · 2025-09-30T18:03:46Z

& apologies for misleading you @iurygregory

tthvo · 2025-11-05T01:40:36Z

We need to either go through (a hopefully better version of) the same rigmarole as the other platforms or generate DeepCopy() functions for the InstallConfig. The latter would be vastly preferable, since the current code is extremely brittle if not already broken (e.g. Port field is missing from VCenters).

Looks like you predicted #10025, Zane. This PR should make the whole copying much easier and individual overrides can be done just like Patrick's previous suggestion.

tthvo · 2025-11-05T01:48:12Z

Clearly this is going to keep happening though. I don't feel like this is fixed until we have code that uses reflect to go through the whole structure and redact any fields with a json tag that contains user or password.

Looks like a good scenarios to use Claude 😁

Current approach looks OK to me though resolve the issue today unless you want to wait for #10025, @iurygregory

iurygregory · 2025-11-06T14:00:02Z

Clearly this is going to keep happening though. I don't feel like this is fixed until we have code that uses reflect to go through the whole structure and redact any fields with a json tag that contains user or password.

Looks like a good scenarios to use Claude 😁

Current approach looks OK to me though resolve the issue today unless you want to wait for #10025, @iurygregory

Hey @tthvo , I'll be waiting for #10025 to land so I can update things!

patrickdillon · 2025-11-12T19:42:13Z

I'm not sure how much #10025 will help. I mean that I honestly don't know.

I did have a thought though that JSON serialization would be a just fine way to copy one go struct to another, right?

edit: Ok clearly #10025 will help, and should merge soon.

iurygregory · 2025-11-14T11:42:32Z

I'm not sure how much #10025 will help. I mean that I honestly don't know.

I did have a thought though that JSON serialization would be a just fine way to copy one go struct to another, right?

edit: Ok clearly #10025 will help, and should merge soon.

Thanks @patrickdillon I will wait for #10025 to merge so I can update this one.

tthvo · 2025-12-02T17:45:51Z

@iurygregory #10025 is merged. You can use deepcopy now :D

iurygregory · 2025-12-10T12:00:36Z

@tthvo thanks! going to update here

patrickdillon · 2026-01-07T17:07:37Z

/test ?

patrickdillon · 2026-01-07T17:09:45Z

/test e2e-metal-ipi-ovn e2e-vsphere-ovn e2e-nutanix-ovn

sadasu · 2026-01-27T16:02:12Z

@iurygregory could the unit tests be updated to cover vSphere and Nutanix? And then we should be OK using passing unit tests to add the verified label?

iurygregory · 2026-01-30T12:16:40Z

@sadasu will update

Assisted-By: Claude Code Sonnet 4 Signed-off-by: Iury Gregory Melo Ferreira <imelofer@redhat.com>

iurygregory · 2026-02-04T02:24:03Z

/retest-required

iurygregory · 2026-02-05T11:31:10Z

/jira refresh

openshift-ci-robot · 2026-02-05T11:31:16Z

@iurygregory: This pull request references Jira Issue OCPBUGS-59514, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.22.0) matches configured target version for branch (4.22.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jadhaj

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

iurygregory · 2026-02-05T11:31:30Z

/retest-required

jadhaj · 2026-02-05T11:43:33Z

/verified later @jadhaj

openshift-ci-robot · 2026-02-05T11:43:46Z

@jadhaj: This PR has been marked to be verified later by @jadhaj.

Details

In response to this:

/verified later @jadhaj

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2026-02-06T21:11:30Z

@iurygregory: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-vsphere-ovn-techpreview	`18cc555`	link	false	`/test e2e-vsphere-ovn-techpreview`
ci/prow/e2e-aws-ovn-fips	`18cc555`	link	false	`/test e2e-aws-ovn-fips`
ci/prow/aws-private	`18cc555`	link	false	`/test aws-private`
ci/prow/e2e-aws-default-config	`18cc555`	link	false	`/test e2e-aws-default-config`
ci/prow/e2e-aws-ovn-shared-vpc-custom-security-groups	`18cc555`	link	false	`/test e2e-aws-ovn-shared-vpc-custom-security-groups`
ci/prow/e2e-aws-ovn-heterogeneous	`18cc555`	link	false	`/test e2e-aws-ovn-heterogeneous`
ci/prow/e2e-ibmcloud-ovn	`18cc555`	link	false	`/test e2e-ibmcloud-ovn`
ci/prow/e2e-openstack-ovn	`18cc555`	link	true	`/test e2e-openstack-ovn`
ci/prow/azure-ovn-marketplace-images	`18cc555`	link	false	`/test azure-ovn-marketplace-images`
ci/prow/e2e-azure-ovn	`18cc555`	link	true	`/test e2e-azure-ovn`
ci/prow/e2e-aws-ovn-shared-vpc-edge-zones	`18cc555`	link	false	`/test e2e-aws-ovn-shared-vpc-edge-zones`
ci/prow/e2e-vsphere-ovn-disk-setup-techpreview	`18cc555`	link	false	`/test e2e-vsphere-ovn-disk-setup-techpreview`
ci/prow/azure-private	`18cc555`	link	false	`/test azure-private`
ci/prow/e2e-metal-single-node-live-iso	`18cc555`	link	false	`/test e2e-metal-single-node-live-iso`
ci/prow/e2e-vsphere-multi-vcenter-ovn	`18cc555`	link	false	`/test e2e-vsphere-multi-vcenter-ovn`
ci/prow/e2e-azurestack	`18cc555`	link	false	`/test e2e-azurestack`
ci/prow/e2e-aws-byo-subnet-role-security-groups	`18cc555`	link	false	`/test e2e-aws-byo-subnet-role-security-groups`
ci/prow/e2e-azure-ovn-shared-vpc	`18cc555`	link	false	`/test e2e-azure-ovn-shared-vpc`
ci/prow/e2e-aws-ovn-single-node	`18cc555`	link	false	`/test e2e-aws-ovn-single-node`
ci/prow/e2e-aws-ovn-edge-zones-manifest-validation	`18cc555`	link	true	`/test e2e-aws-ovn-edge-zones-manifest-validation`
ci/prow/e2e-aws-ovn-edge-zones	`18cc555`	link	false	`/test e2e-aws-ovn-edge-zones`
ci/prow/okd-scos-e2e-aws-ovn	`5e4737a`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/e2e-vsphere-ovn-hybrid-env	`18cc555`	link	false	`/test e2e-vsphere-ovn-hybrid-env`
ci/prow/openstack-manifests	`18cc555`	link	true	`/test openstack-manifests`
ci/prow/e2e-openstack-proxy	`18cc555`	link	false	`/test e2e-openstack-proxy`
ci/prow/e2e-vsphere-ovn-zones	`18cc555`	link	false	`/test e2e-vsphere-ovn-zones`
ci/prow/e2e-azure-default-config	`18cc555`	link	false	`/test e2e-azure-default-config`
ci/prow/e2e-openstack-nfv-intel	`18cc555`	link	false	`/test e2e-openstack-nfv-intel`
ci/prow/e2e-aws-ovn-imdsv2	`18cc555`	link	false	`/test e2e-aws-ovn-imdsv2`
ci/prow/e2e-nutanix-ovn	`4b09c16`	link	false	`/test e2e-nutanix-ovn`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

iurygregory · 2026-02-09T01:05:38Z

/test artifacts-images

iurygregory · 2026-02-09T01:05:48Z

/test okd-scos-images

iurygregory · 2026-02-09T01:06:13Z

@sadasu let me know if this address what you requested. Tks!

iurygregory · 2026-02-11T11:39:04Z

/test okd-scos-images

iurygregory · 2026-02-11T11:45:06Z

@patrickdillon can you take another look? Thanks!

iurygregory · 2026-03-18T11:37:42Z

@patrickdillon @sadasu can you take another look at this? Tks!

patrickdillon · 2026-03-24T13:16:36Z

/lgtm

coderabbitai · 2026-03-24T13:16:58Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ec6429d5-1ded-4ee0-9a98-381c4f4718e1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

openshift-ci-robot · 2026-03-24T21:18:30Z

@iurygregory: Jira Issue OCPBUGS-59514: All pull requests linked via external trackers have merged:

openshift/installer#9972

This pull request has the verified-later tag and will need to be manually moved to VERIFIED after testing. Jira Issue OCPBUGS-59514 has been moved to the MODIFIED state.

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Sep 29, 2025

openshift-ci bot requested review from barbacbd, bfournie and jadhaj September 29, 2025 01:48

iurygregory force-pushed the OCPBUGS-59514 branch from 2d85a68 to 2f70ace Compare September 29, 2025 01:53

patrickdillon reviewed Sep 29, 2025

View reviewed changes

openshift-ci bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Sep 29, 2025

iurygregory force-pushed the OCPBUGS-59514 branch from 2f70ace to 749c837 Compare September 29, 2025 16:34

barbacbd reviewed Sep 30, 2025

View reviewed changes

iurygregory force-pushed the OCPBUGS-59514 branch 3 times, most recently from 1cb7810 to 5e4737a Compare October 24, 2025 16:09

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 7, 2026

OCPBUGS-59514: redacted install config

6e0e314

Assisted-By: Claude Code Sonnet 4 Signed-off-by: Iury Gregory Melo Ferreira <imelofer@redhat.com>

iurygregory force-pushed the OCPBUGS-59514 branch from 4b09c16 to 6e0e314 Compare February 3, 2026 16:04

iurygregory changed the title ~~[4.21] OCPBUGS-59514: redacted install config for baremetal~~ [4.22] OCPBUGS-59514: redacted install config for baremetal Feb 3, 2026

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2026

openshift-ci-robot added verified-later verified Signifies that the PR passed pre-merge verification criteria labels Feb 5, 2026

openshift-ci bot assigned patrickdillon Mar 24, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 24, 2026

openshift-merge-bot bot merged commit 6486502 into openshift:main Mar 24, 2026
15 checks passed

Conversation

iurygregory commented Sep 29, 2025

Uh oh!

openshift-ci-robot commented Sep 29, 2025

Uh oh!

iurygregory commented Sep 29, 2025

Uh oh!

openshift-ci-robot commented Sep 29, 2025

Uh oh!

patrickdillon left a comment

Choose a reason for hiding this comment

Uh oh!

iurygregory commented Sep 29, 2025

Uh oh!

zaneb commented Sep 29, 2025

Uh oh!

barbacbd left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickdillon commented Sep 30, 2025

Uh oh!

patrickdillon commented Sep 30, 2025

Uh oh!

tthvo commented Nov 5, 2025

Uh oh!

tthvo commented Nov 5, 2025

Uh oh!

iurygregory commented Nov 6, 2025

Uh oh!

patrickdillon commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iurygregory commented Nov 14, 2025

Uh oh!

tthvo commented Dec 2, 2025

Uh oh!

iurygregory commented Dec 10, 2025

Uh oh!

patrickdillon commented Jan 7, 2026

Uh oh!

patrickdillon commented Jan 7, 2026

Uh oh!

sadasu commented Jan 27, 2026

Uh oh!

iurygregory commented Jan 30, 2026

Uh oh!

iurygregory commented Feb 4, 2026

Uh oh!

iurygregory commented Feb 5, 2026

Uh oh!

openshift-ci-robot commented Feb 5, 2026

Uh oh!

iurygregory commented Feb 5, 2026

Uh oh!

jadhaj commented Feb 5, 2026

Uh oh!

openshift-ci-robot commented Feb 5, 2026

Uh oh!

openshift-ci bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iurygregory commented Feb 9, 2026

Uh oh!

iurygregory commented Feb 9, 2026

Uh oh!

iurygregory commented Feb 9, 2026

Uh oh!

iurygregory commented Feb 11, 2026

Uh oh!

iurygregory commented Feb 11, 2026

Uh oh!

iurygregory commented Mar 18, 2026

Uh oh!

patrickdillon commented Mar 24, 2026

Uh oh!

coderabbitai bot commented Mar 24, 2026

Review skipped

Uh oh!

Uh oh!

barbacbd left a comment •

edited

Loading

patrickdillon commented Nov 12, 2025 •

edited

Loading

openshift-ci bot commented Feb 6, 2026 •

edited

Loading