Allow spyglass to use cookie auth

prow/cmd/deck/main.go

@@ -49,8 +49,6 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
-	"k8s.io/test-infra/prow/interrupts"
-	"k8s.io/test-infra/prow/simplifypath"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/yaml"
@@ -64,6 +62,7 @@ import (
 	"k8s.io/test-infra/prow/git/v2"
 	prowgithub "k8s.io/test-infra/prow/github"
 	"k8s.io/test-infra/prow/githuboauth"
+	"k8s.io/test-infra/prow/interrupts"
 	"k8s.io/test-infra/prow/kube"
 	"k8s.io/test-infra/prow/logrusutil"
 	"k8s.io/test-infra/prow/metrics"
@@ -72,6 +71,7 @@ import (
 	"k8s.io/test-infra/prow/plugins"
 	"k8s.io/test-infra/prow/plugins/trigger"
 	"k8s.io/test-infra/prow/prstatus"
+	"k8s.io/test-infra/prow/simplifypath"
 	"k8s.io/test-infra/prow/spyglass"
 
 	// Import standard spyglass viewers
@@ -117,6 +117,7 @@ type options struct {
 	spyglass              bool
 	spyglassFilesLocation string
 	gcsCredentialsFile    string
+	gcsCookieAuth         bool
 	rerunCreatesJob       bool
 	allowInsecure         bool
 	dryRun                bool
@@ -181,6 +182,7 @@ func gatherOptions(fs *flag.FlagSet, args ...string) options {
 	fs.StringVar(&o.staticFilesLocation, "static-files-location", "/static", "Path to the static files")
 	fs.StringVar(&o.templateFilesLocation, "template-files-location", "/template", "Path to the template files")
 	fs.StringVar(&o.gcsCredentialsFile, "gcs-credentials-file", "", "Path to the GCS credentials file")
+	fs.BoolVar(&o.gcsCookieAuth, "gcs-cookie-auth", false, "Use storage.cloud.google.com instead of signed URLs")
 	fs.BoolVar(&o.rerunCreatesJob, "rerun-creates-job", false, "Change the re-run option in Deck to actually create the job. **WARNING:** Only use this with non-public deck instances, otherwise strangers can DOS your Prow instance")
 	fs.BoolVar(&o.allowInsecure, "allow-insecure", false, "Allows insecure requests for CSRF and GitHub oauth.")
 	fs.BoolVar(&o.dryRun, "dry-run", false, "Whether or not to make mutating API calls to GitHub.")
@@ -673,17 +675,16 @@ func prodOnlyMain(cfg config.Getter, pluginAgent *plugins.ConfigAgent, o options
 }
 
 func initSpyglass(cfg config.Getter, o options, mux *http.ServeMux, ja *jobs.JobAgent, gitHubClient deckGitHubClient, gitClient git.ClientFactory) {
-	var c *storage.Client
-	var err error
-	if o.gcsCredentialsFile == "" {
-		c, err = storage.NewClient(context.Background(), option.WithoutAuthentication())
-	} else {
-		c, err = storage.NewClient(context.Background(), option.WithCredentialsFile(o.gcsCredentialsFile))
+	ctx := context.TODO()
+	var options []option.ClientOption
+	if creds := o.gcsCredentialsFile; creds != "" {
+		options = append(options, option.WithCredentialsFile(creds))
 	}
+	c, err := storage.NewClient(ctx, options...)
 	if err != nil {
 		logrus.WithError(err).Fatal("Error getting GCS client")
 	}
-	sg := spyglass.New(ja, cfg, c, o.gcsCredentialsFile, context.Background())
+	sg := spyglass.New(ctx, ja, cfg, c, o.gcsCredentialsFile, o.gcsCookieAuth)
 	sg.Start()
 
 	mux.Handle("/spyglass/static/", http.StripPrefix("/spyglass/static", staticHandlerFromDir(o.spyglassFilesLocation)))

prow/spyglass/gcsartifact_fetcher.go

@@ -49,8 +49,9 @@ var (
 
 // GCSArtifactFetcher contains information used for fetching artifacts from GCS
 type GCSArtifactFetcher struct {
-	client       *storage.Client
-	gcsCredsFile string
+	client        *storage.Client
+	gcsCredsFile  string
+	useCookieAuth bool
 }
 
 // gcsJobSource is a location in GCS where Prow job-specific artifacts are stored. This implementation assumes
@@ -66,10 +67,11 @@ type gcsJobSource struct {
 }
 
 // NewGCSArtifactFetcher creates a new ArtifactFetcher with a real GCS Client
-func NewGCSArtifactFetcher(c *storage.Client, gcsCredsFile string) *GCSArtifactFetcher {
+func NewGCSArtifactFetcher(c *storage.Client, gcsCredsFile string, useCookieAuth bool) *GCSArtifactFetcher {
 	return &GCSArtifactFetcher{
-		client:       c,
-		gcsCredsFile: gcsCredsFile,
+		client:        c,
+		gcsCredsFile:  gcsCredsFile,
+		useCookieAuth: useCookieAuth,
 	}
 }
 
@@ -145,17 +147,34 @@ func (af *GCSArtifactFetcher) artifacts(key string) ([]string, error) {
 	return artifacts, nil
 }
 
+const (
+	anonHost   = "storage.googleapis.com"
+	cookieHost = "storage.cloud.google.com"
+)
+
 func (af *GCSArtifactFetcher) signURL(bucket, obj string) (string, error) {
+	// We specifically want to use cookie auth, see:
+	// https://cloud.google.com/storage/docs/access-control/cookie-based-authentication
+	if af.useCookieAuth {
+		artifactLink := &url.URL{
+			Scheme: httpsScheme,
+			Host:   cookieHost,
+			Path:   path.Join(bucket, obj),
+		}
+		return artifactLink.String(), nil
+	}
+
 	// If we're anonymous we can just return a plain URL.
 	if af.gcsCredsFile == "" {
 		artifactLink := &url.URL{
 			Scheme: httpsScheme,
-			Host:   "storage.googleapis.com",
+			Host:   anonHost,
 			Path:   path.Join(bucket, obj),
 		}
 		return artifactLink.String(), nil
 	}
 
+	// TODO(fejta): do not require the json file https://github.com/kubernetes/test-infra/issues/16489
 	// As far as I can tell, there is no sane way to get these values other than just
 	// reading them out of the JSON file ourselves.
 	f, err := os.Open(af.gcsCredsFile)

prow/spyglass/gcsartifact_fetcher_test.go

@@ -17,6 +17,11 @@ limitations under the License.
 package spyglass
 
 import (
+	"encoding/base64"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"strings"
 	"testing"
 )
 
@@ -80,7 +85,7 @@ func TestNewGCSJobSource(t *testing.T) {
 // Tests listing objects associated with the current job in GCS
 func TestArtifacts_ListGCS(t *testing.T) {
 	fakeGCSClient := fakeGCSServer.Client()
-	testAf := NewGCSArtifactFetcher(fakeGCSClient, "")
+	testAf := NewGCSArtifactFetcher(fakeGCSClient, "", false)
 	testCases := []struct {
 		name              string
 		handle            artifactHandle
@@ -132,7 +137,7 @@ func TestArtifacts_ListGCS(t *testing.T) {
 // Tests getting handles to objects associated with the current job in GCS
 func TestFetchArtifacts_GCS(t *testing.T) {
 	fakeGCSClient := fakeGCSServer.Client()
-	testAf := NewGCSArtifactFetcher(fakeGCSClient, "")
+	testAf := NewGCSArtifactFetcher(fakeGCSClient, "", false)
 	maxSize := int64(500e6)
 	testCases := []struct {
 		name         string
@@ -173,3 +178,137 @@ func TestFetchArtifacts_GCS(t *testing.T) {
 		}
 	}
 }
+
+func TestSignURL(t *testing.T) {
+	// This fake key is revoked and thus worthless but still make its contents less obvious
+	fakeKeyBuf, err := base64.StdEncoding.DecodeString(`
+LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tXG5NSUlFdlFJQkFEQU5CZ2txaGtpRzl3MEJBUUVG
+QUFTQ0JLY3dnZ1NqQWdFQUFvSUJBUUN4MEF2aW1yMjcwZDdaXG5pamw3b1FRUW1oZTFOb3dpeWMy
+UStuQW95aFE1YkQvUW1jb01zcWg2YldneVI0UU90aXVBbHM2VWhJenF4Q25pXG5PazRmbWJqVnhp
+STl1Ri9EVTV6ZE5wM0dkQWFiUlVPNW5yWkpMelN0VXhudFBEcjZvK281RHM5YWJJWkNYYUVTXG5o
+UWxOdTBrUm5HbHZGUHNkV1JYMmtSN01Yb3pkcXczcHZZRXZyaGlhRStYZnRhUzhKdmZEc0NPT2RQ
+OWp5TzNTXG5aR2lkaU5hRmhYK2xnZEcrdHdqOUE3UDFlb1NMbTZCdXVhcjRDOGhlOEVkVGVEbXVk
+a1BPeWwvb2tHWU5tSzJkXG5yUkQ0WHBhcy93VGxsTXBLRUZxWllZeVdkRnJvVWQwMFVhQnhHV0cz
+UlZ2TWZoRk80QUhrSkNwZlE1U00rSElmXG5VN2lkRjAyYkFnTUJBQUVDZ2dFQURIaVhoTTZ1bFFB
+OHZZdzB5T2Q3cGdCd3ZqeHpxckwxc0gvb0l1dzlhK09jXG5QREMxRzV2aU5pZjdRVitEc3haeXlh
+T0tISitKVktQcWZodnh3OFNmMHBxQlowdkpwNlR6SVE3R0ZSZXBLUFc4XG5NTVloYWRPZVFiUE00
+emN3dWNpS1VuTW45dU1hcllmc2xxUnZDUjBrSEZDWWtucHB2RjYxckNQMGdZZjJJRXZUXG5qNVlV
+QWFrNDlVRDQyaUdEZnh2OGUzMGlMTmRRWE1iMHE3V2dyRGdxL0ttUHM2Q2dOaGRzME1uSlRFbUE5
+YlFtXG52MHV0K2hUYWpXalcxVWNyUTBnM2JjNng1VWN2V1VjK1ZndUllVmxVcEgvM2dJNXVYZkxn
+bTVQNThNa0s4UlhTXG5YYW92Rk05VkNNRFhTK25PWk1uSXoyNVd5QmhkNmdpVWs5UkJhc05Tb1FL
+QmdRRGFxUXpyYWJUZEZNY1hwVlNnXG41TUpuNEcvSFVPWUxveVM5cE9UZi9qbFN1ZUYrNkt6RGJV
+N1F6TC9wT1JtYjJldVdxdmpmZDVBaU1oUnY2Snk1XG41ZVNpa3dYRDZJeS9sZGh3QUdtMUZrZ1ZX
+TXJ3ZHlqYjJpV2I2Um4rNXRBYjgwdzNEN2ZTWWhEWkxUOWJCNjdCXG4ybGxiOGFycEJRcndZUFFB
+U2pUVUVYQnVJUUtCZ1FEUUxVemkrd0tHNEhLeko1NE1sQjFoR3cwSFZlWEV4T0pmXG53bS9IVjhl
+aThDeHZLMTRoRXpCT3JXQi9aNlo4VFFxWnA0eENnYkNiY0hwY3pLRUxvcDA2K2hqa1N3ZkR2TUJZ
+XG5mNnN6U2RSenNYVTI1NndmcG1hRjJ0TlJZZFpVblh2QWc5MFIrb1BFSjhrRHd4cjdiMGZmL3lu
+b0UrWUx0ckowXG53dklad3Joc093S0JnQWVPbWlTMHRZeUNnRkwvNHNuZ3ZodEs5WElGQ0w1VU9C
+dlp6Qk0xdlJOdjJ5eEFyRi9nXG5zajJqSmVyUWoyTUVpQkRmL2RQelZPYnBwaTByOCthMDNFOEdG
+OGZxakpxK2VnbDg2aXBaQjhxOUU5NTFyOUxSXG5Xa1ZtTEFEVVIxTC8rSjFhakxiWHJzOWlzZkxh
+ZEI2OUJpT1lXWmpPRk0reitocmNkYkR5blZraEFvR0FJbW42XG50ZU1zN2NNWTh3anZsY0MrZ3Br
+SU5GZzgzYVIyajhJQzNIOWtYMGs0N3ovS0ZjbW9TTGxjcEhNc0VJeGozamJXXG5kd0FkZy9TNkpi
+RW1SbGdoaWVoaVNRc21RM05ta0xxNlFJWkorcjR4VkZ4RUZnOWFEM0szVUZMT0xickRCSFpJXG5D
+M3JRWVpMNkpnY1E1TlBtbTk4QXZIN2RucjRiRGpaVDgzSS9McFVDZ1lFQWttNXlvVUtZY0tXMVQz
+R1hadUNIXG40SDNWVGVzZDZyb3pKWUhmTWVkNE9jQ3l1bnBIVmZmSmFCMFIxRjZ2MjFQaitCVWlW
+WjBzU010RjEvTE1uQkc4XG5TQVlQUnVxOHVNUUdNQTFpdE1Hc2VhMmg1V2RhbXNGODhXRFd4VEoy
+QXVnblJHNERsdmJLUDhPQmVLUFFKeDhEXG5RMzJ2SVpNUVkyV1hVMVhwUkMrNWs5RT1cbi0tLS0t
+RU5EIFBSSVZBVEUgS0VZLS0tLS1cbgo=`)
+	if err != nil {
+		t.Fatalf("Failed to decode fake key: %v", err)
+	}
+	fakePrivateKey := strings.TrimSpace(string(fakeKeyBuf))
+	cases := []struct {
+		name      string
+		fakeCreds string
+		useCookie bool
+		expected  string
+		contains  []string
+		err       bool
+	}{
+		{
+			name:     "anon auth works",
+			expected: fmt.Sprintf("https://%s/foo/bar/stuff", anonHost),
+		},
+		{
+			name:      "cookie auth works",
+			useCookie: true,
+			expected:  fmt.Sprintf("https://%s/foo/bar/stuff", cookieHost),
+		},
+		{
+			name:      "invalid json file errors",
+			fakeCreds: "yaml: 123",
+			err:       true,
+		},
+		{
+			name: "bad private key errors",
+			fakeCreds: `{
+			  "type": "service_account",
+			  "private_key": "-----BEGIN PRIVATE KEY-----\nMIIE==\n-----END PRIVATE KEY-----\n",
+			  "client_email": "[email protected]"
+			}`,
+			err: true,
+		},
+		{
+			name: "bad type errors",
+			fakeCreds: `{
+			  "type": "user",
+			  "private_key": "` + fakePrivateKey + `",
+			  "client_email": "[email protected]"
+			}`,
+			err: true,
+		},
+		{
+			name: "signed URLs work",
+			fakeCreds: `{
+			  "type": "service_account",
+			  "private_key": "` + fakePrivateKey + `",
+			  "client_email": "[email protected]"
+			}`,
+			contains: []string{
+				"https://storage.googleapis.com/foo/bar/stuff?",
+				"GoogleAccessId=fake-user%40k8s.io",
+				"Signature=", // Do not particularly care about the Signature contents
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var path string
+			if tc.fakeCreds != "" {
+				fp, err := ioutil.TempFile("", "fake-creds")
+				if err != nil {
+					t.Fatalf("Failed to create fake creds: %v", err)
+				}
+
+				path = fp.Name()
+				defer os.Remove(path)
+				if _, err := fp.Write([]byte(tc.fakeCreds)); err != nil {
+					t.Fatalf("Failed to write fake creds %s: %v", path, err)
+				}
+
+				if err := fp.Close(); err != nil {
+					t.Fatalf("Failed to close fake creds %s: %v", path, err)
+				}
+			}
+			af := NewGCSArtifactFetcher(nil, path, tc.useCookie)
+			actual, err := af.signURL("foo", "bar/stuff")
+			switch {
+			case err != nil:
+				if !tc.err {
+					t.Errorf("unexpected error: %v", err)
+				}
+			case tc.err:
+				t.Errorf("Failed to receive an expected error, got %q", actual)
+			case len(tc.contains) == 0 && actual != tc.expected:
+				t.Errorf("signURL(): got %q, want %q", actual, tc.expected)
+			default:
+				for _, part := range tc.contains {
+					if !strings.Contains(actual, part) {
+						t.Errorf("signURL(): got %q, does not contain %q", actual, part)
+					}
+				}
+			}
+		})
+	}
+}

prow/spyglass/spyglass.go

@@ -77,12 +77,12 @@ type ExtraLink struct {
 }
 
 // New constructs a Spyglass object from a JobAgent, a config.Agent, and a storage Client.
-func New(ja *jobs.JobAgent, cfg config.Getter, c *storage.Client, gcsCredsFile string, ctx context.Context) *Spyglass {
+func New(ctx context.Context, ja *jobs.JobAgent, cfg config.Getter, c *storage.Client, gcsCredsFile string, useCookieAuth bool) *Spyglass {
 	return &Spyglass{
 		JobAgent:              ja,
 		config:                cfg,
 		PodLogArtifactFetcher: NewPodLogArtifactFetcher(ja),
-		GCSArtifactFetcher:    NewGCSArtifactFetcher(c, gcsCredsFile),
+		GCSArtifactFetcher:    NewGCSArtifactFetcher(c, gcsCredsFile, useCookieAuth),
 		testgrid: &TestGrid{
 			conf:   cfg,
 			client: c,