From 9a5e0795396eca693cc03a9df11784af99c38983 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Mon, 30 May 2022 11:26:49 +0100 Subject: [PATCH 1/3] Include request IDs in errors when available. --- internal/githubapiutil/githubapiutil.go | 13 ++++ internal/githubapiutil/githubapiutil_test.go | 14 ++++ internal/pull/pull.go | 5 +- internal/push/push.go | 79 +++++++++++++------- 4 files changed, 83 insertions(+), 28 deletions(-) diff --git a/internal/githubapiutil/githubapiutil.go b/internal/githubapiutil/githubapiutil.go index c9dc867..baa1e69 100644 --- a/internal/githubapiutil/githubapiutil.go +++ b/internal/githubapiutil/githubapiutil.go @@ -4,9 +4,11 @@ import ( "strings" "github.com/google/go-github/v32/github" + "github.com/pkg/errors" ) const xOAuthScopesHeader = "X-OAuth-Scopes" +const xGitHubRequestIDHeader = "X-GitHub-Request-ID" func HasAnyScope(response *github.Response, scopes ...string) bool { if response == nil { @@ -26,3 +28,14 @@ func HasAnyScope(response *github.Response, scopes ...string) bool { } return false } + +func EnrichResponseError(response *github.Response, err error, message string) error { + requestID := "" + if response != nil { + requestID = response.Header.Get(xGitHubRequestIDHeader) + } + if requestID != "" { + message = message + " (" + requestID + ")" + } + return errors.Wrap(err, message) +} diff --git a/internal/githubapiutil/githubapiutil_test.go b/internal/githubapiutil/githubapiutil_test.go index 7b856b1..ffc8b0f 100644 --- a/internal/githubapiutil/githubapiutil_test.go +++ b/internal/githubapiutil/githubapiutil_test.go @@ -1,6 +1,7 @@ package githubapiutil import ( + "errors" "net/http" "testing" @@ -23,3 +24,16 @@ func TestHasAnyScope(t *testing.T) { response.Header.Set(xOAuthScopesHeader, "gist, notifications, admin:org") require.False(t, HasAnyScope(&response, "public_repo", "repo")) } + +func TestEnrichErrorResponse(t *testing.T) { + response := github.Response{ + Response: &http.Response{Header: http.Header{}}, + } + + require.Equal(t, "The error message.: The underlying error.", EnrichResponseError(nil, errors.New("The underlying error."), "The error message.").Error()) + + require.Equal(t, "The error message.: The underlying error.", EnrichResponseError(&response, errors.New("The underlying error."), "The error message.").Error()) + + response.Header.Set(xGitHubRequestIDHeader, "AAAA:BBBB:CCCCCCC:DDDDDDD:EEEEEEEE") + require.Equal(t, "The error message. (AAAA:BBBB:CCCCCCC:DDDDDDD:EEEEEEEE): The underlying error.", EnrichResponseError(&response, errors.New("The underlying error."), "The error message.").Error()) +} diff --git a/internal/pull/pull.go b/internal/pull/pull.go index 67f89b1..34bd0c6 100644 --- a/internal/pull/pull.go +++ b/internal/pull/pull.go @@ -13,6 +13,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/github/codeql-action-sync/internal/actionconfiguration" + "github.com/github/codeql-action-sync/internal/githubapiutil" "github.com/mitchellh/ioprogress" "golang.org/x/oauth2" @@ -191,9 +192,9 @@ func (pullService *pullService) pullReleases() error { for index, releaseTag := range relevantReleases { log.Debugf("Pulling CodeQL bundle %s (%d/%d)...", releaseTag, index+1, len(relevantReleases)) - release, _, err := pullService.githubDotComClient.Repositories.GetReleaseByTag(pullService.ctx, sourceOwner, sourceRepository, releaseTag) + release, response, err := pullService.githubDotComClient.Repositories.GetReleaseByTag(pullService.ctx, sourceOwner, sourceRepository, releaseTag) if err != nil { - return errors.Wrap(err, "Error loading CodeQL release information.") + return githubapiutil.EnrichResponseError(response, err, "Error loading CodeQL release information.") } err = os.MkdirAll(pullService.cacheDirectory.ReleasePath(releaseTag), 0755) if err != nil { diff --git a/internal/push/push.go b/internal/push/push.go index 5fa60db..4e00376 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -72,7 +72,7 @@ func (pushService *pushService) createRepository() (*github.Repository, error) { if response != nil && response.StatusCode == http.StatusUnauthorized { return nil, usererrors.New(errorInvalidDestinationToken) } - return nil, errors.Wrap(err, "Error getting current user.") + return nil, githubapiutil.EnrichResponseError(response, err, "Error getting current user.") } // When creating a repository we can either create it in a named organization or under the current user (represented in go-github by an empty string). @@ -84,11 +84,11 @@ func (pushService *pushService) createRepository() (*github.Repository, error) { if destinationOrganization != "" { _, response, err := pushService.githubEnterpriseClient.Organizations.Get(pushService.ctx, pushService.destinationRepositoryOwner) if err != nil && (response == nil || response.StatusCode != http.StatusNotFound) { - return nil, errors.Wrap(err, "Error checking if destination organization exists.") + return nil, githubapiutil.EnrichResponseError(response, err, "Error checking if destination organization exists.") } if response != nil && response.StatusCode == http.StatusNotFound { log.Debugf("The organization %s does not exist. Creating it...", pushService.destinationRepositoryOwner) - _, _, err := pushService.githubEnterpriseClient.Admin.CreateOrg(pushService.ctx, &github.Organization{ + _, response, err := pushService.githubEnterpriseClient.Admin.CreateOrg(pushService.ctx, &github.Organization{ Login: github.String(pushService.destinationRepositoryOwner), Name: github.String(pushService.destinationRepositoryOwner), }, user.GetLogin()) @@ -96,19 +96,19 @@ func (pushService *pushService) createRepository() (*github.Repository, error) { if response != nil && response.StatusCode == http.StatusNotFound && !githubapiutil.HasAnyScope(response, "site_admin") { return nil, usererrors.New("The destination token you have provided does not have the `site_admin` scope, so the destination organization cannot be created.") } - return nil, errors.Wrap(err, "Error creating organization.") + return nil, githubapiutil.EnrichResponseError(response, err, "Error creating organization.") } } _, response, err = pushService.githubEnterpriseClient.Organizations.IsMember(pushService.ctx, pushService.destinationRepositoryOwner, user.GetLogin()) if err != nil { - return nil, errors.Wrap(err, "Failed to check membership of destination organization.") + return nil, githubapiutil.EnrichResponseError(response, err, "Failed to check membership of destination organization.") } if (response.StatusCode == http.StatusFound || response.StatusCode == http.StatusNotFound) && githubapiutil.HasAnyScope(response, "site_admin") { log.Debugf("No access to destination organization (status code %d). Switching to impersonation token for %s...", response.StatusCode, pushService.actionsAdminUser) - impersonationToken, _, err := pushService.githubEnterpriseClient.Admin.CreateUserImpersonation(pushService.ctx, pushService.actionsAdminUser, &github.ImpersonateUserOptions{Scopes: []string{minimumRepositoryScope, "workflow"}}) + impersonationToken, response, err := pushService.githubEnterpriseClient.Admin.CreateUserImpersonation(pushService.ctx, pushService.actionsAdminUser, &github.ImpersonateUserOptions{Scopes: []string{minimumRepositoryScope, "workflow"}}) if err != nil { - return nil, errors.Wrap(err, "Failed to impersonate Actions admin user.") + return nil, githubapiutil.EnrichResponseError(response, err, "Failed to impersonate Actions admin user.") } pushService.destinationToken.AccessToken = impersonationToken.GetToken() } @@ -116,7 +116,7 @@ func (pushService *pushService) createRepository() (*github.Repository, error) { repository, response, err := pushService.githubEnterpriseClient.Repositories.Get(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName) if err != nil && (response == nil || response.StatusCode != http.StatusNotFound) { - return nil, errors.Wrap(err, "Error checking if destination repository exists.") + return nil, githubapiutil.EnrichResponseError(response, err, "Error checking if destination repository exists.") } if response.StatusCode != http.StatusNotFound && repositoryHomepage != repository.GetHomepage() && !pushService.force { return nil, errors.Errorf(errorAlreadyExists) @@ -143,7 +143,7 @@ func (pushService *pushService) createRepository() (*github.Repository, error) { if response.StatusCode == http.StatusNotFound && !githubapiutil.HasAnyScope(response, acceptableRepositoryScopes...) { return nil, fmt.Errorf("The destination token you have provided does not have the `%s` scope.", minimumRepositoryScope) } - return nil, errors.Wrap(err, "Error creating destination repository.") + return nil, githubapiutil.EnrichResponseError(response, err, "Error creating destination repository.") } } else { log.Debug("Repository already exists. Updating its metadata...") @@ -156,7 +156,7 @@ func (pushService *pushService) createRepository() (*github.Repository, error) { return nil, fmt.Errorf("You don't have permission to update the repository at %s/%s. If you wish to update the bundled CodeQL Action please provide a token with the `site_admin` scope.", pushService.destinationRepositoryOwner, pushService.destinationRepositoryName) } } - return nil, errors.Wrap(err, "Error updating destination repository.") + return nil, githubapiutil.EnrichResponseError(response, err, "Error updating destination repository.") } } @@ -212,6 +212,7 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu } refSpecBatches = append(refSpecBatches, deleteRefSpecs) + defaultBrachRefSpec := "+refs/heads/main:refs/heads/main" if initialPush { releasePathStats, err := ioutil.ReadDir(pushService.cacheDirectory.ReleasesPath()) if err != nil { @@ -228,10 +229,10 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu } refSpecBatches = append(refSpecBatches, initialRefSpecs) } else { - // We've got to push `main` on its own, so that it will be made the default branch if the repository has just been created. We then push everything else afterwards. + // We've got to push the default branch on its own, so that it will be made the default branch if the repository has just been created. We then push everything else afterwards. refSpecBatches = append(refSpecBatches, []config.RefSpec{ - config.RefSpec("+refs/heads/main:refs/heads/main"), + config.RefSpec(defaultBrachRefSpec), }, []config.RefSpec{ config.RefSpec("+refs/*:refs/*"), @@ -245,6 +246,32 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu Auth: credentials, Progress: os.Stderr, }) + if err != nil && strings.Contains(err.Error(), "pre-receive hook declined") { + log.Warn("Push was rejected by a pre-receive hook. This may be because force-pushing is not allowed. Will try and remove and recreate the branch.") + if len(refSpecs) == 1 && refSpecs[0].String() == defaultBrachRefSpec { + // todo + } + negativeRefSpecs := []config.RefSpec{} + for _, refSpec := range refSpecs { + negativeRefSpecs = append(negativeRefSpecs, config.RefSpec(":"+refSpec.Src())) + err = remote.PushContext(pushService.ctx, &git.PushOptions{ + RefSpecs: negativeRefSpecs, + Auth: credentials, + Progress: os.Stderr, + }) + if err != nil { + return errors.Wrap(err, "Error removing existing refs.") + } + err = remote.PushContext(pushService.ctx, &git.PushOptions{ + RefSpecs: refSpecs, + Auth: credentials, + Progress: os.Stderr, + }) + if err != nil && errors.Cause(err) != git.NoErrAlreadyUpToDate { + return errors.Wrap(err, "Error pushing Action to GitHub Enterprise Server.") + } + } + } if err != nil && errors.Cause(err) != git.NoErrAlreadyUpToDate { return errors.Wrap(err, "Error pushing Action to GitHub Enterprise Server.") } @@ -270,20 +297,20 @@ func (pushService *pushService) createOrUpdateRelease(releaseName string) (*gith release, response, err := pushService.githubEnterpriseClient.Repositories.GetReleaseByTag(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, releaseMetadata.GetTagName()) if err != nil && response.StatusCode != http.StatusNotFound { - return nil, errors.Wrap(err, "Error checking for existing CodeQL release.") + return nil, githubapiutil.EnrichResponseError(response, err, "Error checking for existing CodeQL release.") } if release == nil { log.Debugf("Creating release %s...", releaseMetadata.GetTagName()) - release, _, err := pushService.githubEnterpriseClient.Repositories.CreateRelease(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, &releaseMetadata) + release, response, err := pushService.githubEnterpriseClient.Repositories.CreateRelease(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, &releaseMetadata) if err != nil { - return nil, errors.Wrap(err, "Error creating release.") + return nil, githubapiutil.EnrichResponseError(response, err, "Error creating release.") } return release, nil } - release, _, err = pushService.githubEnterpriseClient.Repositories.EditRelease(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, release.GetID(), &releaseMetadata) + release, response, err = pushService.githubEnterpriseClient.Repositories.EditRelease(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, release.GetID(), &releaseMetadata) if err != nil { log.Debugf("Updating release %s...", releaseMetadata.GetTagName()) - return nil, errors.Wrap(err, "Error updating release.") + return nil, githubapiutil.EnrichResponseError(response, err, "Error updating release.") } return release, nil } @@ -301,7 +328,7 @@ func (pushService *pushService) uploadReleaseAsset(release *github.RepositoryRel asset := &github.ReleaseAsset{} response, err := pushService.githubEnterpriseClient.Do(pushService.ctx, request, asset) if err != nil { - return nil, response, errors.Wrap(err, "Error uploading release asset.") + return nil, response, githubapiutil.EnrichResponseError(response, err, "Error uploading release asset.") } return asset, response, nil } @@ -315,9 +342,9 @@ func (pushService *pushService) createOrUpdateReleaseAsset(release *github.Repos return nil } else { log.Warnf("Removing existing release asset %s because it was only partially-uploaded (had size %d, but should have been %d)...", existingAsset.GetName(), actualSize, expectedSize) - _, err := pushService.githubEnterpriseClient.Repositories.DeleteReleaseAsset(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, existingAsset.GetID()) + response, err := pushService.githubEnterpriseClient.Repositories.DeleteReleaseAsset(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, existingAsset.GetID()) if err != nil { - return errors.Wrap(err, "Error deleting existing release asset.") + return githubapiutil.EnrichResponseError(response, err, "Error deleting existing release asset.") } } } @@ -333,9 +360,9 @@ func (pushService *pushService) createOrUpdateReleaseAsset(release *github.Repos if err != nil { return errors.Wrap(err, "Error opening release asset.") } - _, _, err = pushService.uploadReleaseAsset(release, assetPathStat, progressReader) + _, response, err := pushService.uploadReleaseAsset(release, assetPathStat, progressReader) if err != nil { - return errors.Wrap(err, "Error uploading release asset.") + return githubapiutil.EnrichResponseError(response, err, "Error uploading release asset.") } return nil } @@ -358,9 +385,9 @@ func (pushService *pushService) pushReleases() error { existingAssets := []*github.ReleaseAsset{} for page := 1; ; page++ { - assets, _, err := pushService.githubEnterpriseClient.Repositories.ListReleaseAssets(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, release.GetID(), &github.ListOptions{Page: page}) + assets, response, err := pushService.githubEnterpriseClient.Repositories.ListReleaseAssets(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, release.GetID(), &github.ListOptions{Page: page}) if err != nil { - return errors.Wrap(err, "Error fetching existing release assets.") + return githubapiutil.EnrichResponseError(response, err, "Error fetching existing release assets.") } if len(assets) == 0 { break @@ -376,7 +403,7 @@ func (pushService *pushService) pushReleases() error { for _, assetPathStat := range assetPathStats { err := pushService.createOrUpdateReleaseAsset(release, existingAssets, assetPathStat) if err != nil { - return errors.Wrap(err, "Error uploading release assets.") + return err } } } @@ -410,7 +437,7 @@ func Push(ctx context.Context, cacheDirectory cachedirectory.CacheDirectory, des } rootResponse, err := client.Do(ctx, rootRequest, nil) if err != nil { - return errors.Wrap(err, "Error checking connectivity for GitHub Enterprise client.") + return githubapiutil.EnrichResponseError(rootResponse, err, "Error checking connectivity for GitHub Enterprise client.") } if rootRequest.URL != rootResponse.Request.URL { updatedBaseURL, _ := url.Parse(client.BaseURL.String()) From 410478b0ed35d6bba29c482fda8c7b3e5d20d86f Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Tue, 14 Jun 2022 14:23:26 +0100 Subject: [PATCH 2/3] Fix a typo. Co-authored-by: Simon Engledew --- internal/push/push.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/push/push.go b/internal/push/push.go index 4e00376..1a28508 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -212,7 +212,7 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu } refSpecBatches = append(refSpecBatches, deleteRefSpecs) - defaultBrachRefSpec := "+refs/heads/main:refs/heads/main" + defaultBranchRefSpec := "+refs/heads/main:refs/heads/main" if initialPush { releasePathStats, err := ioutil.ReadDir(pushService.cacheDirectory.ReleasesPath()) if err != nil { @@ -232,7 +232,7 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu // We've got to push the default branch on its own, so that it will be made the default branch if the repository has just been created. We then push everything else afterwards. refSpecBatches = append(refSpecBatches, []config.RefSpec{ - config.RefSpec(defaultBrachRefSpec), + config.RefSpec(defaultBranchRefSpec), }, []config.RefSpec{ config.RefSpec("+refs/*:refs/*"), From c4a05147c00efd8842644f52e4ed733efc044b03 Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Tue, 14 Jun 2022 14:25:17 +0100 Subject: [PATCH 3/3] Remove a change meant for another PR. --- internal/push/push.go | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/internal/push/push.go b/internal/push/push.go index 1a28508..94ff3ed 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -246,32 +246,6 @@ func (pushService *pushService) pushGit(repository *github.Repository, initialPu Auth: credentials, Progress: os.Stderr, }) - if err != nil && strings.Contains(err.Error(), "pre-receive hook declined") { - log.Warn("Push was rejected by a pre-receive hook. This may be because force-pushing is not allowed. Will try and remove and recreate the branch.") - if len(refSpecs) == 1 && refSpecs[0].String() == defaultBrachRefSpec { - // todo - } - negativeRefSpecs := []config.RefSpec{} - for _, refSpec := range refSpecs { - negativeRefSpecs = append(negativeRefSpecs, config.RefSpec(":"+refSpec.Src())) - err = remote.PushContext(pushService.ctx, &git.PushOptions{ - RefSpecs: negativeRefSpecs, - Auth: credentials, - Progress: os.Stderr, - }) - if err != nil { - return errors.Wrap(err, "Error removing existing refs.") - } - err = remote.PushContext(pushService.ctx, &git.PushOptions{ - RefSpecs: refSpecs, - Auth: credentials, - Progress: os.Stderr, - }) - if err != nil && errors.Cause(err) != git.NoErrAlreadyUpToDate { - return errors.Wrap(err, "Error pushing Action to GitHub Enterprise Server.") - } - } - } if err != nil && errors.Cause(err) != git.NoErrAlreadyUpToDate { return errors.Wrap(err, "Error pushing Action to GitHub Enterprise Server.") }