update go modules #381

ipochi · 2022-08-08T13:40:09Z

Updates the builder image version to 1.18.5 and updates the go modules
to mitigate CVEs. The list of vulnerabilities found in the binaries are
listed in comment.

Signed-off-by: Imran Pochi imranpochi@microsoft.com

k8s-ci-robot · 2022-08-08T13:40:18Z

Hi @ipochi. 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.

ipochi · 2022-08-08T13:42:57Z

/cc @tallclair @cheftako @jveski

k8s-ci-robot · 2022-08-08T13:42:59Z

@ipochi: GitHub didn't allow me to request PR reviews from the following users: jveski.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @tallclair @cheftako @jveski

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.

cheftako · 2022-08-09T23:29:25Z

go.mod

-	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.0
+	golang.org/x/net v0.0.0-20220805013720-a33c5aa5df48
+	google.golang.org/grpc v1.48.0
+	google.golang.org/protobuf v1.28.1


I realize protobug moved. Need to check that there are no additional steps needed when we switch package.

cheftako · 2022-08-09T23:30:43Z

/ok-to-test

go.mod

ipochi · 2022-08-13T07:25:00Z

/test pull-apiserver-network-proxy-test
/test pull-apiserver-network-proxy-make-lint

ipochi · 2022-08-13T07:25:18Z

/ok-to-test

ipochi · 2022-08-26T14:19:10Z

/test pull-apiserver-network-proxy-make-lint
/test pull-apiserver-network-proxy-test

ipochi · 2022-08-26T21:11:58Z

/test pull-apiserver-network-proxy-test

ipochi · 2022-08-26T21:15:47Z

/test pull-apiserver-network-proxy-make-lint

Updates the builder image version to 1.18.5 and updates the go modules to mitigate CVEs. The list of vulnerabilities found in the binaries are listed in [comment](kubernetes-sigs#372 (comment)). Signed-off-by: Imran Pochi <imranpochi@microsoft.com>

Signed-off-by: Imran Pochi <imranpochi@microsoft.com>

G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec) Configuring with a sensible default of 60s. Nginx uses the same default value (http://nginx.org/en/docs/http/ngx_http_core_module.html#client_header_timeout) Signed-off-by: Imran Pochi <imranpochi@microsoft.com>

ipochi · 2022-09-02T05:07:53Z

/test pull-apiserver-network-proxy-make-lint

ipochi · 2022-09-02T16:32:57Z

/test pull-apiserver-network-proxy-test

ipochi · 2022-09-02T16:34:17Z

/test pull-apiserver-network-proxy-make-lint

ipochi · 2022-09-02T16:35:46Z

/test pull-apiserver-network-proxy-test

ipochi · 2022-09-02T16:39:38Z

@tallclair @cheftako

Finally the lint job succeeds. moved all the components to go v1.18.5. Please review :)

cheftako · 2022-09-02T21:21:34Z

/lgtm
/approve

k8s-ci-robot · 2022-09-02T21:21:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheftako, ipochi

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

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

liggitt · 2022-12-06T04:26:07Z

go.mod

+	k8s.io/apimachinery v0.24.3
+	k8s.io/client-go v0.24.3
+	k8s.io/component-base v0.24.3
+	k8s.io/klog/v2 v2.70.1
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.0
 )

 require (


this added a lot of new dependencies, many of which we explicitly don't want to depend on transitively in Kubernetes:

github.com/go-openapi/...

new incompatible version of github.com/imdario/mergo (xref remove use of github.com/imdario/mergo (currently use is incompatible with latest mergo version, latest mergo version introduces bugs) kubernetes/kubernetes#107499)

Also, this only updated the root go.mod file, not the client konnectivity-client/go.mod file, so now this repo has two disparate dependency trees. If the client go.mod file was updated the same way this was, we would be unable to pull new versions into kubernetes.

How were the versions of these updated modules selected?

The two that you mentioned are both transitive dependencies through client-go. I don't know how that specific client-go version was selected, but since this PR sat open for a long time I'm guessing it was the latest version when this PR was opened.

We haven't been keeping the konnectivity-client go.mod in sync with this since we're tracking the module versions from the oldest version of k8s we support in the client. We should maybe consider adding a presubmit to ensure that konnectivity-client doesn't take any dependencies on kubernetes staging repos, but hopefully our PR review process would catch that.

https://github.com/kubernetes/client-go/blob/v0.24.3/go.mod#L26

oops, I misread the go-openapi additions, these are the ones we want to avoid:

"github.com/go-openapi/analysis": "use k8s.io/kube-openapi/pkg/validation/spec", "github.com/go-openapi/spec": "use k8s.io/kube-openapi/pkg/validation/spec instead", "github.com/go-openapi/strfmt": "use k8s.io/kube-openapi/pkg/validation/strfmt instead", "github.com/go-openapi/validate": "use k8s.io/kube-openapi/pkg/validation/validate instead",

the github.com/imdario/mergo one is a known dependency but the version selected here is known to be problematic (see the linked issue above)

tallclair · 2022-12-06T17:22:25Z

cmd/server/app/server.go

@@ -350,7 +354,7 @@ func (p *Proxy) runAdminServer(o *options.ProxyRunOptions, server *server.ProxyS
 		}
 	}
 	adminServer := &http.Server{
-		Addr:           net.JoinHostPort(o.AdminBindAddress, strconv.Itoa(o.AdminPort)),
+		Addr:           fmt.Sprintf("127.0.0.1:%d", o.AdminPort),


Why was this changed? Looks like maybe a bad rebase?

+1. However this was merged Sept 2, over 3 months ago.

See discussion at kubernetes-sigs#381 What I did: - match requirements with k/k release-1.24 branch - for k8s.io/api and friends, take latest 1.24 tag (v0.24.8) - go mod tidy

See discussion at kubernetes-sigs#381 What I did: - match requirements with k/k release-1.24 branch - for k8s.io/api and friends, take latest 1.24 tag (v0.24.8) - delete indirect require section (to generate from scratch) - go mod tidy

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 8, 2022

k8s-ci-robot requested review from dberkov and Jefftree August 8, 2022 13:40

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 8, 2022

k8s-ci-robot requested a review from tallclair August 8, 2022 13:42

k8s-ci-robot requested a review from cheftako August 8, 2022 13:42

cheftako reviewed Aug 9, 2022

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 Aug 9, 2022

tallclair reviewed Aug 10, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

ipochi force-pushed the imran/go-mod-update branch from 23505fb to db7f800 Compare August 13, 2022 07:24

ipochi mentioned this pull request Aug 19, 2022

apiserver-network-proxy-presubmits: use Go 1.18 kubernetes/test-infra#27124

Merged

ipochi force-pushed the imran/go-mod-update branch 2 times, most recently from bf41994 to 1849963 Compare August 26, 2022 13:42

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 26, 2022

ipochi force-pushed the imran/go-mod-update branch from 1849963 to b1a6d99 Compare August 26, 2022 13:58

ipochi force-pushed the imran/go-mod-update branch from b1a6d99 to ef352a2 Compare August 26, 2022 21:05

ipochi force-pushed the imran/go-mod-update branch 2 times, most recently from 29e7193 to f00900c Compare August 26, 2022 21:29

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

ipochi added 3 commits September 1, 2022 15:20

update go modules

80d2696

Updates the builder image version to 1.18.5 and updates the go modules to mitigate CVEs. The list of vulnerabilities found in the binaries are listed in [comment](kubernetes-sigs#372 (comment)). Signed-off-by: Imran Pochi <imranpochi@microsoft.com>

update gosec and golangci-lint versions

d1ba2fb

Signed-off-by: Imran Pochi <imranpochi@microsoft.com>

ipochi force-pushed the imran/go-mod-update branch from 900a202 to 93d1da1 Compare September 1, 2022 09:57

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 1, 2022

ipochi mentioned this pull request Sep 1, 2022

Using 1.24 variant of kubekins image kubernetes/test-infra#27336

Merged

ipochi force-pushed the imran/go-mod-update branch from 93d1da1 to 04595aa Compare September 1, 2022 17:13

k8s-ci-robot assigned cheftako Sep 2, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 2, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 2, 2022

k8s-ci-robot merged commit 85e1558 into kubernetes-sigs:master Sep 2, 2022

ipochi deleted the imran/go-mod-update branch September 3, 2022 00:14

ipochi mentioned this pull request Sep 5, 2022

New tag required v0.0.33 on account of merged PRs and CVE's #372

Closed

ipochi mentioned this pull request Oct 18, 2022

Have a rule in the release process to check for CVE's before releasing a new tag. #412

Closed

liggitt reviewed Dec 6, 2022

View reviewed changes

tallclair reviewed Dec 6, 2022

View reviewed changes

jkh52 mentioned this pull request Dec 7, 2022

Update go.mod to be more conservative. #432

Merged

ipochi mentioned this pull request Aug 4, 2023

REQUEST: New membership for ipochi kubernetes/org#4368

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update go modules #381

update go modules #381

ipochi commented Aug 8, 2022

k8s-ci-robot commented Aug 8, 2022

ipochi commented Aug 8, 2022

k8s-ci-robot commented Aug 8, 2022

cheftako Aug 9, 2022

cheftako commented Aug 9, 2022

ipochi commented Aug 13, 2022

ipochi commented Aug 13, 2022

ipochi commented Aug 26, 2022

ipochi commented Aug 26, 2022

ipochi commented Aug 26, 2022

ipochi commented Sep 2, 2022

ipochi commented Sep 2, 2022 •

edited

Loading

ipochi commented Sep 2, 2022

ipochi commented Sep 2, 2022

ipochi commented Sep 2, 2022

cheftako commented Sep 2, 2022

k8s-ci-robot commented Sep 2, 2022

liggitt Dec 6, 2022

tallclair Dec 6, 2022

liggitt Dec 6, 2022

tallclair Dec 6, 2022

cheftako Dec 6, 2022

update go modules #381

update go modules #381

Conversation

ipochi commented Aug 8, 2022

k8s-ci-robot commented Aug 8, 2022

ipochi commented Aug 8, 2022

k8s-ci-robot commented Aug 8, 2022

cheftako Aug 9, 2022

Choose a reason for hiding this comment

cheftako commented Aug 9, 2022

ipochi commented Aug 13, 2022

ipochi commented Aug 13, 2022

ipochi commented Aug 26, 2022

ipochi commented Aug 26, 2022

ipochi commented Aug 26, 2022

ipochi commented Sep 2, 2022

ipochi commented Sep 2, 2022 • edited Loading

ipochi commented Sep 2, 2022

ipochi commented Sep 2, 2022

ipochi commented Sep 2, 2022

cheftako commented Sep 2, 2022

k8s-ci-robot commented Sep 2, 2022

liggitt Dec 6, 2022

Choose a reason for hiding this comment

tallclair Dec 6, 2022

Choose a reason for hiding this comment

liggitt Dec 6, 2022

Choose a reason for hiding this comment

tallclair Dec 6, 2022

Choose a reason for hiding this comment

cheftako Dec 6, 2022

Choose a reason for hiding this comment

ipochi commented Sep 2, 2022 •

edited

Loading