fix: Show Errors from Github (#4753)

* fix: show errors from github

Signed-off-by: Alexander Cairns <alexandercairns@discoverygarden.ca>

* fix do not log entire response

Signed-off-by: Alexander Cairns <alexandercairns@discoverygarden.ca>

---------

Signed-off-by: Alexander Cairns <alexandercairns@discoverygarden.ca>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

pkg/generator/github/github.go

@@ -129,6 +129,13 @@ func (g *Generator) generate(
 		return nil, nil, fmt.Errorf("error decoding response: %w", err)
 	}
 
+	if resp.StatusCode >= 300 {
+		if message, ok := gat["message"]; ok {
+			return nil, nil, fmt.Errorf("error generating token: response code: %d, response: %v", resp.StatusCode, message)
+		}
+		return nil, nil, fmt.Errorf("error generating token, failed to extract error message from github request: response code: %d", resp.StatusCode)
+	}
+
 	accessToken, ok := gat["token"].(string)
 	if !ok {
 		return nil, nil, errors.New("token isn't a string or token key doesn't exist")

pkg/generator/github/github_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/core/v1"
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -35,7 +36,7 @@ const (
 	tstCrtName = "github_test.pem"
 )
 
-func testHTTPSrv(t *testing.T, r []byte) *httptest.Server {
+func testHTTPSrv(t *testing.T, r []byte, s int) *httptest.Server {
 	return httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		assert.Equal(t, "POST", req.Method, "Expected POST request")
 		assert.NotEmpty(t, req.Body)
@@ -43,6 +44,7 @@ func testHTTPSrv(t *testing.T, r []byte) *httptest.Server {
 		assert.Equal(t, "application/vnd.github.v3+json", req.Header.Get("Accept"))
 
 		// Send response to be tested
+		rw.WriteHeader(s)
 		rw.Write(r)
 	}))
 }
@@ -70,21 +72,32 @@ func TestGenerate(t *testing.T) {
 		"repository_selection": "selected"
 	  }`)
 
-	server := testHTTPSrv(t, validResponce)
+	invalidResponce := []byte(`{
+		"documentation_url": "https://docs.github.com/rest/reference/apps#create-an-installation-access-token-for-an-app",
+		"message": "There is at least one repository that does not exist or is not accessible to the parent installation.",
+		"status": 422
+	  }`)
+
+	server := testHTTPSrv(t, validResponce, http.StatusCreated)
+	badServer := testHTTPSrv(t, invalidResponce, 422)
 
 	tests := []struct {
-		name    string
-		g       *Generator
-		args    args
-		want    map[string][]byte
-		wantErr bool
+		name      string
+		g         *Generator
+		args      args
+		want      map[string][]byte
+		assertErr func(t *testing.T, err error)
+		server    *httptest.Server
 	}{
 		{
 			name: "nil spec",
 			args: args{
 				jsonSpec: nil,
 			},
-			wantErr: true,
+			assertErr: func(t *testing.T, err error) {
+				require.Error(t, err)
+			},
+			server: server,
 		},
 		{
 			name: "full spec",
@@ -122,22 +135,62 @@ spec:
 			want: map[string][]byte{
 				"token": []byte("ghs_16C7e42F292c6912E7710c838347Ae178B4a"),
 			},
+			assertErr: func(t *testing.T, err error) {
+				require.NoError(t, err)
+			},
+			server: server,
+		},
+		{
+			name: "fail on bad request",
+			args: args{
+				ctx:       context.TODO(),
+				namespace: "foo",
+				kube: clientfake.NewClientBuilder().WithObjects(&v1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "testName",
+						Namespace: "foo",
+					},
+					Data: map[string][]byte{
+						"privateKey": pem,
+					},
+				}).Build(),
+				jsonSpec: &apiextensions.JSON{
+					Raw: []byte(fmt.Sprintf(`apiVersion: generators.external-secrets.io/v1alpha1
+kind: GithubToken
+spec:
+  appID: "0000000"
+  installID: "00000000"
+  URL: %q
+  repositories:
+  - "octocat/Hello-World"
+  permissions:
+    contents: "read"
+  auth:
+    privateKey:
+      secretRef:
+        name: "testName"
+        namespace: "foo"
+        key: "privateKey"`, badServer.URL)),
+				},
+			},
+			assertErr: func(t *testing.T, err error) {
+				assert.ErrorContains(t, err, "error generating token")
+			},
+			server: badServer,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			g := &Generator{httpClient: server.Client()}
+			g := &Generator{httpClient: tt.server.Client()}
 			got, _, err := g.generate(
 				tt.args.ctx,
 				tt.args.jsonSpec,
 				tt.args.kube,
 				tt.args.namespace,
 			)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("Generator.Generate() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
+
+			tt.assertErr(t, err)
 			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("Generator.Generate() = %s, want %s", got, tt.want)
 			}