[arvados] created: 2.6.0-2-gddccc6d8f
git repository hosting
git at public.arvados.org
Thu Apr 20 02:45:26 UTC 2023
at ddccc6d8f1e04c8708b99dd2d0d3ee4b9b70528f (commit)
commit ddccc6d8f1e04c8708b99dd2d0d3ee4b9b70528f
Author: Tom Clegg <tom at curii.com>
Date: Wed Apr 19 22:45:16 2023 -0400
20319: Move /containers/*/log to /container_requests/*/log.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 268b9eefb..430c189a2 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -385,10 +385,6 @@ func (conn *Conn) ContainerUnlock(ctx context.Context, options arvados.GetOption
return conn.chooseBackend(options.UUID).ContainerUnlock(ctx, options)
}
-func (conn *Conn) ContainerLog(ctx context.Context, options arvados.ContainerLogOptions) (http.Handler, error) {
- return conn.chooseBackend(options.UUID).ContainerLog(ctx, options)
-}
-
func (conn *Conn) ContainerSSH(ctx context.Context, options arvados.ContainerSSHOptions) (arvados.ConnectionResponse, error) {
return conn.chooseBackend(options.UUID).ContainerSSH(ctx, options)
}
@@ -459,6 +455,10 @@ func (conn *Conn) ContainerRequestDelete(ctx context.Context, options arvados.De
return conn.chooseBackend(options.UUID).ContainerRequestDelete(ctx, options)
}
+func (conn *Conn) ContainerRequestLog(ctx context.Context, options arvados.ContainerLogOptions) (http.Handler, error) {
+ return conn.chooseBackend(options.UUID).ContainerRequestLog(ctx, options)
+}
+
func (conn *Conn) GroupCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Group, error) {
return conn.chooseBackend(options.ClusterID).GroupCreate(ctx, options)
}
diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go
index 59592ece9..11d139663 100644
--- a/lib/controller/localdb/container_gateway.go
+++ b/lib/controller/localdb/container_gateway.go
@@ -40,22 +40,45 @@ var (
forceInternalURLForTest *arvados.URL
)
-// ContainerLog returns a WebDAV handler that reads logs from the
-// indicated container. It works by proxying the request to
+// ContainerRequestLog returns a WebDAV handler that reads logs from
+// the indicated container request. It works by proxying the incoming
+// HTTP request to
//
-// - the container gateway, if the container is running
+// - the container gateway, if there is an associated container that
+// is running
//
-// - a different controller process, if the container is running and
-// the gateway is accessible through a tunnel to a different
+// - a different controller process, if there is a running container
+// whose gateway is accessible through a tunnel to a different
// controller process
//
// - keep-web, if saved logs exist and there is no gateway (or the
-// container is finished)
+// associated container is finished)
//
// - an empty-collection stub, if there is no gateway and no saved
// log
-func (conn *Conn) ContainerLog(ctx context.Context, opts arvados.ContainerLogOptions) (http.Handler, error) {
- ctr, err := conn.railsProxy.ContainerGet(ctx, arvados.GetOptions{UUID: opts.UUID, Select: []string{"uuid", "state", "gateway_address", "log"}})
+//
+// For an incoming request
+//
+// GET /arvados/v1/container_requests/{cr_uuid}/log/{c_uuid}{/c_log_path}
+//
+// The upstream request may be to {c_uuid}'s container gateway
+//
+// GET /arvados/v1/container_requests/{cr_uuid}/log/{c_uuid}{/c_log_path}
+// X-Webdav-Prefix: /arvados/v1/container_requests/{cr_uuid}/log/{c_uuid}
+// X-Webdav-Source: /log
+//
+// ...or the upstream request may be to keep-web (where {cr_log_uuid}
+// is the container request log collection UUID)
+//
+// GET /arvados/v1/container_requests/{cr_uuid}/log/{c_uuid}{/c_log_path}
+// Host: {cr_log_uuid}.internal
+// X-Webdav-Prefix: /arvados/v1/container_requests/{cr_uuid}/log
+// X-Arvados-Container-Uuid: {c_uuid}
+//
+// ...or the request may be handled locally using an empty-collection
+// stub.
+func (conn *Conn) ContainerRequestLog(ctx context.Context, opts arvados.ContainerLogOptions) (http.Handler, error) {
+ cr, err := conn.railsProxy.ContainerRequestGet(ctx, arvados.GetOptions{UUID: opts.UUID, Select: []string{"uuid", "container_uuid", "log_uuid"}})
if err != nil {
if se := httpserver.HTTPStatusError(nil); errors.As(err, &se) && se.HTTPStatus() == http.StatusUnauthorized {
// Hint to WebDAV client that we accept HTTP basic auth.
@@ -66,10 +89,29 @@ func (conn *Conn) ContainerLog(ctx context.Context, opts arvados.ContainerLogOpt
}
return nil, err
}
+ ctr, err := conn.railsProxy.ContainerGet(ctx, arvados.GetOptions{UUID: cr.ContainerUUID, Select: []string{"uuid", "state", "gateway_address"}})
+ if err != nil {
+ return nil, err
+ }
+ // .../log/{ctr.UUID} is a directory where the currently
+ // assigned container's log data [will] appear (as opposed to
+ // previous attempts in .../log/{previous_ctr_uuid}). Requests
+ // that are outside that directory, and requests on a
+ // non-running container, are proxied to keep-web instead of
+ // going through the container gateway system.
+ //
+ // Side note: a depth>1 directory tree listing starting at
+ // .../{cr_uuid}/log will only include subdirectories for
+ // finished containers, i.e., will not include a subdirectory
+ // with log data for a current (unfinished) container UUID.
+ // In order to access live logs, a client must look up the
+ // container_uuid field of the container request record, and
+ // explicitly request a path under .../{cr_uuid}/log/{c_uuid}.
if ctr.GatewayAddress == "" ||
- (ctr.State != arvados.ContainerStateLocked && ctr.State != arvados.ContainerStateRunning) {
+ (ctr.State != arvados.ContainerStateLocked && ctr.State != arvados.ContainerStateRunning) ||
+ !(opts.Path == "/"+ctr.UUID || strings.HasPrefix(opts.Path, "/"+ctr.UUID+"/")) {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
- conn.serveContainerLogViaKeepWeb(opts, ctr, w, r)
+ conn.serveContainerRequestLogViaKeepWeb(opts, cr, w, r)
}), nil
}
dial, arpc, err := conn.findGateway(ctx, ctr, opts.NoForward)
@@ -78,7 +120,7 @@ func (conn *Conn) ContainerLog(ctx context.Context, opts arvados.ContainerLogOpt
}
if arpc != nil {
opts.NoForward = true
- return arpc.ContainerLog(ctx, opts)
+ return arpc.ContainerRequestLog(ctx, opts)
}
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
r = r.WithContext(ctx)
@@ -119,7 +161,9 @@ func (conn *Conn) ContainerLog(ctx context.Context, opts arvados.ContainerLogOpt
// for net/http to work with.
r.URL.Scheme = "https"
r.URL.Host = "0.0.0.0:0"
- r.Header.Set("X-Arvados-Container-Gateway-Uuid", opts.UUID)
+ r.Header.Set("X-Arvados-Container-Gateway-Uuid", ctr.UUID)
+ r.Header.Set("X-Webdav-Prefix", "/arvados/v1/container_requests/"+cr.UUID+"/log/"+ctr.UUID)
+ r.Header.Set("X-Webdav-Source", "/log")
proxyReq = r
},
ModifyResponse: func(resp *http.Response) error {
@@ -144,7 +188,7 @@ func (conn *Conn) ContainerLog(ctx context.Context, opts arvados.ContainerLogOpt
// after we decided (above) the log was not final.
// In that case we should proxy to keep-web.
ctr, err := conn.railsProxy.ContainerGet(ctx, arvados.GetOptions{
- UUID: opts.UUID,
+ UUID: ctr.UUID,
Select: []string{"uuid", "state", "gateway_address", "log"},
})
if err != nil {
@@ -154,7 +198,7 @@ func (conn *Conn) ContainerLog(ctx context.Context, opts arvados.ContainerLogOpt
// No race, proxyErr was the best we can do
httpserver.Error(w, "proxy error: "+proxyErr.Error(), http.StatusServiceUnavailable)
} else {
- conn.serveContainerLogViaKeepWeb(opts, ctr, w, r)
+ conn.serveContainerRequestLogViaKeepWeb(opts, cr, w, r)
}
}), nil
}
@@ -163,12 +207,12 @@ func (conn *Conn) ContainerLog(ctx context.Context, opts arvados.ContainerLogOpt
// log content by proxying to one of the configured keep-web servers.
//
// It tries to choose a keep-web server that is running on this host.
-func (conn *Conn) serveContainerLogViaKeepWeb(opts arvados.ContainerLogOptions, ctr arvados.Container, w http.ResponseWriter, r *http.Request) {
- if ctr.Log == "" {
+func (conn *Conn) serveContainerRequestLogViaKeepWeb(opts arvados.ContainerLogOptions, cr arvados.ContainerRequest, w http.ResponseWriter, r *http.Request) {
+ if cr.LogUUID == "" {
// Special case: if no log data exists yet, we serve
// an empty collection by ourselves instead of
// proxying to keep-web.
- conn.serveEmptyDir("/arvados/v1/containers/"+ctr.UUID+"/log", w, r)
+ conn.serveEmptyDir("/arvados/v1/container_requests/"+cr.UUID+"/log", w, r)
return
}
myURL, _ := service.URLFromContext(r.Context())
@@ -196,7 +240,7 @@ func (conn *Conn) serveContainerLogViaKeepWeb(opts arvados.ContainerLogOptions,
r.URL.Host = webdavBase.Host
// Outgoing Host header specifies the
// collection ID.
- r.Host = strings.Replace(ctr.Log, "+", "-", -1) + ".internal"
+ r.Host = cr.LogUUID + ".internal"
// We already checked permission on the
// container, so we can use a root token here
// instead of counting on the "access to log
@@ -209,7 +253,14 @@ func (conn *Conn) serveContainerLogViaKeepWeb(opts arvados.ContainerLogOptions,
// headers refer to the same paths) so we tell
// keep-web to map the log collection onto the
// containers/X/log/ namespace.
- r.Header.Set("X-Webdav-Prefix", "/arvados/v1/containers/"+ctr.UUID+"/log")
+ r.Header.Set("X-Webdav-Prefix", "/arvados/v1/container_requests/"+cr.UUID+"/log")
+ if len(opts.Path) >= 28 && opts.Path[6:13] == "-dz642-" {
+ // "/arvados/v1/container_requests/{crUUID}/log/{cUUID}..."
+ // proxies to
+ // "/log for container {cUUID}..."
+ r.Header.Set("X-Webdav-Prefix", "/arvados/v1/container_requests/"+cr.UUID+"/log/"+opts.Path[1:28])
+ r.Header.Set("X-Webdav-Source", "/log for container "+opts.Path[1:28]+"/")
+ }
},
}
if conn.cluster.TLS.Insecure {
diff --git a/lib/controller/localdb/container_gateway_test.go b/lib/controller/localdb/container_gateway_test.go
index 4884eda46..dc8a8cea0 100644
--- a/lib/controller/localdb/container_gateway_test.go
+++ b/lib/controller/localdb/container_gateway_test.go
@@ -39,6 +39,7 @@ var _ = check.Suite(&ContainerGatewaySuite{})
type ContainerGatewaySuite struct {
localdbSuite
+ reqUUID string
ctrUUID string
srv *httptest.Server
gw *crunchrun.Gateway
@@ -47,7 +48,29 @@ type ContainerGatewaySuite struct {
func (s *ContainerGatewaySuite) SetUpTest(c *check.C) {
s.localdbSuite.SetUpTest(c)
- s.ctrUUID = arvadostest.QueuedContainerUUID
+ cr, err := s.localdb.ContainerRequestCreate(s.userctx, arvados.CreateOptions{
+ Attrs: map[string]interface{}{
+ "command": []string{"echo", time.Now().Format(time.RFC3339Nano)},
+ "container_count_max": 1,
+ "container_image": "arvados/apitestfixture:latest",
+ "cwd": "/tmp",
+ "environment": map[string]string{},
+ "output_path": "/out",
+ "priority": 1,
+ "state": arvados.ContainerRequestStateCommitted,
+ "mounts": map[string]interface{}{
+ "/out": map[string]interface{}{
+ "kind": "tmp",
+ "capacity": 1000000,
+ },
+ },
+ "runtime_constraints": map[string]interface{}{
+ "vcpus": 1,
+ "ram": 2,
+ }}})
+ c.Assert(err, check.IsNil)
+ s.reqUUID = cr.UUID
+ s.ctrUUID = cr.ContainerUUID
h := hmac.New(sha256.New, []byte(s.cluster.SystemRootToken))
fmt.Fprint(h, s.ctrUUID)
@@ -75,15 +98,14 @@ func (s *ContainerGatewaySuite) SetUpTest(c *check.C) {
ArvadosClient: ac,
}
c.Assert(s.gw.Start(), check.IsNil)
+
rootctx := ctrlctx.NewWithToken(s.ctx, s.cluster, s.cluster.SystemRootToken)
- // OK if this line fails (because state is already Running
- // from a previous test case) as long as the following line
- // succeeds:
- s.localdb.ContainerUpdate(rootctx, arvados.UpdateOptions{
+ _, err = s.localdb.ContainerUpdate(rootctx, arvados.UpdateOptions{
UUID: s.ctrUUID,
Attrs: map[string]interface{}{
"state": arvados.ContainerStateLocked}})
- _, err := s.localdb.ContainerUpdate(rootctx, arvados.UpdateOptions{
+ c.Assert(err, check.IsNil)
+ _, err = s.localdb.ContainerUpdate(rootctx, arvados.UpdateOptions{
UUID: s.ctrUUID,
Attrs: map[string]interface{}{
"state": arvados.ContainerStateRunning,
@@ -243,18 +265,23 @@ func (s *ContainerGatewaySuite) saveLogAndCloseGateway(c *check.C) {
_, err = s.localdb.ContainerUpdate(rootctx, arvados.UpdateOptions{
UUID: s.ctrUUID,
Attrs: map[string]interface{}{
- "log": coll.PortableDataHash,
- "gateway_address": "",
+ "state": arvados.ContainerStateComplete,
+ "exit_code": 0,
+ "log": coll.PortableDataHash,
}})
c.Assert(err, check.IsNil)
- // gateway_address="" above already ensures localdb
- // can't circumvent the keep-web proxy test by getting
- // content from the container gateway; this is just
- // extra insurance.
+ updatedReq, err := s.localdb.ContainerRequestGet(rootctx, arvados.GetOptions{UUID: s.reqUUID})
+ c.Assert(err, check.IsNil)
+ c.Logf("container request log UUID is %s", updatedReq.LogUUID)
+ crLog, err := s.localdb.CollectionGet(rootctx, arvados.GetOptions{UUID: updatedReq.LogUUID, Select: []string{"manifest_text"}})
+ c.Assert(err, check.IsNil)
+ c.Logf("collection log manifest:\n%s", crLog.ManifestText)
+ // Ensure localdb can't circumvent the keep-web proxy test by
+ // getting content from the container gateway.
s.gw.LogCollection = nil
}
-func (s *ContainerGatewaySuite) TestContainerLogViaTunnel(c *check.C) {
+func (s *ContainerGatewaySuite) TestContainerRequestLogViaTunnel(c *check.C) {
forceProxyForTest = true
defer func() { forceProxyForTest = false }()
@@ -271,9 +298,9 @@ func (s *ContainerGatewaySuite) TestContainerLogViaTunnel(c *check.C) {
defer delete(s.cluster.Services.Controller.InternalURLs, *forceInternalURLForTest)
}
- handler, err := s.localdb.ContainerLog(s.userctx, arvados.ContainerLogOptions{
- UUID: s.ctrUUID,
- WebDAVOptions: arvados.WebDAVOptions{Path: "/stderr.txt"},
+ handler, err := s.localdb.ContainerRequestLog(s.userctx, arvados.ContainerLogOptions{
+ UUID: s.reqUUID,
+ WebDAVOptions: arvados.WebDAVOptions{Path: "/" + s.ctrUUID + "/stderr.txt"},
})
if broken {
c.Check(err, check.ErrorMatches, `.*tunnel endpoint is invalid.*`)
@@ -281,7 +308,7 @@ func (s *ContainerGatewaySuite) TestContainerLogViaTunnel(c *check.C) {
}
c.Check(err, check.IsNil)
c.Assert(handler, check.NotNil)
- r, err := http.NewRequestWithContext(s.userctx, "GET", "https://controller.example/arvados/v1/containers/"+s.ctrUUID+"/log/stderr.txt", nil)
+ r, err := http.NewRequestWithContext(s.userctx, "GET", "https://controller.example/arvados/v1/container_requests/"+s.reqUUID+"/log/"+s.ctrUUID+"/stderr.txt", nil)
c.Assert(err, check.IsNil)
r.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
rec := httptest.NewRecorder()
@@ -294,18 +321,18 @@ func (s *ContainerGatewaySuite) TestContainerLogViaTunnel(c *check.C) {
}
}
-func (s *ContainerGatewaySuite) TestContainerLogViaGateway(c *check.C) {
+func (s *ContainerGatewaySuite) TestContainerRequestLogViaGateway(c *check.C) {
s.setupLogCollection(c)
- s.testContainerLog(c)
+ s.testContainerRequestLog(c)
}
-func (s *ContainerGatewaySuite) TestContainerLogViaKeepWeb(c *check.C) {
+func (s *ContainerGatewaySuite) TestContainerRequestLogViaKeepWeb(c *check.C) {
s.setupLogCollection(c)
s.saveLogAndCloseGateway(c)
- s.testContainerLog(c)
+ s.testContainerRequestLog(c)
}
-func (s *ContainerGatewaySuite) testContainerLog(c *check.C) {
+func (s *ContainerGatewaySuite) testContainerRequestLog(c *check.C) {
for _, trial := range []struct {
method string
path string
@@ -316,7 +343,7 @@ func (s *ContainerGatewaySuite) testContainerLog(c *check.C) {
}{
{
method: "GET",
- path: "/stderr.txt",
+ path: s.ctrUUID + "/stderr.txt",
expectStatus: http.StatusOK,
expectBodyRe: "hello world\n",
expectHeader: http.Header{
@@ -325,7 +352,7 @@ func (s *ContainerGatewaySuite) testContainerLog(c *check.C) {
},
{
method: "GET",
- path: "/stderr.txt",
+ path: s.ctrUUID + "/stderr.txt",
header: http.Header{
"Range": {"bytes=-6"},
},
@@ -338,7 +365,7 @@ func (s *ContainerGatewaySuite) testContainerLog(c *check.C) {
},
{
method: "OPTIONS",
- path: "/stderr.txt",
+ path: s.ctrUUID + "/stderr.txt",
expectStatus: http.StatusOK,
expectBodyRe: "",
expectHeader: http.Header{
@@ -348,25 +375,34 @@ func (s *ContainerGatewaySuite) testContainerLog(c *check.C) {
},
{
method: "PROPFIND",
- path: "",
+ path: s.ctrUUID + "/",
+ expectStatus: http.StatusMultiStatus,
+ expectBodyRe: `.*\Q<D:displayname>stderr.txt</D:displayname>\E.*>\n?`,
+ expectHeader: http.Header{
+ "Content-Type": {"text/xml; charset=utf-8"},
+ },
+ },
+ {
+ method: "PROPFIND",
+ path: s.ctrUUID,
expectStatus: http.StatusMultiStatus,
- expectBodyRe: `.*\Q<D:displayname>stderr.txt</D:displayname>\E.*`,
+ expectBodyRe: `.*\Q<D:displayname>stderr.txt</D:displayname>\E.*>\n?`,
expectHeader: http.Header{
"Content-Type": {"text/xml; charset=utf-8"},
},
},
{
method: "PROPFIND",
- path: "/a/b/c/",
+ path: s.ctrUUID + "/a/b/c/",
expectStatus: http.StatusMultiStatus,
- expectBodyRe: `.*\Q<D:displayname>d.html</D:displayname>\E.*`,
+ expectBodyRe: `.*\Q<D:displayname>d.html</D:displayname>\E.*>\n?`,
expectHeader: http.Header{
"Content-Type": {"text/xml; charset=utf-8"},
},
},
{
method: "GET",
- path: "/a/b/c/d.html",
+ path: s.ctrUUID + "/a/b/c/d.html",
expectStatus: http.StatusOK,
expectBodyRe: "<html></html>\n",
expectHeader: http.Header{
@@ -375,13 +411,13 @@ func (s *ContainerGatewaySuite) testContainerLog(c *check.C) {
},
} {
c.Logf("trial %#v", trial)
- handler, err := s.localdb.ContainerLog(s.userctx, arvados.ContainerLogOptions{
- UUID: s.ctrUUID,
- WebDAVOptions: arvados.WebDAVOptions{Path: trial.path},
+ handler, err := s.localdb.ContainerRequestLog(s.userctx, arvados.ContainerLogOptions{
+ UUID: s.reqUUID,
+ WebDAVOptions: arvados.WebDAVOptions{Path: "/" + trial.path},
})
c.Assert(err, check.IsNil)
c.Assert(handler, check.NotNil)
- r, err := http.NewRequestWithContext(s.userctx, trial.method, "https://controller.example/arvados/v1/containers/"+s.ctrUUID+"/log"+trial.path, nil)
+ r, err := http.NewRequestWithContext(s.userctx, trial.method, "https://controller.example/arvados/v1/container_requests/"+s.reqUUID+"/log/"+trial.path, nil)
c.Assert(err, check.IsNil)
for k := range trial.header {
r.Header.Set(k, trial.header.Get(k))
@@ -399,19 +435,19 @@ func (s *ContainerGatewaySuite) testContainerLog(c *check.C) {
}
}
-func (s *ContainerGatewaySuite) TestContainerLogViaCadaver(c *check.C) {
+func (s *ContainerGatewaySuite) TestContainerRequestLogViaCadaver(c *check.C) {
s.setupLogCollection(c)
- out := s.runCadaver(c, arvadostest.ActiveToken, "/arvados/v1/containers/"+s.ctrUUID+"/log", "ls")
+ out := s.runCadaver(c, arvadostest.ActiveToken, "/arvados/v1/container_requests/"+s.reqUUID+"/log/"+s.ctrUUID, "ls")
c.Check(out, check.Matches, `(?ms).*stderr\.txt\s+12\s.*`)
c.Check(out, check.Matches, `(?ms).*a\s+0\s.*`)
- out = s.runCadaver(c, arvadostest.ActiveTokenV2, "/arvados/v1/containers/"+s.ctrUUID+"/log", "get stderr.txt")
+ out = s.runCadaver(c, arvadostest.ActiveTokenV2, "/arvados/v1/container_requests/"+s.reqUUID+"/log/"+s.ctrUUID, "get stderr.txt")
c.Check(out, check.Matches, `(?ms).*Downloading .* to stderr\.txt: .* succeeded\..*`)
s.saveLogAndCloseGateway(c)
- out = s.runCadaver(c, arvadostest.ActiveTokenV2, "/arvados/v1/containers/"+s.ctrUUID+"/log", "get stderr.txt")
+ out = s.runCadaver(c, arvadostest.ActiveTokenV2, "/arvados/v1/container_requests/"+s.reqUUID+"/log/"+s.ctrUUID, "get stderr.txt")
c.Check(out, check.Matches, `(?ms).*Downloading .* to stderr\.txt: .* succeeded\..*`)
}
diff --git a/lib/controller/router/router.go b/lib/controller/router/router.go
index 2cbd9b88d..b9eb23d05 100644
--- a/lib/controller/router/router.go
+++ b/lib/controller/router/router.go
@@ -209,13 +209,6 @@ func (rtr *router) addRoutes() {
return rtr.backend.ContainerUnlock(ctx, *opts.(*arvados.GetOptions))
},
},
- {
- arvados.EndpointContainerLog,
- func() interface{} { return &arvados.ContainerLogOptions{} },
- func(ctx context.Context, opts interface{}) (interface{}, error) {
- return rtr.backend.ContainerLog(ctx, *opts.(*arvados.ContainerLogOptions))
- },
- },
{
arvados.EndpointContainerSSH,
func() interface{} { return &arvados.ContainerSSHOptions{} },
@@ -290,6 +283,13 @@ func (rtr *router) addRoutes() {
return rtr.backend.ContainerRequestDelete(ctx, *opts.(*arvados.DeleteOptions))
},
},
+ {
+ arvados.EndpointContainerRequestLog,
+ func() interface{} { return &arvados.ContainerLogOptions{} },
+ func(ctx context.Context, opts interface{}) (interface{}, error) {
+ return rtr.backend.ContainerRequestLog(ctx, *opts.(*arvados.ContainerLogOptions))
+ },
+ },
{
arvados.EndpointGroupCreate,
func() interface{} { return &arvados.CreateOptions{} },
diff --git a/lib/controller/router/router_test.go b/lib/controller/router/router_test.go
index b194bd222..d4ee8794c 100644
--- a/lib/controller/router/router_test.go
+++ b/lib/controller/router/router_test.go
@@ -177,7 +177,7 @@ func (s *RouterSuite) TestOptions(c *check.C) {
{
comment: "container log webdav GET root",
method: "GET",
- path: "/arvados/v1/containers/" + arvadostest.CompletedContainerUUID + "/log/",
+ path: "/arvados/v1/container_requests/" + arvadostest.CompletedContainerRequestUUID + "/log/" + arvadostest.CompletedContainerUUID + "/",
shouldCall: "ContainerLog",
withOptions: arvados.ContainerLogOptions{
UUID: arvadostest.CompletedContainerUUID,
@@ -189,7 +189,7 @@ func (s *RouterSuite) TestOptions(c *check.C) {
{
comment: "container log webdav GET root without trailing slash",
method: "GET",
- path: "/arvados/v1/containers/" + arvadostest.CompletedContainerUUID + "/log",
+ path: "/arvados/v1/container_requests/" + arvadostest.CompletedContainerRequestUUID + "/log/" + arvadostest.CompletedContainerUUID + "",
shouldCall: "ContainerLog",
withOptions: arvados.ContainerLogOptions{
UUID: arvadostest.CompletedContainerUUID,
@@ -201,7 +201,7 @@ func (s *RouterSuite) TestOptions(c *check.C) {
{
comment: "container log webdav OPTIONS root",
method: "OPTIONS",
- path: "/arvados/v1/containers/" + arvadostest.CompletedContainerUUID + "/log/",
+ path: "/arvados/v1/container_requests/" + arvadostest.CompletedContainerRequestUUID + "/log/" + arvadostest.CompletedContainerUUID + "/",
shouldCall: "ContainerLog",
withOptions: arvados.ContainerLogOptions{
UUID: arvadostest.CompletedContainerUUID,
@@ -213,7 +213,7 @@ func (s *RouterSuite) TestOptions(c *check.C) {
{
comment: "container log webdav OPTIONS root without trailing slash",
method: "OPTIONS",
- path: "/arvados/v1/containers/" + arvadostest.CompletedContainerUUID + "/log",
+ path: "/arvados/v1/container_requests/" + arvadostest.CompletedContainerRequestUUID + "/log/" + arvadostest.CompletedContainerUUID,
shouldCall: "ContainerLog",
withOptions: arvados.ContainerLogOptions{
UUID: arvadostest.CompletedContainerUUID,
@@ -225,7 +225,7 @@ func (s *RouterSuite) TestOptions(c *check.C) {
{
comment: "container log webdav PROPFIND root",
method: "PROPFIND",
- path: "/arvados/v1/containers/" + arvadostest.CompletedContainerUUID + "/log/",
+ path: "/arvados/v1/container_requests/" + arvadostest.CompletedContainerRequestUUID + "/log/" + arvadostest.CompletedContainerUUID + "/",
shouldCall: "ContainerLog",
withOptions: arvados.ContainerLogOptions{
UUID: arvadostest.CompletedContainerUUID,
@@ -237,7 +237,7 @@ func (s *RouterSuite) TestOptions(c *check.C) {
{
comment: "container log webdav PROPFIND root without trailing slash",
method: "PROPFIND",
- path: "/arvados/v1/containers/" + arvadostest.CompletedContainerUUID + "/log",
+ path: "/arvados/v1/container_requests/" + arvadostest.CompletedContainerRequestUUID + "/log/" + arvadostest.CompletedContainerUUID + "",
shouldCall: "ContainerLog",
withOptions: arvados.ContainerLogOptions{
UUID: arvadostest.CompletedContainerUUID,
@@ -249,7 +249,7 @@ func (s *RouterSuite) TestOptions(c *check.C) {
{
comment: "container log webdav no_forward=true",
method: "GET",
- path: "/arvados/v1/containers/" + arvadostest.CompletedContainerUUID + "/log/?no_forward=true",
+ path: "/arvados/v1/container_requests/" + arvadostest.CompletedContainerRequestUUID + "/log/" + arvadostest.CompletedContainerUUID + "/?no_forward=true",
shouldCall: "ContainerLog",
withOptions: arvados.ContainerLogOptions{
UUID: arvadostest.CompletedContainerUUID,
@@ -260,9 +260,9 @@ func (s *RouterSuite) TestOptions(c *check.C) {
Path: "/"}},
},
{
- comment: "/logX does not route to ContainerLog",
+ comment: "/logX does not route to ContainerRequestLog",
method: "GET",
- path: "/arvados/v1/containers/" + arvadostest.CompletedContainerUUID + "/logX",
+ path: "/arvados/v1/containers/" + arvadostest.CompletedContainerRequestUUID + "/logX",
shouldStatus: http.StatusNotFound,
shouldCall: "",
},
diff --git a/lib/controller/rpc/conn.go b/lib/controller/rpc/conn.go
index 70a936a6f..ca9f9856f 100644
--- a/lib/controller/rpc/conn.go
+++ b/lib/controller/rpc/conn.go
@@ -339,19 +339,6 @@ func (conn *Conn) ContainerUnlock(ctx context.Context, options arvados.GetOption
return resp, err
}
-func (conn *Conn) ContainerLog(ctx context.Context, options arvados.ContainerLogOptions) (resp http.Handler, err error) {
- proxy := &httputil.ReverseProxy{
- Transport: conn.httpClient.Transport,
- Director: func(r *http.Request) {
- u := conn.baseURL
- u.Path = r.URL.Path
- u.RawQuery = fmt.Sprintf("no_forward=%v", options.NoForward)
- r.URL = &u
- },
- }
- return proxy, nil
-}
-
// ContainerSSH returns a connection to the out-of-band SSH server for
// a running container. If the returned error is nil, the caller is
// responsible for closing sshconn.Conn.
@@ -484,6 +471,19 @@ func (conn *Conn) ContainerRequestDelete(ctx context.Context, options arvados.De
return resp, err
}
+func (conn *Conn) ContainerRequestLog(ctx context.Context, options arvados.ContainerLogOptions) (resp http.Handler, err error) {
+ proxy := &httputil.ReverseProxy{
+ Transport: conn.httpClient.Transport,
+ Director: func(r *http.Request) {
+ u := conn.baseURL
+ u.Path = r.URL.Path
+ u.RawQuery = fmt.Sprintf("no_forward=%v", options.NoForward)
+ r.URL = &u
+ },
+ }
+ return proxy, nil
+}
+
func (conn *Conn) GroupCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Group, error) {
ep := arvados.EndpointGroupCreate
var resp arvados.Group
diff --git a/lib/crunchrun/container_gateway.go b/lib/crunchrun/container_gateway.go
index 7fd82a8bf..30f8957a2 100644
--- a/lib/crunchrun/container_gateway.go
+++ b/lib/crunchrun/container_gateway.go
@@ -81,14 +81,13 @@ type Gateway struct {
// controller process at the other end of the tunnel.
UpdateTunnelURL func(url string)
- // Source for serving WebDAV requests at
- // /arvados/v1/containers/{uuid}/log/
+ // Source for serving WebDAV requests with
+ // X-Webdav-Source: /log
LogCollection arvados.CollectionFileSystem
sshConfig ssh.ServerConfig
requestAuth string
respondAuth string
- logPath string
}
// Start starts an http server that allows authenticated clients to open an
@@ -163,8 +162,6 @@ func (gw *Gateway) Start() error {
h.Write([]byte(gw.requestAuth))
gw.respondAuth = fmt.Sprintf("%x", h.Sum(nil))
- gw.logPath = "/arvados/v1/containers/" + gw.ContainerUUID + "/log"
-
srv := &httpserver.Server{
Server: http.Server{
Handler: gw,
@@ -277,6 +274,7 @@ var webdavMethod = map[string]bool{
}
func (gw *Gateway) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+ w.Header().Set("Vary", "X-Arvados-Authorization, X-Arvados-Container-Gateway-Uuid, X-Webdav-Prefix, X-Webdav-Source")
reqUUID := req.Header.Get("X-Arvados-Container-Gateway-Uuid")
if reqUUID == "" {
// older controller versions only send UUID as query param
@@ -295,7 +293,7 @@ func (gw *Gateway) ServeHTTP(w http.ResponseWriter, req *http.Request) {
switch {
case req.Method == "POST" && req.Header.Get("Upgrade") == "ssh":
gw.handleSSH(w, req)
- case req.URL.Path == gw.logPath || strings.HasPrefix(req.URL.Path, gw.logPath):
+ case req.Header.Get("X-Webdav-Source") == "/log":
if !webdavMethod[req.Method] {
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
return
@@ -307,12 +305,17 @@ func (gw *Gateway) ServeHTTP(w http.ResponseWriter, req *http.Request) {
}
func (gw *Gateway) handleLogsWebDAV(w http.ResponseWriter, r *http.Request) {
+ prefix := r.Header.Get("X-Webdav-Prefix")
+ if !strings.HasPrefix(r.URL.Path, prefix) {
+ http.Error(w, "X-Webdav-Prefix header is not a prefix of the requested path", http.StatusBadRequest)
+ return
+ }
if gw.LogCollection == nil {
http.Error(w, "Not found", http.StatusNotFound)
return
}
wh := webdav.Handler{
- Prefix: gw.logPath,
+ Prefix: prefix,
FileSystem: &webdavfs.FS{
FileSystem: gw.LogCollection,
Prefix: "",
diff --git a/sdk/go/arvados/api.go b/sdk/go/arvados/api.go
index 861b8e6ce..fa4a4ac4a 100644
--- a/sdk/go/arvados/api.go
+++ b/sdk/go/arvados/api.go
@@ -49,7 +49,6 @@ var (
EndpointContainerDelete = APIEndpoint{"DELETE", "arvados/v1/containers/{uuid}", ""}
EndpointContainerLock = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/lock", ""}
EndpointContainerUnlock = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/unlock", ""}
- EndpointContainerLog = APIEndpoint{"GET", "arvados/v1/containers/{uuid}/log{path:|/.*}", ""}
EndpointContainerSSH = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/ssh", ""}
EndpointContainerSSHCompat = APIEndpoint{"POST", "arvados/v1/connect/{uuid}/ssh", ""} // for compatibility with arvados <2.7
EndpointContainerGatewayTunnel = APIEndpoint{"POST", "arvados/v1/containers/{uuid}/gateway_tunnel", ""}
@@ -59,6 +58,7 @@ var (
EndpointContainerRequestGet = APIEndpoint{"GET", "arvados/v1/container_requests/{uuid}", ""}
EndpointContainerRequestList = APIEndpoint{"GET", "arvados/v1/container_requests", ""}
EndpointContainerRequestDelete = APIEndpoint{"DELETE", "arvados/v1/container_requests/{uuid}", ""}
+ EndpointContainerRequestLog = APIEndpoint{"GET", "arvados/v1/container_requests/{uuid}/log{path:|/.*}", ""}
EndpointGroupCreate = APIEndpoint{"POST", "arvados/v1/groups", "group"}
EndpointGroupUpdate = APIEndpoint{"PATCH", "arvados/v1/groups/{uuid}", "group"}
EndpointGroupGet = APIEndpoint{"GET", "arvados/v1/groups/{uuid}", ""}
@@ -285,7 +285,6 @@ type API interface {
ContainerDelete(ctx context.Context, options DeleteOptions) (Container, error)
ContainerLock(ctx context.Context, options GetOptions) (Container, error)
ContainerUnlock(ctx context.Context, options GetOptions) (Container, error)
- ContainerLog(ctx context.Context, options ContainerLogOptions) (http.Handler, error)
ContainerSSH(ctx context.Context, options ContainerSSHOptions) (ConnectionResponse, error)
ContainerGatewayTunnel(ctx context.Context, options ContainerGatewayTunnelOptions) (ConnectionResponse, error)
ContainerRequestCreate(ctx context.Context, options CreateOptions) (ContainerRequest, error)
@@ -293,6 +292,7 @@ type API interface {
ContainerRequestGet(ctx context.Context, options GetOptions) (ContainerRequest, error)
ContainerRequestList(ctx context.Context, options ListOptions) (ContainerRequestList, error)
ContainerRequestDelete(ctx context.Context, options DeleteOptions) (ContainerRequest, error)
+ ContainerRequestLog(ctx context.Context, options ContainerLogOptions) (http.Handler, error)
GroupCreate(ctx context.Context, options CreateOptions) (Group, error)
GroupUpdate(ctx context.Context, options UpdateOptions) (Group, error)
GroupGet(ctx context.Context, options GetOptions) (Group, error)
diff --git a/sdk/go/arvadostest/api.go b/sdk/go/arvadostest/api.go
index 483832de5..544d622f4 100644
--- a/sdk/go/arvadostest/api.go
+++ b/sdk/go/arvadostest/api.go
@@ -116,22 +116,6 @@ func (as *APIStub) ContainerUnlock(ctx context.Context, options arvados.GetOptio
as.appendCall(ctx, as.ContainerUnlock, options)
return arvados.Container{}, as.Error
}
-func (as *APIStub) ContainerLog(ctx context.Context, options arvados.ContainerLogOptions) (http.Handler, error) {
- as.appendCall(ctx, as.ContainerLog, options)
- // Return a handler that responds with the configured
- // error/success status.
- return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
- if as.Error == nil {
- w.WriteHeader(http.StatusOK)
- } else if err := httpserver.HTTPStatusError(nil); errors.As(as.Error, &err) {
- w.WriteHeader(err.HTTPStatus())
- io.WriteString(w, err.Error())
- } else {
- w.WriteHeader(http.StatusInternalServerError)
- io.WriteString(w, err.Error())
- }
- }), nil
-}
func (as *APIStub) ContainerSSH(ctx context.Context, options arvados.ContainerSSHOptions) (arvados.ConnectionResponse, error) {
as.appendCall(ctx, as.ContainerSSH, options)
return arvados.ConnectionResponse{}, as.Error
@@ -160,6 +144,22 @@ func (as *APIStub) ContainerRequestDelete(ctx context.Context, options arvados.D
as.appendCall(ctx, as.ContainerRequestDelete, options)
return arvados.ContainerRequest{}, as.Error
}
+func (as *APIStub) ContainerRequestLog(ctx context.Context, options arvados.ContainerLogOptions) (http.Handler, error) {
+ as.appendCall(ctx, as.ContainerRequestLog, options)
+ // Return a handler that responds with the configured
+ // error/success status.
+ return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ if as.Error == nil {
+ w.WriteHeader(http.StatusOK)
+ } else if err := httpserver.HTTPStatusError(nil); errors.As(as.Error, &err) {
+ w.WriteHeader(err.HTTPStatus())
+ io.WriteString(w, err.Error())
+ } else {
+ w.WriteHeader(http.StatusInternalServerError)
+ io.WriteString(w, err.Error())
+ }
+ }), nil
+}
func (as *APIStub) GroupCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Group, error) {
as.appendCall(ctx, as.GroupCreate, options)
return arvados.Group{}, as.Error
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 3b22a13b0..27981c487 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -360,6 +360,10 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
fsprefix = "by_id/" + collectionID + "/"
}
+ if src := r.Header.Get("X-Webdav-Source"); strings.HasPrefix(src, "/") && !strings.Contains(src, "//") && !strings.Contains(src, "/../") {
+ fsprefix += src[1:]
+ }
+
if tokens == nil {
tokens = reqTokens
if h.Cluster.Users.AnonymousUserToken != "" {
@@ -593,7 +597,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
},
LockSystem: webdavfs.NoLockSystem,
Logger: func(r *http.Request, err error) {
- if err != nil {
+ if err != nil && !os.IsNotExist(err) {
ctxlog.FromContext(r.Context()).WithError(err).Error("error reported by webdav handler")
}
},
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 9228c3628..c9b48f99a 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -93,6 +93,120 @@ func (s *UnitSuite) TestCORSPreflight(c *check.C) {
c.Check(resp.Code, check.Equals, http.StatusMethodNotAllowed)
}
+func (s *UnitSuite) TestWebdavPrefixAndSource(c *check.C) {
+ for _, trial := range []struct {
+ method string
+ path string
+ prefix string
+ source string
+ notFound bool
+ seeOther bool
+ }{
+ {
+ method: "PROPFIND",
+ path: "/",
+ },
+ {
+ method: "PROPFIND",
+ path: "/dir1",
+ },
+ {
+ method: "PROPFIND",
+ path: "/dir1/",
+ },
+ {
+ method: "PROPFIND",
+ path: "/dir1/foo",
+ prefix: "/dir1",
+ source: "/dir1",
+ },
+ {
+ method: "PROPFIND",
+ path: "/prefix/dir1/foo",
+ prefix: "/prefix/",
+ source: "",
+ },
+ {
+ method: "PROPFIND",
+ path: "/prefix/dir1/foo",
+ prefix: "/prefix",
+ source: "",
+ },
+ {
+ method: "PROPFIND",
+ path: "/prefix/dir1/foo",
+ prefix: "/prefix/",
+ source: "/",
+ },
+ {
+ method: "PROPFIND",
+ path: "/prefix/foo",
+ prefix: "/prefix/",
+ source: "/dir1/",
+ },
+ {
+ method: "GET",
+ path: "/prefix/foo",
+ prefix: "/prefix/",
+ source: "/dir1/",
+ },
+ {
+ method: "PROPFIND",
+ path: "/prefix/",
+ prefix: "/prefix",
+ source: "/dir1",
+ },
+ {
+ method: "PROPFIND",
+ path: "/prefix",
+ prefix: "/prefix",
+ source: "/dir1/",
+ },
+ {
+ method: "GET",
+ path: "/prefix",
+ prefix: "/prefix",
+ source: "/dir1",
+ seeOther: true,
+ },
+ {
+ method: "PROPFIND",
+ path: "/dir1/foo",
+ prefix: "",
+ source: "/dir1",
+ notFound: true,
+ },
+ } {
+ c.Logf("trial %+v", trial)
+ u := mustParseURL("http://" + arvadostest.FooBarDirCollection + ".keep-web.example" + trial.path)
+ req := &http.Request{
+ Method: trial.method,
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + arvadostest.ActiveTokenV2},
+ "X-Webdav-Prefix": {trial.prefix},
+ "X-Webdav-Source": {trial.source},
+ },
+ Body: ioutil.NopCloser(bytes.NewReader(nil)),
+ }
+
+ resp := httptest.NewRecorder()
+ s.handler.ServeHTTP(resp, req)
+ if trial.notFound {
+ c.Check(resp.Code, check.Equals, http.StatusNotFound)
+ } else if trial.method == "PROPFIND" {
+ c.Check(resp.Code, check.Equals, http.StatusMultiStatus)
+ c.Check(resp.Body.String(), check.Matches, `(?ms).*>\n?$`)
+ } else if trial.seeOther {
+ c.Check(resp.Code, check.Equals, http.StatusSeeOther)
+ } else {
+ c.Check(resp.Code, check.Equals, http.StatusOK)
+ }
+ }
+}
+
func (s *UnitSuite) TestEmptyResponse(c *check.C) {
for _, trial := range []struct {
dataExists bool
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list