Move github credentials to the database #243
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
from
0409fb0 to
64e2e13
Compare
|
Pfu. The generated code is really bumping up the line count 😅 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
3 times, most recently
from
b43115d to
30447d3
Compare
Add database models that deal with github credentials. This change adds models for github endpoints (github.com, GHES, etc). This change also adds code to migrate config credntials to the DB. Tests need to be fixed and new tests need to be written. This will come in a later commit. Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
When updating credentials on an entity, we must ensure that the new credentials belong to the same endpoint as the entity. When an entity is created, the endpoint is determined by the credentials that were used during the create operation. From that point forward the entity is associated with an endpoint, and that cannot change. Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
from
5e513cf to
e8ea711
Compare
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
from
c23ff99 to
8b5584f
Compare
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
2 times, most recently
from
d6691fe to
8ef36f6
Compare
Do not rely on the entity object to hold updated or detailed credentials, fetch them from the DB every time. This change also ensures that we pass in the user context instead of the runner context to the DB methods. Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
2 times, most recently
from
80a87b0 to
7f1aeeb
Compare
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
from
7f1aeeb to
ccf5105
Compare
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
4 times, most recently
from
154c9de to
1dc6b1f
Compare
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
from
1dc6b1f to
0128f59
Compare
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
from
b19dcf2 to
8ec4798
Compare
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
from
8ec4798 to
402c8b7
Compare
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
changed the title
bavarianbidi
reviewed
May 7, 2024
No point in making a DB query if we know we don't want to be able to delete/update the default endpoint. Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
bavarianbidi
reviewed
May 7, 2024
Co-authored-by: Mario Constanti <github@constanti.de>
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
from
e054673 to
27e74ef
Compare
Signed-off-by: Gabriel Adrian Samfira <gsamfira@cloudbasesolutions.com>
gabriel-samfira
force-pushed
the
use-db-for-gh-creds
branch
from
a4d0fb0 to
b3e2c58
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change is part of a larger effort to allow GARM to eventually scale out to multiple instances. The goal is to move as much of the config as possible to the database, allowing a single source of truth for state among multiple instances.
This change also allows us to properly enforce relations between entities (repos, orgs, enterprises), github installations (GHES/github.com) and credentials. This way, we don't end up in an inconsistent state if the credentials are removed from the config, but they're still referenced for an entity in the database.
As part of this change, we aim to maintain as much of the existing user facing API as possible, and also automatically migrate the existing config credentials to the database. The migration is done once, when GARM detects that the needed DB tables are not there. After that, migration is skipped and any credential handling will need to be done using the CLI.
Credentials are scoped to users, and must have a unique name. Different users will be able to create credentials with the same name. The same user will not be able to create multiple credentials with the same name. Although GARM doesn't support multiple users (yet), it may do so in the future. Given the pain of moving the credentials to the DB, this was a decision that was meant to spare us the pain of refactoring later.
Credentials are still referenced by name when assigning them to the entity, but internally we query the DB for a user/credentials combo, given that we have the user ID already stored for the authenticated user, in the context we pass along throughout the code base.
A few more commits are needed to expose an API for handling credentials and to add tests for all the new bits.