[BB-508] sync with github app #67
Conversation
cd868e6 to
7ff46b6
Compare
pkg/config/config.go
Outdated
| syncSecrets = field.BoolField( | ||
| "sync-secrets", | ||
| field.WithDescription(`Whether to sync secrets or not`), | ||
| ) | ||
| installationID = field.StringField( |
There was a problem hiding this comment.
can we not use the org to get this? not sure if customers have this ID on hand
pkg/connector/connector.go
Outdated
| func (gh *GitHub) validateAppCredentials(ctx context.Context) (annotations.Annotations, error) { | ||
| orgLogins := gh.orgs | ||
| if len(orgLogins) > 1 { | ||
| return nil, fmt.Errorf("github-connector: Only one GitHub organization is allowed per group") |
There was a problem hiding this comment.
nit: what does "per group" mean here? should we say something like "Only one GitHub organization is allowed when authenticating with a github app"?
There was a problem hiding this comment.
How about Only one GitHub organization should be specified?
| syncSecrets = field.BoolField( | ||
| "sync-secrets", | ||
| field.WithDescription(`Whether to sync secrets or not`), | ||
| ) | ||
| fieldRelationships = []field.SchemaFieldRelationship{ | ||
| field.FieldsMutuallyExclusive( |
There was a problem hiding this comment.
nit: maybe we should also add FieldsAtLeastOneUsed(accessTokenField, appPrivateKeyPath) since we removed the field.WithRequired(true) from accessTokenField
pkg/connector/org.go
Outdated
| for orgName := range o.orgs { | ||
| installation, resp, err = findInstallation(ctx, o.appClient, orgName) | ||
| if err != nil { | ||
| return nil, "", nil, fmt.Errorf("github-connector: failed to fetch installations: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
why do we have to do this? can't we get the org from o.orgs
| @@ -384,7 +417,7 @@ func (o *orgResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotati | |||
| return nil, nil | |||
| } | |||
|
|
|||
| func orgBuilder(client *github.Client, orgCache *orgNameCache, orgs []string, syncSecrets bool) *orgResourceType { | |||
| func orgBuilder(client, appClient *github.Client, orgCache *orgNameCache, orgs []string, syncSecrets bool) *orgResourceType { | |||
| orgMap := make(map[string]struct{}) 8000 | |||
|
|
|||
| for _, o := range orgs { | |||
There was a problem hiding this comment.
@laurenleach Replied your comment:
We need org ID to construct org resources, that's why we need to call findInstallation() in order to get org id.
o.orgs only has org name.
There was a problem hiding this comment.
I thought you can get org from the name?
https://docs.github.com/en/rest/orgs/orgs?apiVersion=2022-11-28#get-an-organization
There was a problem hiding this comment.
thanks. this is better.
| orgCache: orgCache, | ||
| syncSecrets: syncSecrets, | ||
| } | ||
| } | ||
|
|
||
| func (o *orgResourceType) listOrganizationsFromAppInstallations( |
There was a problem hiding this comment.
we can just return 1 resource for this
Customer wants to get rid of the
service accountand usegithub appinstead.This PR adds the option to sync via the GitHub App.
Today, we use the personal access token to access resources that a user is granted to.
In github app, it's not the same story, since.
installation token is pinned to an organization, that means different organizations using the same app should have different installation tokens and one installation tokens from one org cannot access the other repository.
see here
Test.
The results are consistent with what we get when using a personal access token.
How to configure.
Install the GitHub App on your organizations.
Copy the private key and App ID.
Update the configuration with these two values.
Followup.
Intallation token expires in 1 hour, if the syn process takes more than 1 hour, we should refresh the token.