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

fix(deps): Upgrade to go.1.20.13 and fix high level CVEs #5234

Merged
merged 45 commits into from
Jan 29, 2024

Conversation

sakoush
Copy link
Member

@sakoush sakoush commented Jan 26, 2024

What this PR does / why we need it:
This PR upgrades go to 1.20.13 and various go packages to fix high level CVEs.

The list of changes are can be checked in the commit log.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

dependabot bot and others added 30 commits January 25, 2024 11:19
Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.15.0 to 0.17.0.
- [Commits](golang/crypto@v0.15.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…com/gorilla/mux/otelmux

Bumps [go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux](https://github.com/open-telemetry/opentelemetry-go-contrib) from 0.31.0 to 0.44.0.
- [Release notes](https://github.com/open-telemetry/opentelemetry-go-contrib/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go-contrib@zpages/v0.31.0...zpages/v0.44.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.7.0 to 0.17.0.
- [Commits](golang/net@v0.7.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.15.0 to 0.17.0.
- [Commits](golang/crypto@v0.15.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
THIS SHOULD NOT BE MERGED UPSTREAM
DO NOT MERGE UPSTREAM
…ry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux-0.44.0
…ang.org/x/crypto-0.17.0

build(deps): bump golang.org/x/crypto from 0.15.0 to 0.17.0 in /operator
…ation/golang.org/x/net-0.17.0

build(deps): bump golang.org/x/net from 0.7.0 to 0.17.0 in /tests/integration
….opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux-0.44.0

build(deps): bump go.opentelemetry.io/contrib/instrumentation/github.com/gorilla/mux/otelmux from 0.31.0 to 0.44.0 in /scheduler
Bumps [golang.org/x/sys](https://github.com/golang/sys) from 0.0.0-20220319134239-a9b59b0215f8 to 0.1.0.
- [Commits](https://github.com/golang/sys/commits/v0.1.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sys
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps gopkg.in/yaml.v3 from 3.0.0-20210107192922-496545a6307b to 3.0.0.

---
updated-dependencies:
- dependency-name: gopkg.in/yaml.v3
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [github.com/docker/docker](https://github.com/docker/docker) from 24.0.2+incompatible to 24.0.7+incompatible.
- [Release notes](https://github.com/docker/docker/releases)
- [Commits](moby/moby@v24.0.2...v24.0.7)

---
updated-dependencies:
- dependency-name: github.com/docker/docker
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…ation/github.com/docker/docker-24.0.7incompatible

build(deps): bump github.com/docker/docker from 24.0.2+incompatible to 24.0.7+incompatible in /tests/integration
…tls/gopkg.in/yaml.v3-3.0.0

build(deps): bump gopkg.in/yaml.v3 from 3.0.0-20210107192922-496545a6307b to 3.0.0 in /components/tls
…ls/golang.org/x/sys-0.1.0

build(deps): bump golang.org/x/sys from 0.0.0-20220319134239-a9b59b0215f8 to 0.1.0 in /components/tls
…mponents/tls/golang.org/x/sys-0.1.0

Revert "build(deps): bump golang.org/x/sys from 0.0.0-20220319134239-a9b59b0215f8 to 0.1.0 in /components/tls"
Bumps [golang.org/x/text](https://github.com/golang/text) from 0.3.7 to 0.3.8.
- [Release notes](https://github.com/golang/text/releases)
- [Commits](golang/text@v0.3.7...v0.3.8)

---
updated-dependencies:
- dependency-name: golang.org/x/text
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…lang.org/x/crypto-0.17.0

build(deps): bump golang.org/x/crypto from 0.15.0 to 0.17.0 in /scheduler
…tls/golang.org/x/text-0.3.8

build(deps): bump golang.org/x/text from 0.3.7 to 0.3.8 in /components/tls
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.0.0-20220225172249-27dd8689420f to 0.17.0.
- [Commits](https://github.com/golang/net/commits/v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…ls/golang.org/x/net-0.17.0

build(deps): bump golang.org/x/net from 0.0.0-20220225172249-27dd8689420f to 0.17.0 in /components/tls
Bumps [golang.org/x/text](https://github.com/golang/text) from 0.3.7 to 0.3.8.
- [Release notes](https://github.com/golang/text/releases)
- [Commits](golang/text@v0.3.7...v0.3.8)

---
updated-dependencies:
- dependency-name: golang.org/x/text
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…ang.org/x/text-0.3.8

build(deps): bump golang.org/x/text from 0.3.7 to 0.3.8 in /apis/go
dependabot bot and others added 8 commits January 26, 2024 09:28
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.0.0-20220225172249-27dd8689420f to 0.17.0.
- [Commits](https://github.com/golang/net/commits/v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…ang.org/x/net-0.17.0

build(deps): bump golang.org/x/net from 0.0.0-20220225172249-27dd8689420f to 0.17.0 in /apis/go
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.46.0 to 1.56.3.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.46.0...v1.56.3)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.49.0 to 1.56.3.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.49.0...v1.56.3)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@sakoush
Copy link
Member Author

sakoush commented Jan 29, 2024

ran successfully samples/pipeline-tests.sh using a docker-compose setup.

@sakoush
Copy link
Member Author

sakoush commented Jan 29, 2024

grafana image still have the following high CVEs after upgrade to grafana 10.3.1 which is latest at the time of writing:
GHSA-9763-4f94-gfch
CVE-2023-5363

Copy link
Member

@lc525 lc525 left a comment

Choose a reason for hiding this comment

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

I added some minor points that might be worth thinking about, if not within this PR at least as a strategy for updating moving forward.

apis/go/go.mod Show resolved Hide resolved
apis/go/go.mod Show resolved Hide resolved
@@ -1,4 +1,4 @@
FROM golang:1.20.11-bullseye as builder
FROM golang:1.20.13-bullseye as builder
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering out loud here, whether it would make sense to remove very precise tags like these from Dockerfiles, similarly to how you've changed things in the github workflows from 1.20.11 to just 1.20. The equivalent image here would be golang:1.20-bullseye.

This would give us some upgrades for free when rebuilding images, but it does add some risk that things break. In general I think go is better than others in maintaining compatibility, but it's a discussion worth having (whether to take on that risk).

However, irrespective of the choice, I would apply the same strategy both here and in the github actions to maintain exact compatibility between the tests that github runs and the docker images being built.

Copy link
Member Author

Choose a reason for hiding this comment

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

For now I pinned it to the same versions. Lets discuss if we can relax it as suggested later. Hopefully this will all be done by dependabot.
I did relax it in github workflow initially but I was not sure completely if they are available in lock steps.

Copy link
Member Author

Choose a reason for hiding this comment

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

as discussed we will relax go patch version in docker images.

@@ -1,4 +1,4 @@
FROM golang:1.20.11-alpine AS builder
FROM golang:1.20.13-alpine AS builder
Copy link
Member

Choose a reason for hiding this comment

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

Apologies for the long list of comments on rather trivial changes, but I'm lacking context here. I see golang:*-bullseye, golang:*-alpine and golang:* all used in those dockerfiles. We should probably aim to move to a single base unless there are good reasons to keep things different (just to elliminate possible divergences in the compiled binaries)

Generally this should not present a problem with go as we get staticly linked binaries, but if we're using CGO anywhere there will be a dependency on glibc which might differ between debian (bullseye) and redhat/alpine

Copy link
Member Author

Choose a reason for hiding this comment

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

Some images require glibc, which alpine does not provide (based on some comments in the code). I think also the aim was to keep the image as small as possible. We probably can move them all to the same base image if the size difference is not significant. I will leave it to a future PR.

@@ -14,7 +14,7 @@ RUN gradle build --no-daemon --info
################################################################################

# Some dependencies require glibc, which Alpine does not provide
FROM registry.access.redhat.com/ubi9/openjdk-17-runtime:1.16-3
FROM registry.access.redhat.com/ubi9/openjdk-17-runtime:1.17-1.1705573249
Copy link
Member

Choose a reason for hiding this comment

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

This change is related to the dataflow libraries, might be worth to remove it from this PR as I'll have it my PR.

In my changes, I've updated this to registry.access.redhat.com/ubi9/openjdk-17-runtime:1.17 following the idea I've mentioned above. In fact, I can see dependabot pinning us to even more specific versions here than what we had before.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok moved to openjdk-17-runtime:1.17 in 348301f

@sakoush sakoush merged commit aaf0883 into SeldonIO:v2 Jan 29, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants