-
Notifications
You must be signed in to change notification settings - Fork 663
fix: use correct projectName in container monitor JSON output #6343
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
base: main
Are you sure you want to change the base?
Conversation
0323e7e to
6d0e892
Compare
The projectName field in 'snyk container monitor --json' output was incorrectly set to monitorResult.id (the monitor's public ID) instead of monitorResult.projectName (the actual project name). This caused the JSON output to display a UUID instead of the project's display name (e.g., the image name or --project-name value). Changes: - Fixed src/lib/ecosystems/monitor.ts line 214 to use monitorResult.projectName - Added unit test with mocked registry response to validate the fix - Updated acceptance tests with correct expected projectName values
192702e to
48e3995
Compare
| data: monOutput, | ||
| path: monitorResult.path, | ||
| projectName: monitorResult.id, | ||
| projectName: monitorResult.projectName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes look good. I have only one question, do you think having the a non unique project name reported for base image/ application project could break any automation around the json output?
Current behavior: json output for node:lates
cat json_output | jq '.[].projectName' "bae0b739-9d1c-4e04-99ac-3e7af3faa271" "73e39537-0bf5-45ed-9621-06d83dc9d69f"
the projectName will be same for both projects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should not actually be the same projectName for both projects. One will have a target file appended. For example, when setting --project-name=brians_project on a container with a JS application, there are two different projects created:
brians_project:/usr/lib/node_modules(JS app)brians_project(base image)
|
@bgardiner I personally agree to your framing of this not being a breaking change but there is also a possibility that not all users agree. So I think we want to be more explicit in the commit message/PR so that the release notes will clearly mention this behaviour.
Then I wonder if we can offer an alternative path for users who actually use the project id, for example in automations. |
Thinking about the breaking change nature of this I dismissed my approval to have a bit more of a conversation.
Summary
The
projectNamefield in the JSON output ofsnyk container monitor --jsoncontains the monitor's public ID (UUID) instead of the actual project name.Changes
File:
src/lib/ecosystems/monitor.tsChanged line 214 from:
to:
Root Cause
The registry correctly returns
projectName: project.namein theMonitorDependenciesResponse, but the CLI was overwriting it withmonitorResult.idwhen building the JSON output.Breaking Change Consideration
This is changing the JSON response of a CLI command, so it should be assessed for breaking change. I do NOT think it qualifies as a breaking change. The return type (string) and field key remain the same. The previous field value returned something completely unusable by the user (the monitor's public ID), as there is no public API call that references the monitor ID. This corrects a bug and should NOT negatively impact customer's existing use of the monitor command.
Steps to Reproduce (before fix)
Output before fix:
Expected output after fix:
projectNameshould contain the project's display name (e.g., automatically determined value from the image name or the value passed via--project-name).