Skip to content

Commit 8da7988

Browse files
authored
Package: Validate package types (#436)
This PR adds package type check/validation. It fixes grafana/data-sources#386 error that is return by github api that happens if the package type is random non-supported string. And also it is also improvement for package types that are valid, but not supported by graphql and return errors (more in #366.) So instead of sending package type we know will return error, we return error early with helpful message. [Ok DOCKER package](http://localhost:3000/explore?schemaVersion=1&panes=%7B%22g9y%22:%7B%22datasource%22:%22ddyrfpv6iitj4b%22,%22queries%22:%5B%7B%22refId%22:%22A%22,%22datasource%22:%7B%22type%22:%22grafana-github-datasource%22,%22uid%22:%22ddyrfpv6iitj4b%22%7D,%22queryType%22:%22Packages%22,%22options%22:%7B%22packageType%22:%22DOCKER%22%7D,%22repository%22:%22grafana%22,%22owner%22:%22grafana%22%7D%5D,%22range%22:%7B%22from%22:%22now-1h%22,%22to%22:%22now%22%7D%7D%7D&orgId=1) <img width="1504" alt="image" src="https://github.com/user-attachments/assets/e6bce93d-9d61-4a88-8fd9-84f114fa6cfe" /> [Not supported NPM package](http://localhost:3000/explore?schemaVersion=1&panes=%7B%22g9y%22:%7B%22datasource%22:%22ddyrfpv6iitj4b%22,%22queries%22:%5B%7B%22refId%22:%22A%22,%22datasource%22:%7B%22type%22:%22grafana-github-datasource%22,%22uid%22:%22ddyrfpv6iitj4b%22%7D,%22queryType%22:%22Packages%22,%22options%22:%7B%22packageType%22:%22NPM%22%7D,%22repository%22:%22grafana%22,%22owner%22:%22grafana%22%7D%5D,%22range%22:%7B%22from%22:%22now-1h%22,%22to%22:%22now%22%7D%7D%7D&orgId=1) <img width="1510" alt="image" src="https://github.com/user-attachments/assets/cef5fe39-d7e8-4ba0-aca6-1b80caa87e4b" /> [Invalid "abc" package](http://localhost:3000/explore?schemaVersion=1&panes=%7B%22g9y%22%3A%7B%22datasource%22%3A%22ddyrfpv6iitj4b%22%2C%22queries%22%3A%5B%7B%22refId%22%3A%22A%22%2C%22datasource%22%3A%7B%22type%22%3A%22grafana-github-datasource%22%2C%22uid%22%3A%22ddyrfpv6iitj4b%22%7D%2C%22queryType%22%3A%22Packages%22%2C%22options%22%3A%7B%22packageType%22%3A%22abc%22%7D%2C%22repository%22%3A%22grafana%22%2C%22owner%22%3A%22grafana%22%7D%5D%2C%22range%22%3A%7B%22from%22%3A%22now-1h%22%2C%22to%22%3A%22now%22%7D%7D%7D&orgId=1) <img width="1502" alt="image" src="https://github.com/user-attachments/assets/07e1eeb5-46dd-47c8-a971-75c08865ad15" />
1 parent 37372c6 commit 8da7988

File tree

5 files changed

+133
-5
lines changed

5 files changed

+133
-5
lines changed

.changeset/stupid-teams-trade.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'grafana-github-datasource': patch
3+
---
4+
5+
chore: Add validation for package types

pkg/github/client/errorsourcehandling.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ var (
2020
"Resource not accessible by personal access token",
2121
"API rate limit exceeded",
2222
"Resource not accessible by integration", // issue with incorrectly set permissions for token/app
23-
"registry is not supported by GraphQL APIs",
2423
}
2524
)
2625

pkg/github/datasource.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,10 @@ func (d *Datasource) HandleMilestonesQuery(ctx context.Context, query *models.Mi
117117

118118
// HandlePackagesQuery is the query handler for listing GitHub Packages
119119
func (d *Datasource) HandlePackagesQuery(ctx context.Context, query *models.PackagesQuery, req backend.DataQuery) (dfutil.Framer, error) {
120-
opt := models.PackagesOptionsWithRepo(query.Options, query.Owner, query.Repository)
120+
opt, err := models.PackagesOptionsWithRepo(query.Options, query.Owner, query.Repository)
121+
if err != nil {
122+
return nil, err
123+
}
121124

122125
return GetAllPackages(ctx, d.client, opt)
123126
}

pkg/models/packages.go

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
package models
22

3-
import "github.com/shurcooL/githubv4"
3+
import (
4+
"fmt"
5+
"slices"
6+
7+
"github.com/grafana/grafana-plugin-sdk-go/backend"
8+
"github.com/shurcooL/githubv4"
9+
)
410

511
// ListPackagesOptions provides options when retrieving commits
612
type ListPackagesOptions struct {
@@ -11,11 +17,43 @@ type ListPackagesOptions struct {
1117
}
1218

1319
// PackagesOptionsWithRepo adds Owner and Repo to a ListPackagesOptions. This is just for convenience
14-
func PackagesOptionsWithRepo(opt ListPackagesOptions, owner string, repo string) ListPackagesOptions {
20+
func PackagesOptionsWithRepo(opt ListPackagesOptions, owner string, repo string) (ListPackagesOptions, error) {
21+
err := validatePackageType(opt.PackageType)
22+
if err != nil {
23+
return ListPackagesOptions{}, err
24+
}
25+
1526
return ListPackagesOptions{
1627
Owner: owner,
1728
Repository: repo,
1829
Names: opt.Names,
1930
PackageType: opt.PackageType,
20-
}
31+
}, nil
32+
}
33+
34+
// validPackageTypes is a list of valid package types that are supported by the GitHub graphql API that we are using
35+
var validPackageTypes = []githubv4.PackageType{
36+
githubv4.PackageTypeMaven,
37+
githubv4.PackageTypeDocker,
38+
githubv4.PackageTypeDebian,
39+
githubv4.PackageTypePypi,
2140
}
41+
42+
// notSupportedPackageTypes is a list of package types that are not supported by the GitHub graphql API
43+
// They were supported in the past but are not supported anymore and we want to return an error if they are used
44+
var notSupportedPackageTypes = []githubv4.PackageType{
45+
githubv4.PackageTypeNpm,
46+
githubv4.PackageTypeRubygems,
47+
githubv4.PackageTypeNuget,
48+
}
49+
50+
func validatePackageType(packageType githubv4.PackageType) error {
51+
if slices.Contains(validPackageTypes, packageType) {
52+
return nil
53+
}
54+
55+
if slices.Contains(notSupportedPackageTypes, packageType) {
56+
return backend.DownstreamError(fmt.Errorf("package type %q is not supported. Valid types are: MAVEN, DOCKER, DEBIAN, PYPI", packageType))
57+
}
58+
return backend.DownstreamError(fmt.Errorf("invalid package type %q. Valid types are: MAVEN, DOCKER, DEBIAN, PYPI", packageType))
59+
}

pkg/models/packages_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package models
2+
3+
import (
4+
"testing"
5+
6+
"github.com/shurcooL/githubv4"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func Test_validatePackageType(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
packageType githubv4.PackageType
14+
wantErr bool
15+
errMessage string
16+
}{
17+
// Valid package types
18+
{
19+
name: "valid PYPI package type",
20+
packageType: githubv4.PackageTypePypi,
21+
wantErr: false,
22+
},
23+
{
24+
name: "valid Docker package type",
25+
packageType: githubv4.PackageTypeDocker,
26+
wantErr: false,
27+
},
28+
{
29+
name: "valid Maven package type",
30+
packageType: githubv4.PackageTypeMaven,
31+
wantErr: false,
32+
},
33+
{
34+
name: "valid Debian package type",
35+
packageType: githubv4.PackageTypeDebian,
36+
wantErr: false,
37+
},
38+
// Not supported package types by GraphQL API anymore
39+
{
40+
name: "not supported NPM package type",
41+
packageType: githubv4.PackageTypeNpm,
42+
wantErr: true,
43+
errMessage: `package type "NPM" is not supported`,
44+
},
45+
{
46+
name: "not supported Rubygems package type",
47+
packageType: githubv4.PackageTypeRubygems,
48+
wantErr: true,
49+
errMessage: `package type "RUBYGEMS" is not supported`,
50+
},
51+
{
52+
name: "not supported Nuget package type",
53+
packageType: githubv4.PackageTypeNuget,
54+
wantErr: true,
55+
errMessage: `package type "NUGET" is not supported`,
56+
},
57+
// Invalid package types
58+
{
59+
name: "invalid package type",
60+
packageType: "INVALID",
61+
wantErr: true,
62+
errMessage: `invalid package type "INVALID"`,
63+
},
64+
{
65+
name: "empty package type",
66+
packageType: "",
67+
wantErr: true,
68+
errMessage: `invalid package type ""`,
69+
},
70+
}
71+
72+
for _, tt := range tests {
73+
t.Run(tt.name, func(t *testing.T) {
74+
err := validatePackageType(tt.packageType)
75+
if tt.wantErr {
76+
require.Error(t, err)
77+
require.ErrorContains(t, err, tt.errMessage)
78+
} else {
79+
require.NoError(t, err)
80+
}
81+
})
82+
}
83+
}

0 commit comments

Comments
 (0)