diff --git a/prow/cmd/deck/main.go b/prow/cmd/deck/main.go index 785c0fd36487..4c58f6b8b84d 100644 --- a/prow/cmd/deck/main.go +++ b/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))) diff --git a/prow/spyglass/gcsartifact_fetcher.go b/prow/spyglass/gcsartifact_fetcher.go index ca39eca49184..6032b3d382f4 100644 --- a/prow/spyglass/gcsartifact_fetcher.go +++ b/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) diff --git a/prow/spyglass/gcsartifact_fetcher_test.go b/prow/spyglass/gcsartifact_fetcher_test.go index 5d211963056a..762e80b40b62 100644 --- a/prow/spyglass/gcsartifact_fetcher_test.go +++ b/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": "fake-user@k8s.io" + }`, + err: true, + }, + { + name: "bad type errors", + fakeCreds: `{ + "type": "user", + "private_key": "` + fakePrivateKey + `", + "client_email": "fake-user@k8s.io" + }`, + err: true, + }, + { + name: "signed URLs work", + fakeCreds: `{ + "type": "service_account", + "private_key": "` + fakePrivateKey + `", + "client_email": "fake-user@k8s.io" + }`, + 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) + } + } + } + }) + } +} diff --git a/prow/spyglass/spyglass.go b/prow/spyglass/spyglass.go index de2fe5df9f5b..50466418fa86 100644 --- a/prow/spyglass/spyglass.go +++ b/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, diff --git a/prow/spyglass/spyglass_test.go b/prow/spyglass/spyglass_test.go index 63e8720e94d1..6e090836f318 100644 --- a/prow/spyglass/spyglass_test.go +++ b/prow/spyglass/spyglass_test.go @@ -243,7 +243,7 @@ func TestViews(t *testing.T) { }, }, } - sg := New(fakeJa, c.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, c.Config, fakeGCSClient, "", false) _, ls := sg.Lenses(tc.lenses) for _, l := range ls { var found bool @@ -439,7 +439,7 @@ func TestJobPath(t *testing.T) { for _, tc := range testCases { fakeGCSClient := fakeGCSServer.Client() fca := config.Agent{} - sg := New(fakeJa, fca.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fca.Config, fakeGCSClient, "", false) jobPath, err := sg.JobPath(tc.src) if tc.expError && err == nil { t.Errorf("test %q: JobPath(%q) expected error", tc.name, tc.src) @@ -565,7 +565,7 @@ func TestProwJobName(t *testing.T) { for _, tc := range testCases { fakeGCSClient := fakeGCSServer.Client() fca := config.Agent{} - sg := New(fakeJa, fca.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fca.Config, fakeGCSClient, "", false) jobPath, err := sg.ProwJobName(tc.src) if tc.expError && err == nil { t.Errorf("test %q: JobPath(%q) expected error", tc.name, tc.src) @@ -690,7 +690,7 @@ func TestRunPath(t *testing.T) { }, }, }) - sg := New(fakeJa, fca.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fca.Config, fakeGCSClient, "", false) jobPath, err := sg.RunPath(tc.src) if tc.expError && err == nil { t.Errorf("test %q: RunPath(%q) expected error, got %q", tc.name, tc.src, jobPath) @@ -850,7 +850,7 @@ func TestRunToPR(t *testing.T) { }, }, }) - sg := New(fakeJa, fca.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fca.Config, fakeGCSClient, "", false) org, repo, num, err := sg.RunToPR(tc.src) if tc.expError && err == nil { t.Errorf("test %q: RunToPR(%q) expected error", tc.name, tc.src) @@ -937,7 +937,7 @@ func TestProwToGCS(t *testing.T) { } fakeJa = jobs.NewJobAgent(kc, map[string]jobs.PodLogClient{kube.DefaultClusterAlias: fpkc("clusterA"), "trusted": fpkc("clusterB")}, fakeConfigAgent.Config) fakeJa.Start() - sg := New(fakeJa, fakeConfigAgent.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fakeConfigAgent.Config, fakeGCSClient, "", false) p, err := sg.prowToGCS(tc.key) if err != nil && !tc.expectError { @@ -1072,7 +1072,7 @@ func TestGCSPathRoundTrip(t *testing.T) { fakeGCSClient := fakeGCSServer.Client() - sg := New(fakeJa, fakeConfigAgent.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fakeConfigAgent.Config, fakeGCSClient, "", false) gcspath, _, _ := gcsupload.PathsForJob( &prowapi.GCSConfiguration{Bucket: "test-bucket", PathStrategy: tc.pathStrategy}, &downwardapi.JobSpec{ @@ -1184,7 +1184,7 @@ func TestTestGridLink(t *testing.T) { }, }, }) - sg := New(fakeJa, fca.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fca.Config, fakeGCSClient, "", false) sg.testgrid = &tg link, err := sg.TestGridLink(tc.src) if tc.expError { @@ -1231,7 +1231,7 @@ func TestFetchArtifactsPodLog(t *testing.T) { fakeGCSClient := fakeGCSServer.Client() - sg := New(fakeJa, fakeConfigAgent.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fakeConfigAgent.Config, fakeGCSClient, "", false) testKeys := []string{ "prowjob/job/123", "gcs/kubernetes-jenkins/logs/job/123/", @@ -1389,7 +1389,7 @@ func TestResolveSymlink(t *testing.T) { fakeGCSClient := fakeGCSServer.Client() - sg := New(fakeJa, fakeConfigAgent.Config, fakeGCSClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fakeConfigAgent.Config, fakeGCSClient, "", false) result, err := sg.ResolveSymlink(tc.path) if err != nil { @@ -1481,7 +1481,7 @@ func TestExtraLinks(t *testing.T) { fakeConfigAgent := fca{} fakeJa = jobs.NewJobAgent(fkc{}, map[string]jobs.PodLogClient{kube.DefaultClusterAlias: fpkc("clusterA")}, fakeConfigAgent.Config) fakeJa.Start() - sg := New(fakeJa, fakeConfigAgent.Config, gcsClient, "", context.Background()) + sg := New(context.Background(), fakeJa, fakeConfigAgent.Config, gcsClient, "", false) result, err := sg.ExtraLinks("gcs/test-bucket/logs/some-job/42") if err != nil {