Bump k8s dependecies to v1.25 #761

yaron-idan · 2023-06-01T07:07:21Z

This PR bumps k8s dependencies to v1.25 to address CVE-2020-8558 and close issue #742

k8s-ci-robot · 2023-06-01T07:07:29Z

Welcome @yaron-idan!

It looks like this is your first PR to kubernetes/node-problem-detector 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/node-problem-detector has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2023-06-01T07:07:30Z

Hi @yaron-idan. Thanks for your PR.

I'm waiting for a kubernetes 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 · 2023-06-01T07:07:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yaron-idan
Once this PR has been reviewed and has the lgtm label, please assign vteratipally for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

MartinForReal

Please remove files under .idea

yaron-idan · 2023-06-01T08:27:25Z

Hey @MartinForReal, I've removed the .idea folder from the PR.

MartinForReal · 2023-06-01T08:25:47Z

pkg/exporters/k8sexporter/problemclient/problem_client.go

@@ -107,7 +108,7 @@ func (c *nodeProblemClient) SetConditions(newConditions []v1.NodeCondition) erro
 	if err != nil {
 		return err
 	}
-	return c.client.RESTClient().Patch(types.StrategicMergePatchType).Resource("nodes").Name(c.nodeName).SubResource("status").Body(patch).Do().Error()
+	return c.client.RESTClient().Patch(types.StrategicMergePatchType).Resource("nodes").Name(c.nodeName).SubResource("status").Body(patch).Do(context.TODO()).Error()


I think we'd better to introduce context package into NPD, context.TODO() should not be used with product code.

I agree, but let's make that a separate change. I think it's fine to use TODO for now.

I've worked on a commit adding context last night, here it is for reference.

MartinForReal · 2023-06-01T08:29:29Z

pkg/systemstatsmonitor/memory_collector_linux.go

@@ -38,30 +38,30 @@ func (mc *memoryCollector) collect() {
 	}

 	if mc.mBytesUsed != nil {
-		memUsed := meminfo.MemTotal - meminfo.MemFree - meminfo.Buffers - meminfo.Cached - meminfo.Slab
-		mc.mBytesUsed.Record(map[string]string{stateLabel: "free"}, int64(meminfo.MemFree)*1024)
+		memUsed := *meminfo.MemTotal - *meminfo.MemFree - *meminfo.Buffers - *meminfo.Cached - *meminfo.Slab


Do you think if we need to make sure pointer is not nil.

It's a good idea, I've added a check to the condition.

I mean maybe we should check if meminfo.MemFree, meminfo.Buffers or other item is nil

Gotcha, I updated the condition. Does this fit what you meant?

MartinForReal · 2023-06-01T08:34:57Z

go.mod

+	golang.org/x/sys v0.8.0
+	google.golang.org/api v0.118.0
+	k8s.io/api v0.27.2
+	k8s.io/apimachinery v0.27.2


Please check the status of go mod graph. Although k8s.io related dependencies are set to 1.25.10 in replace rules, the actual required version is 1.27. we need to find the library which requires k8s.io 1.27 and downgrade them if any.

I haven't found any mention of v1.27 when running go mod graph, are you sure it's been required somewhere in the project?

MartinForReal · 2023-06-01T08:53:53Z

pkg/util/tail/tail.go

@@ -24,8 +25,8 @@ import (
 	"sync"
 	"time"

-	"k8s.io/klog"
-	inotify "k8s.io/utils/inotify"
+	"github.com/fsnotify/fsnotify"


I'm wondering why this library is replaced with "github.com/fsnotify/fsnotify"

MartinForReal · 2023-06-02T12:28:24Z

pkg/systemstatsmonitor/memory_collector_linux.go

@@ -38,30 +38,30 @@ func (mc *memoryCollector) collect() {
 	}

 	if mc.mBytesUsed != nil {
-		memUsed := meminfo.MemTotal - meminfo.MemFree - meminfo.Buffers - meminfo.Cached - meminfo.Slab
-		mc.mBytesUsed.Record(map[string]string{stateLabel: "free"}, int64(meminfo.MemFree)*1024)
+		memUsed := *meminfo.MemTotal - *meminfo.MemFree - *meminfo.Buffers - *meminfo.Cached - *meminfo.Slab


I mean maybe we should check if meminfo.MemFree, meminfo.Buffers or other item is nil

vteratipally · 2023-06-02T13:52:03Z

/ok-to-test

yaron-idan · 2023-06-04T06:31:40Z

/retest

k8s-ci-robot · 2023-06-07T19:22:16Z

@yaron-idan: 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
pull-npd-test	`e31f5a3`	link	true	`/test pull-npd-test`
pull-npd-e2e-test	`e31f5a3`	link	true	`/test pull-npd-e2e-test`
pull-npd-build	`e31f5a3`	link	true	`/test pull-npd-build`
pull-npd-e2e-node	`e31f5a3`	link	true	`/test pull-npd-e2e-node`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

k8s-ci-robot · 2023-06-26T06:36:53Z

PR needs rebase.

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.

MartinForReal · 2023-09-07T15:26:50Z

#793 is merged.

hakman · 2023-09-19T07:21:06Z

NPD dependencies were bumped to 1.28.2. Most CVEs should be addressed in the next release.

MartinForReal · 2024-01-04T02:32:48Z

/close

k8s-ci-robot · 2024-01-04T02:32:53Z

@MartinForReal: Closed this PR.

In response to this:

/close

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.

yaron-idan added 5 commits May 27, 2023 11:09

Upgrade to v1.21

a7250b4

update ssh module

e5531f7

fix CVE-2020-8558

d52ac45

add missing files

f95dd7c

Merge remote-tracking branch 'upstream/master' into CVE-2020-8558

c03202f

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 1, 2023

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 1, 2023

k8s-ci-robot requested review from mmiranda96 and vteratipally June 1, 2023 07:07

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 1, 2023

This was referenced Jun 1, 2023

CVE-2020-8558 for k8s.io/kubernetes v1.14.6 #742

Closed

bump k8s.io dependencies to 1.17.2 #760

Merged

MartinForReal reviewed Jun 1, 2023

View reviewed changes

This comment was marked as spam.

Sign in to view

This comment was marked as resolved.

Sign in to view

remove .idea

f074a82

MartinForReal suggested changes Jun 1, 2023

View reviewed changes

check if meminfo pointer is not nil

184f371

MartinForReal reviewed Jun 2, 2023

View reviewed changes

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 Jun 2, 2023

check pointer items are not nil

48518d0

yaron-idan added 2 commits June 6, 2023 09:18

add nil pointer conditions

75dd516

fix condition

f6c428f

add context

e31f5a3

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 26, 2023

MartinForReal mentioned this pull request Jun 26, 2023

bump k8s.io to 1.24.15 and replace boskos client #771

Closed

MartinForReal mentioned this pull request Dec 22, 2023

Add MartinForReal as a reviewer #775

Closed

k8s-ci-robot closed this Jan 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump k8s dependecies to v1.25 #761

Bump k8s dependecies to v1.25 #761

yaron-idan commented Jun 1, 2023

k8s-ci-robot commented Jun 1, 2023

k8s-ci-robot commented Jun 1, 2023

k8s-ci-robot commented Jun 1, 2023

MartinForReal left a comment •

edited

Loading

This comment was marked as spam.

This comment was marked as resolved.

yaron-idan commented Jun 1, 2023

MartinForReal Jun 1, 2023

mmiranda96 Jun 7, 2023

yaron-idan Jun 8, 2023

MartinForReal Jun 1, 2023

yaron-idan Jun 2, 2023

MartinForReal Jun 2, 2023

yaron-idan Jun 2, 2023

MartinForReal Jun 1, 2023 •

edited

Loading

yaron-idan Jun 8, 2023

MartinForReal Jun 1, 2023

MartinForReal Jun 2, 2023

vteratipally commented Jun 2, 2023

yaron-idan commented Jun 4, 2023

k8s-ci-robot commented Jun 7, 2023

k8s-ci-robot commented Jun 26, 2023

MartinForReal commented Sep 7, 2023

hakman commented Sep 19, 2023

MartinForReal commented Jan 4, 2024

k8s-ci-robot commented Jan 4, 2024

Bump k8s dependecies to v1.25 #761

Bump k8s dependecies to v1.25 #761

Conversation

yaron-idan commented Jun 1, 2023

k8s-ci-robot commented Jun 1, 2023

k8s-ci-robot commented Jun 1, 2023

k8s-ci-robot commented Jun 1, 2023

MartinForReal left a comment • edited Loading

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as resolved.

yaron-idan commented Jun 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinForReal Jun 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vteratipally commented Jun 2, 2023

yaron-idan commented Jun 4, 2023

k8s-ci-robot commented Jun 7, 2023

k8s-ci-robot commented Jun 26, 2023

MartinForReal commented Sep 7, 2023

hakman commented Sep 19, 2023

MartinForReal commented Jan 4, 2024

k8s-ci-robot commented Jan 4, 2024

MartinForReal left a comment •

edited

Loading

MartinForReal Jun 1, 2023 •

edited

Loading