Skip to content

Add MySQL support to backfill script#2081

Merged
Hayden-IO merged 3 commits intosigstore:mainfrom
cmurphy:mysql-backfill
Apr 30, 2024
Merged

Add MySQL support to backfill script#2081
Hayden-IO merged 3 commits intosigstore:mainfrom
cmurphy:mysql-backfill

Conversation

@cmurphy
Copy link
Copy Markdown
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

@codecov
Copy link
Copy Markdown

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 requested a review from Hayden-IO 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
Copy Markdown
Contributor

@Hayden-IO Hayden-IO 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

Comment thread cmd/backfill-index/main.go
Comment thread docker-compose.backfill-test.yml Outdated
@@ -0,0 +1,61 @@
#
# Copyright 2022 The Sigstore Authors.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2024

cmurphy added 3 commits April 30, 2024 10:35
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>
@Hayden-IO Hayden-IO merged commit 5d0bb6e into sigstore:main Apr 30, 2024
@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.

2 participants