Skip to content
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.

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

CFE-1022: UPSTREAM: 640: Add labels from command line option to filestore backup resource #41

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

bharath-b-rh
Copy link

Backport of the upstream change kubernetes-sigs#640 which fixes the issue kubernetes-sigs#639.

This change enables adding labels from command line option to filestore backup resource created for an OpenShift Cluster.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 13, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 13, 2024

@bharath-b-rh: This pull request references CFE-1022 which is a valid jira issue.

In response to this:

Backport of the upstream change kubernetes-sigs#640 which fixes the issue kubernetes-sigs#639.

This change enables adding labels from command line option to filestore backup resource created for an OpenShift Cluster.

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 openshift-ci bot requested review from mpatlasov and tsmetana July 13, 2024 07:28
@mpatlasov
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 15, 2024
Copy link

openshift-ci bot commented Jul 15, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bharath-b-rh, mpatlasov

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 15, 2024
@bharath-b-rh
Copy link
Author

@mpatlasov Thank you for approving the changes!

Security CI job failing is during synk static code check with below errors, which was not introduced by this commit.

✗ [Medium] Use of Hardcoded Credentials
   ID: df6532bc-5b0a-43f4-a0d9-261ee145e854 
   Path: pkg/csi_driver/node.go, line 44 
   Info: Do not hardcode passwords in code. Found hardcoded saved in optionSmbPassword.
 ✗ [Medium] Use of Hardcoded Credentials
   ID: d6c8ce32-a664-4337-ac26-1a[39](https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_gcp-filestore-csi-driver/41/pull-ci-openshift-gcp-filestore-csi-driver-master-security/1812808764399554560#1:build-log.txt%3A39)40b5550c 
   Path: pkg/csi_driver/node.go, line 127 
   Info: Do not hardcode passwords in code. Found hardcoded saved in optionSmbPassword.
 ✗ [Medium] Use of Hardcoded Credentials
   ID: 2d2af722-[42](https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_gcp-filestore-csi-driver/41/pull-ci-openshift-gcp-filestore-csi-driver-master-security/1812808764399554560#1:build-log.txt%3A42)96-48ff-a7bf-50ad03555a7e 
   Path: pkg/csi_driver/node.go, line [44](https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_gcp-filestore-csi-driver/41/pull-ci-openshift-gcp-filestore-csi-driver-master-security/1812808764399554560#1:build-log.txt%3A44)4 
   Info: Do not hardcode passwords in code. Found hardcoded saved in optionSmbPassword.
 ✗ [Medium] Path Traversal
   ID: ff3d85f1-6d34-48d6-b624-73c8e727ae15 
   Path: cmd/main.go, line 117 
   Info: Unsanitized input from a CLI argument flows into os.Open, where it is used as a path. This may result in a Path Traversal vulnerability and allow an attacker to open arbitrary files.

Could you please advise on how I can proceed?

  • The last finding seems to be false positive, since we are not expecting a file in a predefined path, instead it's the complete path provided by the user and directory traversing does not happen. Synk can be configured to ignore the issue.
  • But for the hardcoded password, it's for the common windows server username/password, it would require changes to make it user configurable.
    Please let me know your thoughts.

@mpatlasov
Copy link

@bharath-b-rh , I'd suggest you to proceed ignoring security job failure as false-positive. That's why we tagged it as Required=false. IMHO, optionSmbPassword doesn't contain a password. It contains magic string "smbPassword" which is used as a key to get password value from a secret.

@bharath-b-rh
Copy link
Author

/label px-approved
/label docs-approved
no user facing changes

@openshift-ci openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Jul 17, 2024
@bharath-b-rh
Copy link
Author

@mpatlasov We can't configure ignores for code scans in the local snyk policy file, we have to do it in the UI. I have requested the access for the same, but I am not sure if that will be approved.
This was observed during upstream rebase and the failure was ignored.

@mpatlasov
Copy link

@bharath-b-rh , you only need qe-approved from QE team to have this PR merged. No need to configure ignores for code scans (unless you you are able and willing to do that).

@bharath-b-rh
Copy link
Author

/test security

Copy link

openshift-ci bot commented Jul 25, 2024

@bharath-b-rh: all tests passed!

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.

@chao007
Copy link

chao007 commented Aug 2, 2024

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Aug 2, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 2, 2024

@bharath-b-rh: This pull request references CFE-1022 which is a valid jira issue.

In response to this:

Backport of the upstream change kubernetes-sigs#640 which fixes the issue kubernetes-sigs#639.

This change enables adding labels from command line option to filestore backup resource created for an OpenShift Cluster.

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-merge-bot openshift-merge-bot bot merged commit 4eb4082 into openshift:master Aug 2, 2024
6 checks passed
@bharath-b-rh bharath-b-rh deleted the cfe-1022-639 branch August 2, 2024 07:59
@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: ose-gcp-filestore-csi-driver
This PR has been included in build ose-gcp-filestore-csi-driver-container-v4.18.0-202408020744.p0.g4eb4082.assembly.stream.el9.
All builds following this will include this PR.

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. docs-approved Signifies that Docs has signed off on this PR 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. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants