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

grouped security updates: use the group if one is defined #8742

Merged
merged 35 commits into from
Jan 31, 2024

Conversation

jakecoffman
Copy link
Member

@jakecoffman jakecoffman commented Jan 9, 2024

Previously we assumed that a grouped security update would just group all of the dependencies passed in the job.

This PR will cause grouped security updates to use the one defined in the job instead.

We can't merge this until the service starts sending groups for these types of jobs.

I've updated this to fallback to the old behavior, so it should be mergeable now.

@jakecoffman jakecoffman changed the title use the security group if one is defined grouped security updates: use the group if one is defined Jan 10, 2024
@jakecoffman
Copy link
Member Author

jakecoffman commented Jan 26, 2024

Almost good to go, but the devcontainers smoke test is trying to tell me something is wrong.

@jakecoffman jakecoffman marked this pull request as ready for review January 26, 2024 20:19
@jakecoffman jakecoffman requested a review from a team as a code owner January 26, 2024 20:19
@jakecoffman
Copy link
Member Author

Some of the new tests I just merged in are failing, so taking it back to draft.

@jakecoffman jakecoffman marked this pull request as draft January 29, 2024 15:44
@jakecoffman jakecoffman marked this pull request as ready for review January 29, 2024 19:35
@jakecoffman
Copy link
Member Author

I deployed this PR and saw exceptions where a grouped security update with 1 dependency wasn't getting picked up by the GroupUpdateAllVersions operation. So in 06673bf I swapped the check for an implicit group to be if directories is present which works better.

This will all be cleaned up once the API sends groups! 😓

@jakecoffman jakecoffman merged commit 3d7d061 into main Jan 31, 2024
120 of 121 checks passed
@jakecoffman jakecoffman deleted the jakecoffman/use-security-group-if-one-is-defined branch January 31, 2024 16:31
@jakecoffman
Copy link
Member Author

Redeployed and it looks good!

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

3 participants