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

fix: resolve tag conflicts for versioned artifacts #920

Merged
merged 7 commits into from
Sep 11, 2023

Conversation

rdimitrov
Copy link
Member

On each publish event for a package/artifact we insert a new record in artifact_versions which has a given set of tags associated with it. After a while we end up having multiple records that share the same tag entries (or one is a subset of the other) which is wrong given that a tag should be matched to only one image digest.

The following PR introduces the idea of searching for all existing entries that match the incoming tag value in their Tags field. If found, the existing artifact is updated by removing the incoming tag from its tags column.

Fixes #814

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
@rdimitrov rdimitrov self-assigned this Sep 8, 2023
@rdimitrov rdimitrov added bug Something isn't working go Pull requests that update Go code labels Sep 8, 2023
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
pkg/controlplane/handlers_githubwebhooks.go Outdated Show resolved Hide resolved
pkg/controlplane/handlers_githubwebhooks.go Show resolved Hide resolved
pkg/controlplane/handlers_githubwebhooks.go Show resolved Hide resolved
pkg/controlplane/handlers_githubwebhooks.go Outdated Show resolved Hide resolved
pkg/controlplane/handlers_githubwebhooks.go Outdated Show resolved Hide resolved
@JAORMX
Copy link
Contributor

JAORMX commented Sep 11, 2023

Your patch looks good overall. Just added some "nits". Feel free to take them or continue as-is if you want.

Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
@rdimitrov
Copy link
Member Author

Your patch looks good overall. Just added some "nits". Feel free to take them or continue as-is if you want.

Thanks, I've just addressed your feedback 👍

Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

looking good, some nits inline

JAORMX
JAORMX previously approved these changes Sep 11, 2023
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
@rdimitrov
Copy link
Member Author

@jhrozek - addressed your feedback, so feel free to re-review 👍

Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

thank you for the fix!

@rdimitrov rdimitrov merged commit eb75023 into mindersec:main Sep 11, 2023
12 checks passed
@rdimitrov rdimitrov deleted the fix-multiple-tags branch September 11, 2023 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

when an artifact is re-tagged, the old tag is still stored in the DB
3 participants