Add clearer error for VPP token constraint failure #21967
Conversation
dantecatalfamo
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
dantecatalfamo
changed the title
dantecatalfamo
had a problem deploying
to
Docker Hub
GitHub Actions
Error
server/datastore/mysql/vpp.go
Outdated
| @@ -858,6 +860,17 @@ func (ds *Datastore) UpdateVPPTokenTeams(ctx context.Context, id uint, teams []u | |||
| }) | |||
|
|
|||
| if err != nil { | |||
| mysqlErr := &mysql.MySQLError{} | |||
| // https://dev.mysql.com/doc/mysql-errors/8.4/en/server-error-reference.html#error_er_dup_entry | |||
| if errors.As(err, &mysqlErr) && mysqlErr.Number == 1062 { | |||
There was a problem hiding this comment.
Nice! Heads up: we have an IsDuplicate function in the mysql/errors.go file that does this check, can probably use that
There was a problem hiding this comment.
Dang good to know
dantecatalfamo
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #21967 +/- ##
==========================================
- Coverage 64.96% 64.93% -0.03%
==========================================
Files 1492 1492
Lines 116166 116670 +504
Branches 3466 3466
==========================================
+ Hits 75467 75760 +293
- Misses 33694 33853 +159
- Partials 7005 7057 +52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -858,6 +860,17 @@ func (ds *Datastore) UpdateVPPTokenTeams(ctx context.Context, id uint, teams []u | |||
| }) | |||
|
|
|||
| if err != nil { | |||
| var mysqlErr *mysql.MySQLError | |||
There was a problem hiding this comment.
This is an example of how a duplicate error is handed elsewhere in the codebase, sorry should have linked this originally! IsDuplicate does the cast to a mysql.MySQLError, so that should be ok to leave out here.
https://github.com/fleetdm/fleet/blob/main/server/datastore/mysql/app_configs.go#L172-L175
There was a problem hiding this comment.
I do the cast so I can extract the ID of the duplicate below, but good to know as well!
server/fleet/vpp.go
Outdated
| } | ||
|
|
||
| func (e ErrVPPTokenTeamConstraint) Error() string { | ||
| return fmt.Sprintf("Error: %q team already has a VPP token. Each team can only have on VPP token.", e.Name) |
There was a problem hiding this comment.
nit: smol typo
can only have one VPP token
dantecatalfamo
temporarily deployed
to
Docker Hub
GitHub Actions
Inactive
|
The integration test doesn't fail when I run it locally, I'm going to try running the whole suite locally to see if I can find the issue |
Everything passes without issue, very interesting |
#21890
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.