Skip to content

Conversation

pablintino
Copy link
Contributor

@pablintino pablintino commented Sep 24, 2025

  • What I did

This change adds the missing pieces of the mco-sanitizer to finish its implementation, including encryption and a brief README.

  • How to verify it
  1. Create an OCP cluster (the version is not relevant) and create a must-gather of it. Preserve a separated copy of it, as to properly test it we need to run the sanitizer twice.
  2. Run the tool: ./mco-sanitize --input /path/to/must-gather-directory. Check if the CRs of the MCO directories are properly sanitized and that are safe to push.
  3. Restore the content of the must-gather with the backup.
  4. Run the tool: ./mco-sanitize --input /path/to/must-gather-directory --output encrypted.tar.gz. Check that the output file is encrypted.
  • Description for the changelog

Implements the main functionality of the mco-sanitizer.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 24, 2025
@pablintino pablintino force-pushed the devex-mco-sanitize-main branch from 34153ab to e348dff Compare September 24, 2025 12:04
@pablintino pablintino changed the title Devex mco sanitize main MCO-1685: Add mco-sanitize utility main logic Sep 24, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 24, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 24, 2025

@pablintino: This pull request references MCO-1685 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

In response to this:

  • What I did

This change adds the main content of the mco-sanitizer, the utility binary that takes a must-gather directory and removes/sanitizes the content based on some given configuration.
This change is is missing the encryption part and a proper README that will come with a follow up PR.

  • How to verify it

UT testing is enough to test this PR.

  • Description for the changelog

Implements the main functionality of the mco-sanitizer.

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.

Copy link
Member

@isabella-janssen isabella-janssen left a comment

Choose a reason for hiding this comment

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

/lgtm

The changes look fair & I see that the unit tests for the new functionality are passing.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 25, 2025
@pablintino
Copy link
Contributor Author

/verified bypass
The content delivered by this PR is tested by UTs. The entire content will be tested by QE when the entire feature is pushed.

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Sep 25, 2025
@openshift-ci-robot
Copy link
Contributor

@pablintino: The verified label has been added.

In response to this:

/verified bypass
The content delivered by this PR is tested by UTs. The entire content will be tested by QE when the entire feature is pushed.

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.

@pablintino
Copy link
Contributor Author

/retest-required

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 4f14943 and 2 for PR HEAD e348dff in total

@pablintino pablintino force-pushed the devex-mco-sanitize-main branch from e348dff to 52e59dd Compare September 26, 2025 10:27
@openshift-ci-robot openshift-ci-robot removed the verified Signifies that the PR passed pre-merge verification criteria label Sep 26, 2025
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 26, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 26, 2025

@pablintino: This pull request references MCO-1685 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

In response to this:

  • What I did

This change adds the missing pieces of the mco-sanitizer to finish its implementation, including encryption and a brief README.

  • How to verify it
  1. Create an OCP cluster (the version is not relevant) and create a must-gather of it. Preserve a separated copy of it, as to properly test it we need to run the sanitizer twice.
  2. Run the tool: ./mco-sanitize --input /path/to/must-gather-directory. Check if the CRs of the MCO directories are properly sanitized and that are safe to push.
  3. Restore the content of the must-gather with the backup.
  4. Run the tool: ./mco-sanitize --input /path/to/must-gather-directory --output encrypted.tar.gz. Check that the output file is encrypted.
  • Description for the changelog

Implements the main functionality of the mco-sanitizer.

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.

@pablintino pablintino force-pushed the devex-mco-sanitize-main branch 4 times, most recently from 5e88be9 to 018d259 Compare September 29, 2025 15:13
@pablintino
Copy link
Contributor Author

/retest-required

@ptalgulk01
Copy link

Pre-merge tested

Steps to use the tool

  • Pull the PR
$ go build -o mco-sanitize ./devex/cmd/mco-sanitize
$ ./mco-sanitize  --help
Removes MCO sensitive information from a must-gather report

Usage:
  mco-sanitize [flags]

Flags:
  -h, --help                           help for mco-sanitize
      --input string                   Path to the must-gather directory.
      --log-flush-frequency duration   Maximum number of seconds between log flushes (default 5s)
      --output string                  Path to where the tar.gz output should be saved.
  -v, --v Level                        number for the log level verbosity
      --vmodule moduleSpec             comma-separated list of pattern=N settings for file-filtered logging (only works for the default text log format)
      --workers int                    Worker count. Defaults to CPU core count. (default 16)
  • Passed the must-gather path
$ ./mco-sanitize --input ~/Downloads/kubeconfig/mco-san
I0929 17:02:32.384842  240444 walker.go:133] file quay-io-openshift-release-dev-ocp-v4-0-art-dev-sha256-481f13f1ad180f6d58021969d12153a4a19072b6328de279ca4a8da10cbcf0d7/cluster-scoped-resources/machineconfiguration.openshift.io/machineconfigs/97-worker-generated-kubelet.yaml redacted
I0929 17:02:32.384990  240444 walker.go:133] file quay-io-openshift-release-dev-ocp-v4-0-art-dev-sha256-481f13f1ad180f6d58021969d12153a4a19072b6328de279ca4a8da10cbcf0d7/cluster-scoped-resources/machineconfiguration.openshift.io/machineconfigs/01-master-container-runtime.yaml redacted
....

WITH REDACTED

$ omc get mc 98-master-generated-kubelet -o yaml
spec:
 ...
    storage:
      files:
      - contents:
          _REDACTED: This field has been redacted
          length: 6502

WITHOUT REDACTED

$ omc get mc 98-master-generated-kubelet -o yaml
spec:
 ...
    storage:
      files:
      - contents:
          compression: ""
          source: data:text/plain;charset=utf-8;base64,YXBpVmVyc2lvbjoga3ViZWxldC5jb25maWcuazhzLm
  • To get the encrypted tar file and to decrypt the tar file
$ gpg --full-generate-key # generate your gpg-key
$  gpg --armour --export ,email.id> public-key.asc
$ export MCO_MUST_GATHER_SANITIZER_KEY="$(base64 -w 0 < public-key.asc )"
$  echo $MCO_MUST_GATHER_SANITIZER_KEY   
$ ./mco-sanitize --input mco-san --output encrypted.tar.gz
$ gpg --decrypt encrypted.tar.gz > decrypted.tar.gz    
$ tar -xvf  decrypted.tar.gz

/qe-approved

Copy link
Member

@isabella-janssen isabella-janssen left a comment

Choose a reason for hiding this comment

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

/lgtm

Thanks for adding a readme!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2025
@pablintino
Copy link
Contributor Author

/verified bypass
Tested by QE at #5303 (comment)

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Sep 30, 2025
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD adb087c and 1 for PR HEAD 018d259 in total

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2025
@yuqi-zhang
Copy link
Contributor

ah, looks like we have a conflict. Can override the failing tests once that's been resolved.

This changes adds encryption to the output of the sanitizer.
For now, till the mco-sanitize tool is properly validated in CI,
encryption cannot be disabled.
@pablintino pablintino force-pushed the devex-mco-sanitize-main branch from 018d259 to 2e453ab Compare October 1, 2025 07:34
@openshift-ci-robot openshift-ci-robot removed the verified Signifies that the PR passed pre-merge verification criteria label Oct 1, 2025
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 1, 2025
@pablintino
Copy link
Contributor Author

@yuqi-zhang how inconvenient :(. Fixed

Copy link
Member

@isabella-janssen isabella-janssen left a comment

Choose a reason for hiding this comment

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

/lgtm

Tagged was removed with a rebase, so I'm adding it back.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2025
Copy link
Contributor

openshift-ci bot commented Oct 1, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: isabella-janssen, pablintino

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [isabella-janssen,pablintino]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@isabella-janssen
Copy link
Member

/retest-required

@isabella-janssen
Copy link
Member

@pablintino I think your verified label was stripped in the rebase.

@pablintino
Copy link
Contributor Author

/verified by QE at #5303 (comment)

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Oct 1, 2025
@openshift-ci-robot
Copy link
Contributor

@pablintino: This PR has been marked as verified by QE at https://github.com/openshift/machine-config-operator/pull/5303#issuecomment-3352336245.

In response to this:

/verified by QE at #5303 (comment)

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.

@pablintino
Copy link
Contributor Author

/retest-required

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 1c25a90 and 2 for PR HEAD 2e453ab in total

@pablintino
Copy link
Contributor Author

/retest-required

@pablintino
Copy link
Contributor Author

/retest

@pablintino
Copy link
Contributor Author

/override ci/prow/e2e-aws-ovn-upgrade
/override ci/prow/e2e-aws-ovn

Copy link
Contributor

openshift-ci bot commented Oct 2, 2025

@pablintino: Overrode contexts on behalf of pablintino: ci/prow/e2e-aws-ovn, ci/prow/e2e-aws-ovn-upgrade

In response to this:

/override ci/prow/e2e-aws-ovn-upgrade
/override ci/prow/e2e-aws-ovn

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.

@djoshy
Copy link
Contributor

djoshy commented Oct 2, 2025

/override ci/prow/e2e-aws-ovn-upgrade
/override ci/prow/e2e-aws-ovn

overriding again as another PR merged, causing reruns

Copy link
Contributor

openshift-ci bot commented Oct 2, 2025

@djoshy: Overrode contexts on behalf of djoshy: ci/prow/e2e-aws-ovn, ci/prow/e2e-aws-ovn-upgrade

In response to this:

/override ci/prow/e2e-aws-ovn-upgrade
/override ci/prow/e2e-aws-ovn

overriding again as another PR merged, causing reruns

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.

@openshift-merge-bot openshift-merge-bot bot merged commit b6edf8e into openshift:main Oct 2, 2025
16 of 22 checks passed
Copy link
Contributor

openshift-ci bot commented Oct 2, 2025

@pablintino: 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-aws-mco-disruptive 2e453ab link false /test e2e-aws-mco-disruptive
ci/prow/e2e-gcp-op-ocl 2e453ab link false /test e2e-gcp-op-ocl
ci/prow/e2e-azure-ovn-upgrade-out-of-change 2e453ab link false /test e2e-azure-ovn-upgrade-out-of-change
ci/prow/bootstrap-unit 2e453ab link false /test bootstrap-unit
ci/prow/e2e-aws-ovn-upgrade-out-of-change 2e453ab link false /test e2e-aws-ovn-upgrade-out-of-change
ci/prow/e2e-gcp-mco-disruptive 2e453ab link false /test e2e-gcp-mco-disruptive

Full PR test history. Your PR dashboard.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants