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

[chore] replace adapter logger with logr interface #1

Merged
merged 3 commits into from
Nov 8, 2023
Merged

[chore] replace adapter logger with logr interface #1

merged 3 commits into from
Nov 8, 2023

Conversation

jan-law
Copy link
Collaborator

@jan-law jan-law commented Oct 24, 2023

Related #1672
Copy of dbason/opentelemetry-collector-contrib#1

Question - the docker build CI fails on ea68927 with

 ../../pkg/stanza/adapter/factory.go:14:2: github.com/go-logr/zapr@v1.2.4: missing go.sum entry for go.mod file; to add it:
	go mod download github.com/go-logr/zapr

Running the suggested fix, go mod tidy, and go mod tidy -e don't produce any changes. All the tests inpkg/stanza pass with go test. The opni supportagent tests also pass. What am I missing?

@jan-law jan-law requested a review from dbason October 24, 2023 21:59
@jan-law jan-law changed the title replace adapter logger with logr interface [chore] replace adapter logger with logr interface Oct 30, 2023
Janelle Law and others added 2 commits November 1, 2023 18:14
Signed-off-by: Janelle Law <janelle.law@suse.com>
Signed-off-by: Janelle Law <janelle.law@suse.com>
Signed-off-by: Janelle Law <janelle.law@suse.com>
@jan-law
Copy link
Collaborator Author

jan-law commented Nov 7, 2023

I was able to fix the lint issues, but I'm having trouble upgrading the package vulnerabilities. Listed the error and what I've tried below. Any ideas would be much appreciated!

Steps I did to make commit 286b3f7, and the govulncheck errors that occurred after pushing the commit:

go get github.com/cyphar/filepath-securejoin@v0.2.4
go get [google.golang.org/grpc@v1.58.3](http://google.golang.org/grpc@v1.58.3)
make gotidy

Vulnerability #1: GO-2023-2048
    Paths outside of the rootfs could be produced on Windows
  More info: https://pkg.go.dev/vuln/GO-2023-2048
  Module: github.com/cyphar/filepath-securejoin
    Found in: github.com/cyphar/filepath-securejoin@v0.2.3
    Fixed in: github.com/cyphar/filepath-securejoin@v0.2.4
    Platforms: windows
    Example traces found:
Error:       #1: internal/cadvisor/cadvisor_linux.go:392:23: cadvisor.Cadvisor.initManager calls manager.manager.Start, which eventually calls filepath.SecureJoin

Vulnerability #1: GO-2023-2153
    denial of service from HTTP/2 Rapid Reset in google.golang.org/grpc
  More info: https://pkg.go.dev/vuln/GO-2023-2153
  Module: google.golang.org/grpc
    Found in: google.golang.org/grpc@v1.[57](https://github.com/rancher-sandbox/otel-collector-contrib/actions/runs/6786416463/job/18446894025?pr=1#step:6:58).0
    Fixed in: google.golang.org/grpc@v1.[58](https://github.com/rancher-sandbox/otel-collector-contrib/actions/runs/6786416463/job/18446894025?pr=1#step:6:59).3
    Example traces found:
Error:       #1: loki.go:152:35: lokireceiver.startGRPCServer calls grpc.Server.Serve, which eventually calls transport.NewServerTransport
Error:       #2: loki.go:109:43: lokireceiver.lokiReceiver.startProtocolsServers calls configgrpc.GRPCServerSettings.ToServer, which calls grpc.NewServer
Error:       #3: loki.go:152:35: lokireceiver.startGRPCServer calls grpc.Server.Serve

This resulted in no changes ('nothing to commit, working tree clean'):

cd receiver
go get google.golang.org/grpc@v1.58.3
make gotidy

This resulted in compile errors due to cadvisor api changes:

go mod why -m github.com/cyphar/filepath-securejoin
-> mentions cadvisor, whose current version is 0.47.3
go get github.com/google/cadvisor@v0.48.0
make gotidy

@alexandreLamarre
Copy link

@jan-law are the vulnerabilities in the dependencies introduced specifically by deps introduced in the updates?

Otherwise, this is fine to merge without dependency updates and then we can rebase onto an upstream version that probably has the vulnerabilities fixed.

@alexandreLamarre
Copy link

We're on a quite "old" version from Sep 6

@jan-law
Copy link
Collaborator Author

jan-law commented Nov 7, 2023

@alexandreLamarre The vulnerabilities aren't related to my changes. They're in a different go module. I can revert my attempt to fix those packages

@jan-law
Copy link
Collaborator Author

jan-law commented Nov 8, 2023

The build-package (deb) job fails consistently with the error below. All other jobs besides the govulnchecks are passing.

git describe --abbrev=0 --match 'v[0-9]*'
fatal: No names found, cannot describe anything.

Is there anything I can do on my end?

@jan-law jan-law merged commit 9f8e669 into main Nov 8, 2023
85 of 91 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.

3 participants