Skip to content

Make common errors easier to read. #36

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

Merged
merged 1 commit into from
Aug 25, 2020
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
19 changes: 10 additions & 9 deletions internal/cachedirectory/cachedirectory.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package cachedirectory

import (
"fmt"
usererrors "errors"
"io"
"io/ioutil"
"os"
Expand All @@ -15,7 +15,8 @@ import (
const errorCacheWrongVersion = "The cache you are trying to push was created with an old version of the CodeQL Action Sync tool. Please re-pull it with this version of the tool."
const errorNotACacheOrEmpty = "The cache directory you have selected is not empty, but was not created by the CodeQL Action Sync tool. If you are sure you want to use this directory, please delete it and run the sync tool again."
const errorCacheParentDoesNotExist = "Cannot create cache directory because its parent, does not exist."
const errorPushNonCache = "The directory you have provided does not appear to be valid. Please check it exists and that you have run the `pull` command to populate it."
const errorPushNonCache = "The cache directory you have provided does not appear to be valid. Please check it exists and that you have run the `pull` command to populate it."
const errorCacheLocked = "The cache directory is locked, likely due to a `pull` command being interrupted. Please run `pull` again to ensure all required data is downloaded."

const CacheReferencePrefix = "refs/remotes/" + git.DefaultRemoteName + "/"

Expand All @@ -35,7 +36,7 @@ func isEmptyOrNonExistentDirectory(path string) (bool, error) {
if os.IsNotExist(err) {
return true, nil
}
return false, errors.Wrap(err, fmt.Sprintf("Could not access directory %s.", path))
return false, errors.Wrapf(err, "Could not access directory %s.", path)
}
defer f.Close()

Expand All @@ -44,7 +45,7 @@ func isEmptyOrNonExistentDirectory(path string) (bool, error) {
if err == io.EOF {
return true, nil
}
return false, errors.Wrap(err, fmt.Sprintf("Could not read contents of directory %s.", path))
return false, errors.Wrapf(err, "Could not read contents of directory %s.", path)
}
return false, nil
}
Expand All @@ -67,7 +68,7 @@ func (cacheDirectory *CacheDirectory) CheckOrCreateVersionFile(pull bool, versio
_, err := os.Stat(cacheParentPath)
if err != nil {
if os.IsNotExist(err) {
return errors.New(errorCacheParentDoesNotExist)
return usererrors.New(errorCacheParentDoesNotExist)
}
return errors.Wrap(err, "Could not access parent path of cache directory.")
}
Expand All @@ -94,13 +95,13 @@ func (cacheDirectory *CacheDirectory) CheckOrCreateVersionFile(pull bool, versio
}
return nil
}
return errors.New(errorNotACacheOrEmpty)
return usererrors.New(errorNotACacheOrEmpty)
}

if cacheVersionFileExists {
return errors.New(errorCacheWrongVersion)
return usererrors.New(errorCacheWrongVersion)
}
return errors.New(errorPushNonCache)
return usererrors.New(errorPushNonCache)
}

func (cacheDirectory *CacheDirectory) Lock() error {
Expand All @@ -124,7 +125,7 @@ func (cacheDirectory *CacheDirectory) Unlock() error {
func (cacheDirectory *CacheDirectory) CheckLock() error {
_, err := os.Stat(cacheDirectory.lockFilePath())
if err == nil {
return errors.New("The cache directory is locked, likely due to a `pull` command being interrupted. Please run `pull` again to ensure all required data is downloaded.")
return usererrors.New(errorCacheLocked)
}
if os.IsNotExist(err) {
return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/cachedirectory/cachedirectory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestLocking(t *testing.T) {
require.NoError(t, cacheDirectory.CheckOrCreateVersionFile(true, aVersion))
require.NoError(t, cacheDirectory.Lock())
require.NoError(t, cacheDirectory.Lock())
require.Error(t, cacheDirectory.CheckLock())
require.EqualError(t, cacheDirectory.CheckLock(), errorCacheLocked)
require.NoError(t, cacheDirectory.Unlock())
require.NoError(t, cacheDirectory.CheckLock())
}
28 changes: 28 additions & 0 deletions internal/githubapiutil/githubapiutil.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package githubapiutil

import (
"strings"

"github.com/google/go-github/v32/github"
)

const xOAuthScopesHeader = "X-OAuth-Scopes"

func MissingAllScopes(response *github.Response, requiredAnyScopes ...string) bool {
if response == nil {
return false
}
if len(response.Header.Values(xOAuthScopesHeader)) == 0 {
return false
}
actualScopes := strings.Split(response.Header.Get(xOAuthScopesHeader), ",")
for _, actualScope := range actualScopes {
actualScope = strings.Trim(actualScope, " ")
for _, requiredAnyScope := range requiredAnyScopes {
if actualScope == requiredAnyScope {
return false
}
}
}
return true
}
25 changes: 25 additions & 0 deletions internal/githubapiutil/githubapiutil_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package githubapiutil

import (
"net/http"
"testing"

"github.com/stretchr/testify/require"

"github.com/google/go-github/v32/github"
)

func TestHasAnyScopes(t *testing.T) {
response := github.Response{
Response: &http.Response{Header: http.Header{}},
}

response.Header.Set(xOAuthScopesHeader, "gist, notifications, admin:org, repo")
require.False(t, MissingAllScopes(&response, "public_repo", "repo"))

response.Header.Set(xOAuthScopesHeader, "gist, notifications, public_repo, admin:org")
require.False(t, MissingAllScopes(&response, "public_repo", "repo"))

response.Header.Set(xOAuthScopesHeader, "gist, notifications, admin:org")
require.True(t, MissingAllScopes(&response, "public_repo", "repo"))
}
22 changes: 19 additions & 3 deletions internal/push/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package push
import (
"context"
"encoding/json"
usererrors "errors"
"fmt"
"io"
"io/ioutil"
Expand All @@ -13,6 +14,8 @@ import (
"path/filepath"
"strings"

"github.com/github/codeql-action-sync/internal/githubapiutil"

log "github.com/sirupsen/logrus"

"github.com/github/codeql-action-sync/internal/cachedirectory"
Expand All @@ -30,6 +33,7 @@ const remoteName = "enterprise"
const repositoryHomepage = "https://github.com/github/codeql-action-sync-tool/"

const errorAlreadyExists = "The destination repository already exists, but it was not created with the CodeQL Action sync tool. If you are sure you want to push the CodeQL Action to it, re-run this command with the `--force` flag."
const errorInvalidDestinationToken = "The destination token you've provided is not valid."

type pushService struct {
ctx context.Context
Expand All @@ -43,8 +47,11 @@ type pushService struct {

func (pushService *pushService) createRepository() (*github.Repository, error) {
log.Debug("Ensuring repository exists...")
user, _, err := pushService.githubEnterpriseClient.Users.Get(pushService.ctx, "")
user, response, err := pushService.githubEnterpriseClient.Users.Get(pushService.ctx, "")
if err != nil {
if response.StatusCode == http.StatusUnauthorized {
return nil, usererrors.New(errorInvalidDestinationToken)
}
return nil, errors.Wrap(err, "Error getting current user.")
}

Expand All @@ -66,6 +73,9 @@ func (pushService *pushService) createRepository() (*github.Repository, error) {
Name: github.String(pushService.destinationRepositoryOwner),
}, user.GetLogin())
if err != nil {
if response.StatusCode == http.StatusNotFound && githubapiutil.MissingAllScopes(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.")
}
}
Expand All @@ -90,13 +100,19 @@ func (pushService *pushService) createRepository() (*github.Repository, error) {
Private: github.Bool(false),
}
if response.StatusCode == http.StatusNotFound {
repository, _, err = pushService.githubEnterpriseClient.Repositories.Create(pushService.ctx, destinationOrganization, &desiredRepositoryProperties)
repository, response, err = pushService.githubEnterpriseClient.Repositories.Create(pushService.ctx, destinationOrganization, &desiredRepositoryProperties)
if err != nil {
if response.StatusCode == http.StatusNotFound && githubapiutil.MissingAllScopes(response, "public_repo", "repo") {
return nil, usererrors.New("The destination token you have provided does not have the `public_repo` scope.")
}
return nil, errors.Wrap(err, "Error creating destination repository.")
}
} else {
repository, _, err = pushService.githubEnterpriseClient.Repositories.Edit(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, &desiredRepositoryProperties)
repository, response, err = pushService.githubEnterpriseClient.Repositories.Edit(pushService.ctx, pushService.destinationRepositoryOwner, pushService.destinationRepositoryName, &desiredRepositoryProperties)
if err != nil {
if response.StatusCode == http.StatusNotFound && githubapiutil.MissingAllScopes(response, "public_repo", "repo") {
return nil, usererrors.New("The destination token you have provided does not have the `public_repo` scope.")
}
return nil, errors.Wrap(err, "Error updating destination repository.")
}
}
Expand Down