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.

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

Jump to bottom

Fix CRITICAL/ HIGH security vulns in v0.6.2 reported by Trivy #1194

Merged
merged 6 commits into from
Mar 2, 2023
Merged

Fix CRITICAL/ HIGH security vulns in v0.6.2 reported by Trivy #1194

merged 6 commits into from
Mar 2, 2023

Conversation

cruizen
Copy link

@cruizen cruizen commented Feb 17, 2023

What this PR does / why we need it: Fixes security issues reported by Trivy in v0.6.2

  • There is another issue to prepare for release v0.7 - however that involves updating the k8s APIs from k8s.io/* v0.23.2 to k8s.io/api v0.25.x - this PR on the other hand only patches these APIs to k8s.io/api v0.23.16
  • golang built-in packages are updated without upgrading golang from 1.17 to 1.18 or higher

Which issue(s) this PR fixes
Fixes # #1190

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 17, 2023
@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 17, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @cruizen. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 17, 2023
@cruizen cruizen changed the title Bump dependencies to fix security vulns reported by Trivy Fix CRITICAL/ HIGH security vulns in v0.6.2 reported by Trivy Feb 17, 2023
@cruizen cruizen mentioned this pull request Feb 17, 2023
Closed
@cruizen
Copy link
Author

cruizen commented Feb 17, 2023

Hi @serathius , @yangjunmyfm192085 does this look like a reasonable patch for v0.6 to solve security issues - and is it okay-to-test?

@serathius
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 17, 2023
@logicalhan
Copy link

/triage accepted
/assign @serathius

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 23, 2023
@cruizen
Copy link
Author

cruizen commented Feb 23, 2023

/retest

yangjunmyfm192085
yangjunmyfm192085 reviewed Feb 26, 2023
View reviewed changes
Copy link
Contributor

@yangjunmyfm192085 yangjunmyfm192085 left a comment

Choose a reason for hiding this comment

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

/retest

@yangjunmyfm192085
Copy link
Contributor

/test pull-metrics-server-verify

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 26, 2023
@yangjunmyfm192085
Copy link
Contributor

Hi, @cruizen Coud you rebase this pr?

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 28, 2023
yangjunmyfm192085 and others added 3 commits March 1, 2023 20:53
@yangjunmyfm192085 @cruizen
fix verify fail
52c6fce
@cruizen
Update modules
28f01bd
@cruizen
Fix errors in find -type f -name "*.go" ! -path "*/vendor/*" | xargs …
670a5f4 
…/Users/trilokk/go/bin/addlicense -check || (echo 'Run "make update"' && exit 1)

golangci-lint run --timeout 10m --modules-download-mode=readonly || (echo 'Run "make update"' && exit 1)
/Users/trilokk/go/bin/mdtoc --inplace --dryrun FAQ.md KNOWN_ISSUES.md
go mod verify
all modules verified
go mod tidy
# pkg/api/generated/openapi/zz_generated.openapi.go
go install -mod=readonly k8s.io/kube-openapi/cmd/openapi-gen
/Users/trilokk/go/bin/openapi-gen --logtostderr -i k8s.io/metrics/pkg/apis/metrics/v1beta1,k8s.io/apimachinery/pkg/apis/meta/v1,k8s.io/apimachinery/pkg/api/resource,k8s.io/apimachinery/pkg/version -p pkg/api/generated/openapi/ -O zz_generated.openapi -o /Users/trilokk/code/cruizen/metrics-server -h /Users/trilokk/code/cruizen/metrics-server/scripts/boilerplate.go.txt -r /dev/null
/Users/trilokk/go/bin/logcheck ./... || (echo 'Fix structured logging' && exit 1)
@cruizen cruizen force-pushed the release-0.6-bump-go-modules-sec-fixes branch from f96c622 to 670a5f4 Compare March 1, 2023 15:27
@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 1, 2023
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 1, 2023
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 1, 2023
@cruizen
Copy link
Author

cruizen commented Mar 1, 2023

/retest

@cruizen
Copy link
Author

cruizen commented Mar 1, 2023

Hi, @cruizen Coud you rebase this pr?

@yangjunmyfm192085 Thanks for fixing the error in verify. Rebased and now pull-metrics-server-verify test passes. The test pull-metrics-server-test-e2e-ha failed earlier but passed a few minutes ago on retest.

@cruizen
Copy link
Author

cruizen commented Mar 2, 2023

/retest

@cruizen cruizen requested review from yangjunmyfm192085 and removed request for serathius March 2, 2023 06:08
@cruizen
Copy link
Author

cruizen commented Mar 2, 2023

Looks like I accidentally removed @serathius from the review when I re-requested review from @yangjunmyfm192085
All the checks are passing now.

serathius
serathius reviewed Mar 2, 2023
View reviewed changes
go.mod Outdated Show resolved Hide resolved
@serathius
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 2, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cruizen, serathius

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 2, 2023
@k8s-ci-robot k8s-ci-robot merged commit e22363f into kubernetes-sigs:release-0.6 Mar 2, 2023
@cruizen cruizen mentioned this pull request Mar 2, 2023
Closed
@knechtionscoding knechtionscoding mentioned this pull request Mar 6, 2023
Closed
@dgrisonnet dgrisonnet mentioned this pull request Mar 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serathius serathius serathius left review comments

@yangjunmyfm192085 yangjunmyfm192085 Awaiting requested review from yangjunmyfm192085

Assignees

@serathius serathius

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cruizen @k8s-ci-robot @serathius @logicalhan @yangjunmyfm192085