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

Bump otelhttp to resolve CVE; Bump Go version; Bump golangci-lint + adjusted lint configs; critical copylock fix #120

Merged
merged 11 commits into from Nov 22, 2023

Conversation

pintohutch
Copy link
Collaborator

This resolves https://nvd.nist.gov/vuln/detail/CVE-2023-45142 by bumping otelhttp to v0.45.0. It's a follow-up to
#113.

I had to bump the otlptrace packages to v1.19.0 to make go mod tidy happy as well.

This resolves https://nvd.nist.gov/vuln/detail/CVE-2023-45142 by bumping
otelhttp to v0.45.0. It's a follow-up to
#113.

I had to bump the otlptrace packages to v1.19.0 to make `go mod tidy`
happy as well.
@pintohutch
Copy link
Collaborator Author

Trying like hell to get the CI to pass, but hitting issues left and right...gonna let this bake overnight

@bwplotka
Copy link
Collaborator

Looks like golangci-lint consistently timeouts. We also run it in 3 places in our CI.. 🙈

image

@bwplotka
Copy link
Collaborator

  1. -dep=false made no difference, trying to help by reverting dep=false and tryting different things.
  2. Local run for 1.49.0 golancilint is timing out too. I guess it must be Go updates we made, perhaps old version had some issues. Indeed (1.51 adds Go1.20 support).
  3. Trying newer version 1.55.2 immidiately shows multiple new issues.

We could downgrade golangci-lint, but soon we might have another version of Go or some important fix, so I would use latest golangci-lint on our supported fork versions. Solid question is--can we disable golangci-lint-ing. We mostly import and inject export package here, but also fixed multiple security issues... it would be nice to lint on important things.

Fixing less important linting errors is adding too much changes to our fork diff vs upstream for no good reason e.g. readability pointers:

storage/interface.go:121:35: unused-parameter: parameter 'name' seems to be unused, consider removing or renaming it as _ (revive)

I propose adjusting certain linters:

  • depguard - Disalbing. This is mostly blacklisting certain packages we migreated from in Prometheus. No need for that in our fork.
  • revive - Adjusting to only show errors (critical issues)
  • staticheck - filter out deprecation notices.

I fixed one govet signal which makes sense (also fixed in latest upstream) https://github.com/prometheus/prometheus/blob/main/web/web.go#L750

TL;DR we need the latest Prometheus version as our main GMP collector version ASAP ;p

@bwplotka bwplotka changed the title Bump otelhttp to resolve CVE Bump otelhttp to resolve CVE; Bump Go version; Bump golangci-lint + adjusted lint configs; critical copylock fix Nov 22, 2023
Signed-off-by: bwplotka <bwplotka@gmail.com>
@bwplotka
Copy link
Collaborator

Another fix - skipping linting of some documentation code, somehow downloading does not work.

@pintohutch
Copy link
Collaborator Author

Totally agree about the need to bump it (@TheSpiritXIII has a pending PR in #562) - though I wonder if we should be aiming for 2.45 for LTS support.

@pintohutch
Copy link
Collaborator Author

Thanks @bwplotka for the contributions here! Merging with your changes.

@pintohutch pintohutch merged commit 4c0f55e into release-2.41.0-gmp Nov 22, 2023
43 checks passed
@pintohutch pintohutch deleted the pintohutch/release-2.41.0-gmp branch November 22, 2023 18:12
@pintohutch
Copy link
Collaborator Author

Crap - I mean to squash these commits into one.

Gonna fast-and-loose just do that in the release branch 😬

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.

None yet

2 participants