Skip to content

Commit

Permalink
Merge pull request #16491 from fejta/rev
Browse files Browse the repository at this point in the history
Allow spyglass to use cookie auth
  • Loading branch information
k8s-ci-robot authored Mar 4, 2020
2 parents 3dbf7d4 + 95eb5fb commit 34d7713
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 30 deletions.
19 changes: 10 additions & 9 deletions prow/cmd/deck/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -117,6 +117,7 @@ type options struct {
spyglass bool
spyglassFilesLocation string
gcsCredentialsFile string
gcsCookieAuth bool
rerunCreatesJob bool
allowInsecure bool
dryRun bool
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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)))
Expand Down
31 changes: 25 additions & 6 deletions prow/spyglass/gcsartifact_fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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)
Expand Down
143 changes: 141 additions & 2 deletions prow/spyglass/gcsartifact_fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ limitations under the License.
package spyglass

import (
"encoding/base64"
"fmt"
"io/ioutil"
"os"
"strings"
"testing"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
}
})
}
}
4 changes: 2 additions & 2 deletions prow/spyglass/spyglass.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 34d7713

Please sign in to comment.