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

Track whether GitHub App is installed on org #2947

Merged
merged 1 commit into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/dev/app/container/cmd_verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func buildGitHubClient(token string) (provifv1.GitHub, error) {
}`),
},
sql.NullString{},
false,
credentials.NewGitHubTokenCredential(token),
&serverconfig.ProviderConfig{},
)
Expand Down
1 change: 1 addition & 0 deletions cmd/dev/app/rule_type/rttst.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ func testCmdRun(cmd *cobra.Command, _ []string) error {
}`),
},
sql.NullString{},
false,
credentials.NewGitHubTokenCredential(token),
&serverconfig.ProviderConfig{
GitHubApp: &serverconfig.GitHubAppConfig{
Expand Down
19 changes: 19 additions & 0 deletions database/migrations/000048_installation_metadata.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- Copyright 2024 Stacklok, Inc
--
-- Licensed under the Apache License, Version 2.0 (the "License");
-- you may not use this file except in compliance with the License.
-- You may obtain a copy of the License at
--
-- http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing, software
-- distributed under the License is distributed on an "AS IS" BASIS,
-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-- See the License for the specific language governing permissions and
-- limitations under the License.

BEGIN;

ALTER TABLE provider_github_app_installations DROP COLUMN is_org;

COMMIT;
19 changes: 19 additions & 0 deletions database/migrations/000048_installation_metadata.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- Copyright 2024 Stacklok, Inc
--
-- Licensed under the Apache License, Version 2.0 (the "License");
-- you may not use this file except in compliance with the License.
-- You may obtain a copy of the License at
--
-- http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing, software
-- distributed under the License is distributed on an "AS IS" BASIS,
-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-- See the License for the specific language governing permissions and
-- limitations under the License.

BEGIN;

ALTER TABLE provider_github_app_installations ADD COLUMN is_org BOOLEAN NOT NULL DEFAULT FALSE;
Copy link
Member

Choose a reason for hiding this comment

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

This looks safe to me because the default should apply for existing binaries that don't know about the column, allowing them to pass the NOT NULL constraint.


COMMIT;
5 changes: 3 additions & 2 deletions database/query/provider_github_app_installations.sql
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@ SELECT * FROM provider_github_app_installations WHERE app_installation_id = $1;

-- name: UpsertInstallationID :one
INSERT INTO provider_github_app_installations
(organization_id, app_installation_id, provider_id, enrolling_user_id, enrollment_nonce, project_id)
VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT (organization_id)
(organization_id, app_installation_id, provider_id, enrolling_user_id, enrollment_nonce, project_id, is_org)
VALUES ($1, $2, $3, $4, $5, $6, $7) ON CONFLICT (organization_id)
DO
UPDATE SET
app_installation_id = $2,
provider_id = $3,
enrolling_user_id = $4,
enrollment_nonce = $5,
project_id = $6,
is_org = $7,
updated_at = NOW()
WHERE provider_github_app_installations.organization_id = $1
RETURNING *;
Expand Down
8 changes: 3 additions & 5 deletions internal/controlplane/handlers_githubwebhooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,8 +634,7 @@ func gatherArtifactInfo(
}

// we also need to fill in the visibility which is not in the payload
isOrg := client.GetOwner() != ""
ghArtifact, err := client.GetPackageByName(ctx, isOrg, artifact.Owner, string(verifyif.ArtifactTypeContainer), artifact.Name)
ghArtifact, err := client.GetPackageByName(ctx, artifact.Owner, string(verifyif.ArtifactTypeContainer), artifact.Name)
if err != nil {
return nil, fmt.Errorf("error extracting artifact from repo: %w", err)
}
Expand Down Expand Up @@ -691,9 +690,8 @@ func updateArtifactVersionFromRegistry(
) error {
// we'll grab the artifact version from the REST endpoint because we need the visibility
// and createdAt fields which are not in the payload
isOrg := client.GetOwner() != ""
ghVersion, err := client.GetPackageVersionById(ctx, isOrg,
artifactOwnerLogin, string(verifyif.ArtifactTypeContainer), artifactName, version.VersionId)
ghVersion, err := client.GetPackageVersionById(ctx, artifactOwnerLogin, string(verifyif.ArtifactTypeContainer),
artifactName, version.VersionId)
if err != nil {
return fmt.Errorf("error getting package version from repository: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/controlplane/handlers_githubwebhooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ func (s *UnitTestSuite) TestHandleWebHookRepository() {
ID: 1,
ProjectID: projectID,
Provider: providerName,
}, nil).Times(2)
}, nil)

mockStore.EXPECT().
GetRepositoryByRepoID(gomock.Any(), gomock.Any()).
Expand Down
1 change: 1 addition & 0 deletions internal/db/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 15 additions & 7 deletions internal/db/provider_github_app_installations.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func testGithubProviderBuilder(baseURL string) *providers.ProviderBuilder {
Definition: json.RawMessage(definitionJSON),
},
sql.NullString{},
false,
credentials.NewGitHubTokenCredential("token"),
&serverconfig.ProviderConfig{},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func testGithubProviderBuilder() *providers.ProviderBuilder {
Definition: json.RawMessage(definitionJSON),
},
sql.NullString{},
false,
credentials.NewGitHubTokenCredential("token"),
&serverconfig.ProviderConfig{},
)
Expand Down
1 change: 1 addition & 0 deletions internal/engine/actions/remediate/remediate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ var (
}`),
},
sql.NullString{},
false,
credentials.NewGitHubTokenCredential("token"),
&serverconfig.ProviderConfig{},
)
Expand Down
3 changes: 3 additions & 0 deletions internal/engine/actions/remediate/rest/rest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ var (
}`),
},
sql.NullString{},
false,
credentials.NewGitHubTokenCredential("token"),
&serverconfig.ProviderConfig{},
)
Expand All @@ -74,6 +75,7 @@ var (
}`),
},
sql.NullString{},
false,
credentials.NewGitHubTokenCredential("token"),
&serverconfig.ProviderConfig{},
)
Expand All @@ -99,6 +101,7 @@ func testGithubProviderBuilder(baseURL string) *providers.ProviderBuilder {
Definition: json.RawMessage(definitionJSON),
},
sql.NullString{},
false,
credentials.NewGitHubTokenCredential("token"),
&serverconfig.ProviderConfig{},
)
Expand Down
2 changes: 1 addition & 1 deletion internal/engine/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func TestExecutor_handleEntityEvent(t *testing.T) {
}).
Return(db.ProviderAccessToken{
EncryptedToken: authtoken,
}, nil).Times(2)
}, nil)

// list one profile
crs := []*minderv1.Profile_Rule{
Expand Down
3 changes: 1 addition & 2 deletions internal/engine/ingester/artifact/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,7 @@ func getAndFilterArtifactVersions(
}

// Fetch all available versions of the artifact
isOrg := (ghCli.GetOwner() != "")
upstreamVersions, err := ghCli.GetPackageVersions(ctx, isOrg, artifact.Owner, artifact.GetTypeLower(), artifact.GetName())
upstreamVersions, err := ghCli.GetPackageVersions(ctx, artifact.Owner, artifact.GetTypeLower(), artifact.GetName())
if err != nil {
return nil, fmt.Errorf("error retrieving artifact versions: %w", err)
}
Expand Down
Loading
Loading