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

Add MySQL support to backfill script #2081

Merged
merged 3 commits into from Apr 30, 2024

Conversation

cmurphy
Copy link
Contributor

@cmurphy cmurphy commented Apr 12, 2024

Rename backfill-redis to backfill-index and add support for when MySQL
is used as the index storage backend.

Some Redis-specific parameters are renamed to clearly differentiate them
from MySQL parameters. MySQL connection parameters mirror the
rekor-server paramters, using a single --dsn flag instead of separate
host, port, password, etc flags.

To do:

  • Rename backfill-redis and combine both backfill scripts into one?
  • Functional tests for backfill-redis
  • Functional tests for backfill-mysql

Summary

Release Note

Documentation

Copy link

codecov bot commented Apr 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.95%. Comparing base (488eb97) to head (0a325fd).
Report is 100 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2081       +/-   ##
===========================================
- Coverage   66.46%   48.95%   -17.51%     
===========================================
  Files          92       80       -12     
  Lines        9258     6639     -2619     
===========================================
- Hits         6153     3250     -2903     
- Misses       2359     2985      +626     
+ Partials      746      404      -342     
Flag Coverage Δ
e2etests ?
unittests 48.95% <ø> (+1.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cmurphy cmurphy force-pushed the mysql-backfill branch 7 times, most recently from 2e50cd7 to 0753006 Compare April 25, 2024 15:34
@cmurphy cmurphy changed the title [WIP] Add backfill-mysql script Add MySQL support to backfill script Apr 25, 2024
@cmurphy cmurphy marked this pull request as ready for review April 25, 2024 16:52
@cmurphy cmurphy requested a review from a team as a code owner April 25, 2024 16:52
@cmurphy cmurphy force-pushed the mysql-backfill branch 2 times, most recently from f3d78c4 to f94da75 Compare April 25, 2024 16:55
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

LGTM, fantastic work on the very thorough tests!

run: echo "GOVERSION=$(cat Dockerfile|grep golang | awk ' { print $2 } ' | cut -d '@' -f 1 | cut -d ':' -f 2 | uniq)" >> $GITHUB_ENV
- uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
with:
go-version: ${{ env.GOVERSION }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use go-version-file: go.mod instead? Dockerfile and go.mod should stay in sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can, but that would make this job inconsistent with every other workflow in rekor, fulcio, and cosign, for example line 89 above. Do you want this changed everywhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep it consistent, we can change it in a follow up.

cmd/backfill-index/main.go Show resolved Hide resolved
@@ -0,0 +1,61 @@
#
# Copyright 2022 The Sigstore Authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

2024

Signed-off-by: Colleen Murphy <colleenmurphy@google.com>
Rename backfill-redis to backfill-index and add support for when MySQL
is used as the index storage backend.

Some Redis-specific parameters are renamed to clearly differentiate them
from MySQL parameters. MySQL connection parameters mirror the
rekor-server paramters, using a single --dsn flag instead of separate
host, port, password, etc flags.

Signed-off-by: Colleen Murphy <colleenmurphy@google.com>
Signed-off-by: Colleen Murphy <colleenmurphy@google.com>
@haydentherapper haydentherapper merged commit 5d0bb6e into sigstore:main Apr 30, 2024
15 checks passed
@github-actions github-actions bot added this to the v1.2.2 milestone Apr 30, 2024
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

2 participants