SPLAT-1429: Adds flag to omit credentials in logging #8287

faermanj · 2024-04-18T15:03:23Z

Fixes SPLAT-1429 by adding a flag that allow users to omit password from logging.

barbacbd · 2024-04-18T18:44:30Z

cmd/openshift-install/create.go

@@ -288,6 +291,9 @@ func newCreateCmd(ctx context.Context) *cobra.Command {
 	for _, t := range targets {
 		t.command.Args = cobra.ExactArgs(0)
 		t.command.Run = runTargetCmd(ctx, t.assets...)
+		if "Cluster" == t.name {
+			cmd.PersistentFlags().BoolVar(&skipPasswordPrintFlag, "skip-password-print", false, "Do not print the generated user password.")


Suggested change

cmd.PersistentFlags().BoolVar(&skipPasswordPrintFlag, "skip-password-print", false, "Do not print the generated user password.")

cmd.PersistentFlags().BoolVar(&skipPasswordPrintFlag, "skip-password-print", true, "Do not print the generated user password.")

Don't we want to default this to true to skip the password printing by default?

@barbacbd yoda condition fixed, thanks.
But I'd say it's safer to leave false as the default, at least for one release, as user scripts/hacks may depend on that.

I am fine with that, thanks.

4.16 where this will land is a LTS release. So, it is better to have the default to true to skip printing the password sooner rather than a later release.
Also, can we call the the flag printUserPassword which is set to false by default? User scripts can be updated to specifically turn on password printing.

barbacbd

/lgtm

barbacbd · 2024-04-22T13:50:32Z

cmd/openshift-install/create.go

@@ -288,6 +291,9 @@ func newCreateCmd(ctx context.Context) *cobra.Command {
 	for _, t := range targets {
 		t.command.Args = cobra.ExactArgs(0)
 		t.command.Run = runTargetCmd(ctx, t.assets...)
+		if "Cluster" == t.name {
+			cmd.PersistentFlags().BoolVar(&skipPasswordPrintFlag, "skip-password-print", false, "Do not print the generated user password.")


I am fine with that, thanks.

barbacbd · 2024-04-22T13:52:45Z

/retitle no-jira: Adds flag to omit credentials in logging

openshift-ci-robot · 2024-04-22T13:53:35Z

@faermanj: This pull request explicitly references no jira issue.

In response to this:

Fixes SPLAT-1429 by adding a flag that allow users to omit password from logging.

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.

barbacbd · 2024-04-22T13:58:50Z

/retest-required

barbacbd · 2024-04-22T14:01:21Z

/lgtm

zaneb · 2024-04-25T22:42:05Z

If this "Fixes SPLAT-1429" then how can it be no-jira?

cmd/openshift-install/create.go

r4f4 · 2024-04-30T08:24:34Z

/retitle SPLAT-1429: Adds flag to omit credentials in logging

openshift-ci-robot · 2024-04-30T08:24:52Z

@faermanj: This pull request references SPLAT-1429 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 epic to target the "4.16.0" version, but no target version was set.

In response to this:

Fixes SPLAT-1429 by adding a flag that allow users to omit password from logging.

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.

r4f4 · 2024-04-30T08:26:21Z

/hold
for the fixes.

Co-authored-by: Rafael F. <r4f4rfs@gmail.com>

r4f4

/lgtm

r4f4 · 2024-05-02T15:11:07Z

/label tide/merge-method-squash
/hold cancel

openshift-ci · 2024-05-02T18:27:02Z

@faermanj: The following test 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/okd-e2e-aws-ovn-upgrade	`d3788e2`	link	false	`/test okd-e2e-aws-ovn-upgrade`

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/test-infra repository. I understand the commands that are listed here.

patrickdillon · 2024-05-03T12:29:01Z

/approve

openshift-ci · 2024-05-03T12:29:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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~~ [patrickdillon]

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

* Adds flag to omit credentials in logging * Update cmd/openshift-install/create.go Co-authored-by: Rafael F. <r4f4rfs@gmail.com> * Update cmd/openshift-install/create.go Co-authored-by: Rafael F. <r4f4rfs@gmail.com> --------- Co-authored-by: Rafael F. <r4f4rfs@gmail.com>

openshift-ci bot requested review from AnnaZivkovic and barbacbd April 18, 2024 15:04

barbacbd reviewed Apr 18, 2024

View reviewed changes

Adds flag to omit credentials in logging

8a51b42

faermanj force-pushed the skip-password-print branch from f9726e6 to 8a51b42 Compare April 19, 2024 13:08

faermanj requested a review from barbacbd April 19, 2024 13:24

barbacbd reviewed Apr 22, 2024

View reviewed changes

openshift-ci bot changed the title ~~Adds flag to omit credentials in logging~~ no-jira: Adds flag to omit credentials in logging Apr 22, 2024

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 22, 2024

openshift-ci bot assigned barbacbd Apr 22, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 22, 2024

r4f4 reviewed Apr 30, 2024

View reviewed changes

cmd/openshift-install/create.go Outdated Show resolved Hide resolved

cmd/openshift-install/create.go Outdated Show resolved Hide resolved

openshift-ci bot changed the title ~~no-jira: Adds flag to omit credentials in logging~~ SPLAT-1429: Adds flag to omit credentials in logging Apr 30, 2024

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 30, 2024

Update cmd/openshift-install/create.go

87acdd3

Co-authored-by: Rafael F. <r4f4rfs@gmail.com>

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 2, 2024

faermanj and others added 2 commits May 2, 2024 16:55

Update cmd/openshift-install/create.go

9c5cf69

Co-authored-by: Rafael F. <r4f4rfs@gmail.com>

Merge branch 'openshift:master' into skip-password-print

d3788e2

r4f4 reviewed May 2, 2024

View reviewed changes

openshift-ci bot assigned r4f4 May 2, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 2, 2024

openshift-ci bot added tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels May 2, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 3, 2024

openshift-merge-bot bot merged commit 51c6a5b into openshift:master May 3, 2024
19 of 20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SPLAT-1429: Adds flag to omit credentials in logging #8287

SPLAT-1429: Adds flag to omit credentials in logging #8287

faermanj commented Apr 18, 2024 •

edited by openshift-ci bot

barbacbd Apr 18, 2024

faermanj Apr 19, 2024

barbacbd Apr 22, 2024

sadasu Apr 24, 2024

barbacbd left a comment •

edited

barbacbd Apr 22, 2024

barbacbd commented Apr 22, 2024

openshift-ci-robot commented Apr 22, 2024

barbacbd commented Apr 22, 2024

barbacbd commented Apr 22, 2024

zaneb commented Apr 25, 2024

r4f4 commented Apr 30, 2024 •

edited by openshift-ci bot

openshift-ci-robot commented Apr 30, 2024 •

edited by openshift-ci bot

r4f4 commented Apr 30, 2024

r4f4 left a comment

r4f4 commented May 2, 2024

openshift-ci bot commented May 2, 2024

patrickdillon commented May 3, 2024

openshift-ci bot commented May 3, 2024

	cmd.PersistentFlags().BoolVar(&skipPasswordPrintFlag, "skip-password-print", false, "Do not print the generated user password.")
	cmd.PersistentFlags().BoolVar(&skipPasswordPrintFlag, "skip-password-print", true, "Do not print the generated user password.")

SPLAT-1429: Adds flag to omit credentials in logging #8287

SPLAT-1429: Adds flag to omit credentials in logging #8287

Conversation

faermanj commented Apr 18, 2024 • edited by openshift-ci bot

barbacbd Apr 18, 2024

Choose a reason for hiding this comment

faermanj Apr 19, 2024

Choose a reason for hiding this comment

barbacbd Apr 22, 2024

Choose a reason for hiding this comment

sadasu Apr 24, 2024

Choose a reason for hiding this comment

barbacbd left a comment • edited

Choose a reason for hiding this comment

barbacbd Apr 22, 2024

Choose a reason for hiding this comment

barbacbd commented Apr 22, 2024

openshift-ci-robot commented Apr 22, 2024

barbacbd commented Apr 22, 2024

barbacbd commented Apr 22, 2024

zaneb commented Apr 25, 2024

r4f4 commented Apr 30, 2024 • edited by openshift-ci bot

openshift-ci-robot commented Apr 30, 2024 • edited by openshift-ci bot

r4f4 commented Apr 30, 2024

r4f4 left a comment

Choose a reason for hiding this comment

r4f4 commented May 2, 2024

openshift-ci bot commented May 2, 2024

patrickdillon commented May 3, 2024

openshift-ci bot commented May 3, 2024

faermanj commented Apr 18, 2024 •

edited by openshift-ci bot

barbacbd left a comment •

edited

r4f4 commented Apr 30, 2024 •

edited by openshift-ci bot

openshift-ci-robot commented Apr 30, 2024 •

edited by openshift-ci bot