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 Grant revocation #118

Merged
merged 2 commits into from
Nov 21, 2022
Merged

Fix Grant revocation #118

merged 2 commits into from
Nov 21, 2022

Conversation

jabbrwcky
Copy link
Contributor

If the grant is already gone when the reconciler tries to revoke it,
it should mark the resource as successfully removed, not error.

MySQL 8.x+ supports
REVOKE %s IF EXISTS ON %s.%s FROM %s@%s IGNORE UNKNOWN USER
to avoid an error (which would be a cleaner solution IMO),
but this will only be possible when dropping support for earlier versions.

Description of your changes

Checks the actual mysql error code that is returned when the grant
revocation SQL errors; if the code states that the grant does not exist,
it returns nil for successful removal of the grant.

Basically uses the same code as

var myErr *mysqldriver.MySQLError
if errors.As(err, &myErr) && myErr.Number == errCodeNoSuchGrant {
// The user doesn't (yet) exist and therefore no grants either
return nil, &managed.ExternalObservation{ResourceExists: false}, nil
}

Fixes #47

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Added unit test for this case to grants/reconciler_test.go

@chlunde
Copy link
Contributor

chlunde commented Nov 10, 2022

Nice, could you git commit -s? See https://github.com/crossplane-contrib/provider-sql/pull/118/checks?check_run_id=9415901160

Just a heads up: I also think we will need a rebase when #105 is merged

@jabbrwcky
Copy link
Contributor Author

Commit is signed

@Duologic
Copy link
Member

Can you fix the lint error?

@jabbrwcky
Copy link
Contributor Author

Fixed the reported linter issues.

You might consider upgrading golangci-lint (if possible for github-actions?), the output from v 1.50.1 is much more helpful:

pkg/controller/mysql/grant/reconciler.go:333:4: commentFormatting: put a space between `//` and comment text (gocritic)
                        //MySQL automatically deletes related grants if the user has been deleted

Jens Hausherr added 2 commits November 18, 2022 13:26
If the grant is already gone when the reconclier tries to revoke it,
it should mark the resource as successfully removed, not error.

Signed-off-by: Jens Hausherr <jens.hausherr@sapiens.com>
Signed-off-by: Jens Hausherr <jens.hausherr@sapiens.com>
@Duologic Duologic merged commit 031c0bd into crossplane-contrib:master Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants