From 4dc202534fe162e53ec98591c5354dd4ad3258ad Mon Sep 17 00:00:00 2001 From: David Collom Date: Tue, 27 Aug 2024 17:08:53 +0100 Subject: [PATCH] Adding Logic and Tests for Authorization token to only be used if its provided (#227) --- go.mod | 7 +- go.sum | 8 +- pkg/client/client_test.go | 3 + pkg/client/ghcr/ghcr.go | 2 +- pkg/client/ghcr/ghcr_test.go | 167 +++++++++++++++++++++++++++++++++++ pkg/client/ghcr/path.go | 5 ++ pkg/client/ghcr/path_test.go | 24 ++++- 7 files changed, 209 insertions(+), 7 deletions(-) create mode 100644 pkg/client/ghcr/ghcr_test.go diff --git a/go.mod b/go.mod index eb1c8ad..34bfc65 100644 --- a/go.mod +++ b/go.mod @@ -6,8 +6,6 @@ go 1.22.6 // please place any replace statements here at the top for visibility and add a // comment to it as to when it can be removed -replace github.com/imdario/mergo => github.com/imdario/mergo v0.3.16 - require ( github.com/Azure/go-autorest/autorest v0.11.29 github.com/Azure/go-autorest/autorest/adal v0.9.24 @@ -33,7 +31,8 @@ require ( github.com/aws/aws-sdk-go-v2/service/ecr v1.32.2 github.com/gofri/go-github-ratelimit v1.1.0 github.com/google/go-containerregistry v0.20.2 - github.com/google/go-github/v58 v58.0.0 + github.com/google/go-github/v62 v62.0.0 + github.com/jarcoal/httpmock v1.3.1 github.com/stretchr/testify v1.9.0 ) @@ -127,3 +126,5 @@ require ( sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect ) + +replace github.com/imdario/mergo => github.com/imdario/mergo v0.3.16 diff --git a/go.sum b/go.sum index c4b2f44..79f6514 100644 --- a/go.sum +++ b/go.sum @@ -104,8 +104,8 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-containerregistry v0.20.2 h1:B1wPJ1SN/S7pB+ZAimcciVD+r+yV/l/DSArMxlbwseo= github.com/google/go-containerregistry v0.20.2/go.mod h1:z38EKdKh4h7IP2gSfUUqEvalZBqs6AoLeWfUy34nQC8= -github.com/google/go-github/v58 v58.0.0 h1:Una7GGERlF/37XfkPwpzYJe0Vp4dt2k1kCjlxwjIvzw= -github.com/google/go-github/v58 v58.0.0/go.mod h1:k4hxDKEfoWpSqFlc8LTpGd9fu2KrV1YAa6Hi6FmDNY4= +github.com/google/go-github/v62 v62.0.0 h1:/6mGCaRywZz9MuHyw9gD1CwsbmBX8GWsbFkwMmHdhl4= +github.com/google/go-github/v62 v62.0.0/go.mod h1:EMxeUqGJq2xRu9DYBMwel/mr7kZrzUOfQmmpYrZn2a4= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -129,6 +129,8 @@ github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4= github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/jarcoal/httpmock v1.3.1 h1:iUx3whfZWVf3jT01hQTO/Eo5sAYtB2/rqaUuOtpInww= +github.com/jarcoal/httpmock v1.3.1/go.mod h1:3yb8rc4BI7TCBhFY8ng0gjuLKJNquuDNiPaZjnENuYg= github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg= github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo= github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8= @@ -155,6 +157,8 @@ github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxec github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/maxatome/go-testdeep v1.12.0 h1:Ql7Go8Tg0C1D/uMMX59LAoYK7LffeJQ6X2T04nTH68g= +github.com/maxatome/go-testdeep v1.12.0/go.mod h1:lPZc/HAcJMP92l7yI6TRz1aZN5URwUBUAfUNvrclaNM= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0= diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 4e6c9ba..1de2042 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -24,6 +24,9 @@ func TestFromImageURL(t *testing.T) { Host: "https://docker.repositories.yourdomain.com", }, }, + GHCR: ghcr.Options{ + Token: "test-token", + }, }) if err != nil { t.Fatal(err) diff --git a/pkg/client/ghcr/ghcr.go b/pkg/client/ghcr/ghcr.go index 169a12f..03e7f6b 100644 --- a/pkg/client/ghcr/ghcr.go +++ b/pkg/client/ghcr/ghcr.go @@ -7,7 +7,7 @@ import ( "strings" "github.com/gofri/go-github-ratelimit/github_ratelimit" - "github.com/google/go-github/v58/github" + "github.com/google/go-github/v62/github" "github.com/jetstack/version-checker/pkg/api" ) diff --git a/pkg/client/ghcr/ghcr_test.go b/pkg/client/ghcr/ghcr_test.go new file mode 100644 index 0000000..c8c687a --- /dev/null +++ b/pkg/client/ghcr/ghcr_test.go @@ -0,0 +1,167 @@ +package ghcr + +import ( + "context" + "net/http" + "testing" + + "github.com/google/go-github/v62/github" + "github.com/jarcoal/httpmock" + "github.com/stretchr/testify/assert" +) + +func setup() { + httpmock.Activate() +} + +func teardown() { + httpmock.DeactivateAndReset() +} + +func registerCommonResponders() { + httpmock.RegisterResponder("GET", "https://api.github.com/users/test-user-owner", + func(req *http.Request) (*http.Response, error) { + return httpmock.NewStringResponse(200, `{"type":"User"}`), nil + }) + httpmock.RegisterResponder("GET", "https://api.github.com/users/test-org-owner", + func(req *http.Request) (*http.Response, error) { + return httpmock.NewStringResponse(200, `{"type":"Organization"}`), nil + }) +} + +func registerTagResponders() { + httpmock.RegisterResponder("GET", "https://api.github.com/users/test-user-owner/packages/container/test-repo/versions", + func(req *http.Request) (*http.Response, error) { + return httpmock.NewStringResponse(200, `[ + { + "name": "sha123", + "metadata": { + "container": { + "tags": ["tag1", "tag2"] + } + }, + "created_at": "2023-07-08T12:34:56Z" + } + ]`), nil + }) + httpmock.RegisterResponder("GET", "https://api.github.com/orgs/test-org-owner/packages/container/test-repo/versions", + func(req *http.Request) (*http.Response, error) { + return httpmock.NewStringResponse(200, `[ + { + "name": "sha123", + "metadata": { + "container": { + "tags": ["tag1", "tag2"] + } + }, + "created_at": "2023-07-08T12:34:56Z" + } + ]`), nil + }) +} + +func TestClient_Tags(t *testing.T) { + setup() + defer teardown() + + ctx := context.Background() + host := "ghcr.io" + + t.Run("successful tags fetch", func(t *testing.T) { + httpmock.Reset() + registerCommonResponders() + registerTagResponders() + + client := New(Options{}) + client.client = github.NewClient(nil) // Use the default HTTP client + + tags, err := client.Tags(ctx, host, "test-user-owner", "test-repo") + assert.NoError(t, err) + assert.Len(t, tags, 2) + assert.Equal(t, "tag1", tags[0].Tag) + assert.Equal(t, "tag2", tags[1].Tag) + }) + + t.Run("failed to fetch owner type", func(t *testing.T) { + httpmock.Reset() + httpmock.RegisterResponder("GET", "https://api.github.com/users/test-user-owner", + func(req *http.Request) (*http.Response, error) { + return httpmock.NewStringResponse(404, `{"message": "Not Found"}`), nil + }) + + client := New(Options{}) + client.client = github.NewClient(nil) // Use the default HTTP client + + _, err := client.Tags(ctx, host, "test-user-owner", "test-repo") + assert.Error(t, err) + }) + + t.Run("token not set, no authorization header", func(t *testing.T) { + httpmock.Reset() + httpmock.RegisterResponder("GET", "https://api.github.com/users/test-user-owner", + func(req *http.Request) (*http.Response, error) { + if req.Header.Get("Authorization") != "" { + t.Errorf("expected no Authorization header, got %s", req.Header.Get("Authorization")) + } + return httpmock.NewStringResponse(200, `{"type":"User"}`), nil + }) + registerTagResponders() + + client := New(Options{}) // No token provided + client.client = github.NewClient(nil) + + _, err := client.Tags(ctx, host, "test-user-owner", "test-repo") + assert.NoError(t, err) + }) + + t.Run("token set, authorization header sent", func(t *testing.T) { + token := "test-token" + httpmock.Reset() + httpmock.RegisterResponder("GET", "https://api.github.com/users/test-user-owner", + func(req *http.Request) (*http.Response, error) { + authHeader := req.Header.Get("Authorization") + expectedAuthHeader := "Bearer " + token + if authHeader != expectedAuthHeader { + t.Errorf("expected Authorization header %s, got %s", expectedAuthHeader, authHeader) + } + return httpmock.NewStringResponse(200, `{"type":"User"}`), nil + }) + + registerTagResponders() + + client := New(Options{Token: token}) + + _, err := client.Tags(ctx, host, "test-user-owner", "test-repo") + assert.NoError(t, err) + }) + + t.Run("ownerType returns user", func(t *testing.T) { + httpmock.Reset() + registerCommonResponders() + registerTagResponders() + + client := New(Options{}) + client.client = github.NewClient(nil) // Use the default HTTP client + + tags, err := client.Tags(ctx, host, "test-user-owner", "test-repo") + assert.NoError(t, err) + assert.Len(t, tags, 2) + assert.Equal(t, "tag1", tags[0].Tag) + assert.Equal(t, "tag2", tags[1].Tag) + }) + + t.Run("ownerType returns org", func(t *testing.T) { + httpmock.Reset() + registerCommonResponders() + registerTagResponders() + + client := New(Options{}) + client.client = github.NewClient(nil) // Use the default HTTP client + + tags, err := client.Tags(ctx, host, "test-org-owner", "test-repo") + assert.NoError(t, err) + assert.Len(t, tags, 2) + assert.Equal(t, "tag1", tags[0].Tag) + assert.Equal(t, "tag2", tags[1].Tag) + }) +} diff --git a/pkg/client/ghcr/path.go b/pkg/client/ghcr/path.go index 60dbd56..c65d7be 100644 --- a/pkg/client/ghcr/path.go +++ b/pkg/client/ghcr/path.go @@ -5,6 +5,11 @@ import ( ) func (c *Client) IsHost(host string) bool { + // Package API requires Authentication + // This forces the Client to use the fallback method + if c.opts.Token == "" { + return false + } return host == "ghcr.io" } diff --git a/pkg/client/ghcr/path_test.go b/pkg/client/ghcr/path_test.go index 8be5df7..56f1bda 100644 --- a/pkg/client/ghcr/path_test.go +++ b/pkg/client/ghcr/path_test.go @@ -4,34 +4,52 @@ import "testing" func TestIsHost(t *testing.T) { tests := map[string]struct { + token string host string expIs bool }{ - "an empty host should be false": { + "an empty token should be false": { + token: "test-token", + host: "", + expIs: false, + }, + "an empty host and token should be false": { + token: "", + host: "", + expIs: false, + }, + "an empty host should be false": { + token: "test-token", host: "", expIs: false, }, "random string should be false": { + token: "test-token", host: "foobar", expIs: false, }, "random string with dots should be false": { + token: "test-token", host: "foobar.foo", expIs: false, }, "just ghcr.io should be true": { + token: "test-token", host: "ghcr.io", expIs: true, }, "gcr.io with random sub domains should be false": { + token: "test-token", host: "ghcr.gcr.io", expIs: false, }, "foodghcr.io should be false": { + token: "test-token", host: "foodghcr.io", expIs: false, }, "ghcr.iofoo should be false": { + token: "test-token", host: "ghcr.iofoo", expIs: false, }, @@ -40,6 +58,9 @@ func TestIsHost(t *testing.T) { handler := new(Client) for name, test := range tests { t.Run(name, func(t *testing.T) { + if test.token != "" { + handler.opts.Token = test.token + } if isHost := handler.IsHost(test.host); isHost != test.expIs { t.Errorf("%s: unexpected IsHost, exp=%t got=%t", test.host, test.expIs, isHost) @@ -78,6 +99,7 @@ func TestRepoImage(t *testing.T) { handler := new(Client) for name, test := range tests { t.Run(name, func(t *testing.T) { + handler.opts.Token = "fake-token" repo, image := handler.RepoImageFromPath(test.path) if repo != test.expRepo && image != test.expImage { t.Errorf("%s: unexpected repo/image, exp=%s/%s got=%s/%s",