Skip to content

Modify govulncheck to only check cwagent components and fix lint issues #316

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

Closed
wants to merge 13 commits into from

Conversation

lisguo
Copy link

@lisguo lisguo commented May 20, 2025

Description

Fix the PR build checks on this repo by running make fmt and modifying the govulncheck workflow to only look at components referenced in the cloudwatch agent repo.

Also modified PR template to run make fmt and verify checks

Testing

Tested as part of this PR check: https://github.com/amazon-contributing/opentelemetry-collector-contrib/actions/runs/15144600074/job/42577134176?pr=316

Looks at the following components:

./exporter/awscloudwatchlogsexporter
./exporter/awsemfexporter
./exporter/awsxrayexporter
./extension/awsmiddleware
./extension/awsproxy 
./internal/coreinternal 
./internal/k8sconfig 
./internal/kubelet 
./internal/metadataproviders
./override/aws 
./pkg/resourcetotelemetry
./pkg/stanza 
./processor/resourcedetectionprocessor 
./receiver/awscontainerinsightreceiver 
./receiver/awscontainerinsightskueuereceiver 
./receiver/awsxrayreceiver 
./receiver/jmxreceiver 
./receiver/prometheusreceiver

@lisguo lisguo changed the title Fix lint. Modify govulncheck to only check cwagent components Modify govulncheck to only check cwagent components and fix lint issues May 20, 2025
@lisguo lisguo requested review from jefchien and dricross May 20, 2025 17:52
@lisguo lisguo marked this pull request as ready for review May 20, 2025 17:52
@lisguo lisguo requested a review from mxiamxia as a code owner May 20, 2025 17:52
@lisguo lisguo removed the request for review from mxiamxia May 20, 2025 17:58
@dricross
Copy link

Should we fix the existing govulnchecks as part of this PR?

@lisguo
Copy link
Author

lisguo commented May 21, 2025

Should we fix the existing govulnchecks as part of this PR?

Yeah I was thinking about it, but I believe we are already working on an otel bump which should resolve this.

@@ -141,25 +141,24 @@ jobs:
fail-fast: false

Choose a reason for hiding this comment

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

nit: Don't need the matrix if it's only one item.

id: get-components
run: |
CWAGENT_COMPONENTS=$(grep -o 'amazon-contributing/opentelemetry-collector-contrib/[^[:space:]/]*/[^[:space:]]*' amazon-cloudwatch-agent/go.mod | \
grep -v 'pull' | \

Choose a reason for hiding this comment

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

Guessing this is to ignore the comments with PR links.

@@ -181,7 +180,9 @@ jobs:
if: steps.go-cache.outputs.cache-hit != 'true'
run: make install-tools
- name: Run `govulncheck`
run: make -j2 gogovulncheck GROUP=${{ matrix.group }}
run: make -j2 gogovulncheck GROUP=cwagent

Choose a reason for hiding this comment

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

Can you have it print out the components it's checking?

run: |
CWAGENT_COMPONENTS=$(grep -o 'amazon-contributing/opentelemetry-collector-contrib/[^[:space:]/]*/[^[:space:]]*' amazon-cloudwatch-agent/go.mod | \
grep -v 'pull' | \
sed -n 's|amazon-contributing/opentelemetry-collector-contrib/\([^/]*/[^/]*\)$|./\1|p' | \

Choose a reason for hiding this comment

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

Seems like this will only work for packages that have only two levels (e.g. internal/coreutils) and not anything nested further. The list in the description is missing all the internal/aws/* packages and the pkg/translator/prometheus package.

Copy link

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jun 12, 2025
@lisguo lisguo closed this Jun 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants