[ARVADOS] updated: 2.1.0-298-g87694b32d
Git user
git at public.arvados.org
Tue Jan 26 20:28:21 UTC 2021
Summary of changes:
cmd/arvados-client/container_gateway.go | 26 ++++++++++++
doc/api/methods/containers.html.textile.liquid | 2 +
lib/config/config.default.yml | 20 +++++++++
lib/config/export.go | 3 ++
lib/config/generated_config.go | 20 +++++++++
lib/controller/localdb/container_gateway.go | 22 +++++++++-
lib/controller/localdb/container_gateway_test.go | 49 ++++++++++++++++++++++
sdk/go/arvados/config.go | 4 ++
sdk/go/arvados/container.go | 47 +++++++++++----------
sdk/python/tests/run_test_server.py | 4 ++
services/api/app/models/container.rb | 4 ++
...dd_interactive_session_started_to_containers.rb | 9 ++++
services/api/db/structure.sql | 6 ++-
services/api/test/unit/container_test.rb | 2 +
14 files changed, 191 insertions(+), 27 deletions(-)
create mode 100644 services/api/db/migrate/20210126183521_add_interactive_session_started_to_containers.rb
via 87694b32da9e3c8a92f7eedb5c71299d199023fb (commit)
via 82a9595a6a35c648fff62c2497d858f1ab062578 (commit)
via 0d4be3c57b4e6f5be9699cc9fe445c2d5af0174e (commit)
from b36492a0a569b9116ccf156430c901f4002d8814 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 87694b32da9e3c8a92f7eedb5c71299d199023fb
Author: Tom Clegg <tom at curii.com>
Date: Tue Jan 26 15:23:37 2021 -0500
17170: Attempt tunnel setup before exec()ing ssh client.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/cmd/arvados-client/container_gateway.go b/cmd/arvados-client/container_gateway.go
index a638000ae..2d6fd5eec 100644
--- a/cmd/arvados-client/container_gateway.go
+++ b/cmd/arvados-client/container_gateway.go
@@ -5,6 +5,7 @@
package main
import (
+ "bytes"
"context"
"flag"
"fmt"
@@ -52,6 +53,26 @@ Options:
}
sshargs := f.Args()[1:]
+ // Try setting up a tunnel, and exit right away if it
+ // fails. This tunnel won't get used -- we'll set up a new
+ // tunnel when running as SSH client's ProxyCommand child --
+ // but in most cases where the real tunnel setup would fail,
+ // we catch the problem earlier here. This makes it less
+ // likely that an error message about tunnel setup will get
+ // hidden behind noisy errors from SSH client like this:
+ //
+ // [useful tunnel setup error message here]
+ // kex_exchange_identification: Connection closed by remote host
+ // Connection closed by UNKNOWN port 65535
+ // exit status 255
+ exitcode := connectSSHCommand{}.RunCommand(
+ "arvados-client connect-ssh",
+ []string{"-detach-keys=" + *detachKeys, "-probe-only=true", target},
+ &bytes.Buffer{}, &bytes.Buffer{}, stderr)
+ if exitcode != 0 {
+ return exitcode
+ }
+
selfbin, err := os.Readlink("/proc/self/exe")
if err != nil {
fmt.Fprintln(stderr, err)
@@ -92,6 +113,7 @@ Options:
`)
f.PrintDefaults()
}
+ probeOnly := f.Bool("probe-only", false, "do not transfer IO, just exit 0 immediately if tunnel setup succeeds")
detachKeys := f.String("detach-keys", "", "set detach key sequence, as in docker-attach(1)")
if err := f.Parse(args); err != nil {
fmt.Fprintln(stderr, err)
@@ -148,6 +170,10 @@ Options:
}
defer sshconn.Conn.Close()
+ if *probeOnly {
+ return 0
+ }
+
ctx, cancel := context.WithCancel(context.Background())
go func() {
defer cancel()
commit 82a9595a6a35c648fff62c2497d858f1ab062578
Author: Tom Clegg <tom at curii.com>
Date: Tue Jan 26 14:53:16 2021 -0500
17170: Add interactive_session_started flag to containers.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/doc/api/methods/containers.html.textile.liquid b/doc/api/methods/containers.html.textile.liquid
index 8a7ebc36e..096a1fcaa 100644
--- a/doc/api/methods/containers.html.textile.liquid
+++ b/doc/api/methods/containers.html.textile.liquid
@@ -57,6 +57,8 @@ Generally this will contain additional keys that are not present in any correspo
|auth_uuid|string|UUID of a token to be passed into the container itself, used to access Keep-backed mounts, etc. Automatically assigned.|Null if state∉{"Locked","Running"} or if @runtime_token@ was provided.|
|locked_by_uuid|string|UUID of a token, indicating which dispatch process changed state to Locked. If null, any token can be used to lock. If not null, only the indicated token can modify this container.|Null if state∉{"Locked","Running"}|
|runtime_token|string|A v2 token to be passed into the container itself, used to access Keep-backed mounts, etc.|Not returned in API responses. Reset to null when state is "Complete" or "Cancelled".|
+|gateway_address|string|Address (host:port) of gateway server.|Internal use only.|
+|interactive_session_started|boolean|Indicates whether @arvados-client shell@ has been used to run commands in the container, which may have altered the container's behavior and output.||
h2(#container_states). Container states
diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go
index b8d85516a..e50927877 100644
--- a/lib/controller/localdb/container_gateway.go
+++ b/lib/controller/localdb/container_gateway.go
@@ -38,12 +38,12 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt
if err != nil {
return
}
+ ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
if !user.IsAdmin || !conn.cluster.Containers.ShellAccess.Admin {
if !conn.cluster.Containers.ShellAccess.User {
err = httpserver.ErrorWithStatus(errors.New("shell access is disabled in config"), http.StatusServiceUnavailable)
return
}
- ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
var crs arvados.ContainerRequestList
crs, err = conn.railsProxy.ContainerRequestList(ctxRoot, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"container_uuid", "=", opts.UUID}}})
if err != nil {
@@ -153,6 +153,20 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt
netconn.Close()
return
}
+
+ if !ctr.InteractiveSessionStarted {
+ _, err = conn.railsProxy.ContainerUpdate(ctxRoot, arvados.UpdateOptions{
+ UUID: opts.UUID,
+ Attrs: map[string]interface{}{
+ "interactive_session_started": true,
+ },
+ })
+ if err != nil {
+ netconn.Close()
+ return
+ }
+ }
+
sshconn.Conn = netconn
sshconn.Bufrw = &bufio.ReadWriter{Reader: bufr, Writer: bufw}
sshconn.Logger = ctxlog.FromContext(ctx)
diff --git a/lib/controller/localdb/container_gateway_test.go b/lib/controller/localdb/container_gateway_test.go
index 54c1b2149..aff569b09 100644
--- a/lib/controller/localdb/container_gateway_test.go
+++ b/lib/controller/localdb/container_gateway_test.go
@@ -77,6 +77,8 @@ func (s *ContainerGatewaySuite) SetUpSuite(c *check.C) {
func (s *ContainerGatewaySuite) SetUpTest(c *check.C) {
s.cluster.Containers.ShellAccess.Admin = true
s.cluster.Containers.ShellAccess.User = true
+ _, err := arvadostest.DB(c, s.cluster).Exec(`update containers set interactive_session_started=$1 where uuid=$2`, false, s.ctrUUID)
+ c.Check(err, check.IsNil)
}
func (s *ContainerGatewaySuite) TestConfig(c *check.C) {
@@ -152,6 +154,9 @@ func (s *ContainerGatewaySuite) TestConnect(c *check.C) {
case <-time.After(time.Second):
c.Fail()
}
+ ctr, err := s.localdb.ContainerGet(s.ctx, arvados.GetOptions{UUID: s.ctrUUID})
+ c.Check(err, check.IsNil)
+ c.Check(ctr.InteractiveSessionStarted, check.Equals, true)
}
func (s *ContainerGatewaySuite) TestConnectFail(c *check.C) {
diff --git a/sdk/go/arvados/container.go b/sdk/go/arvados/container.go
index 4d1511b41..86b7c8684 100644
--- a/sdk/go/arvados/container.go
+++ b/sdk/go/arvados/container.go
@@ -8,29 +8,30 @@ import "time"
// Container is an arvados#container resource.
type Container struct {
- UUID string `json:"uuid"`
- Etag string `json:"etag"`
- CreatedAt time.Time `json:"created_at"`
- ModifiedByClientUUID string `json:"modified_by_client_uuid"`
- ModifiedByUserUUID string `json:"modified_by_user_uuid"`
- ModifiedAt time.Time `json:"modified_at"`
- Command []string `json:"command"`
- ContainerImage string `json:"container_image"`
- Cwd string `json:"cwd"`
- Environment map[string]string `json:"environment"`
- LockedByUUID string `json:"locked_by_uuid"`
- Mounts map[string]Mount `json:"mounts"`
- Output string `json:"output"`
- OutputPath string `json:"output_path"`
- Priority int64 `json:"priority"`
- RuntimeConstraints RuntimeConstraints `json:"runtime_constraints"`
- State ContainerState `json:"state"`
- SchedulingParameters SchedulingParameters `json:"scheduling_parameters"`
- ExitCode int `json:"exit_code"`
- RuntimeStatus map[string]interface{} `json:"runtime_status"`
- StartedAt *time.Time `json:"started_at"` // nil if not yet started
- FinishedAt *time.Time `json:"finished_at"` // nil if not yet finished
- GatewayAddress string `json:"gateway_address"`
+ UUID string `json:"uuid"`
+ Etag string `json:"etag"`
+ CreatedAt time.Time `json:"created_at"`
+ ModifiedByClientUUID string `json:"modified_by_client_uuid"`
+ ModifiedByUserUUID string `json:"modified_by_user_uuid"`
+ ModifiedAt time.Time `json:"modified_at"`
+ Command []string `json:"command"`
+ ContainerImage string `json:"container_image"`
+ Cwd string `json:"cwd"`
+ Environment map[string]string `json:"environment"`
+ LockedByUUID string `json:"locked_by_uuid"`
+ Mounts map[string]Mount `json:"mounts"`
+ Output string `json:"output"`
+ OutputPath string `json:"output_path"`
+ Priority int64 `json:"priority"`
+ RuntimeConstraints RuntimeConstraints `json:"runtime_constraints"`
+ State ContainerState `json:"state"`
+ SchedulingParameters SchedulingParameters `json:"scheduling_parameters"`
+ ExitCode int `json:"exit_code"`
+ RuntimeStatus map[string]interface{} `json:"runtime_status"`
+ StartedAt *time.Time `json:"started_at"` // nil if not yet started
+ FinishedAt *time.Time `json:"finished_at"` // nil if not yet finished
+ GatewayAddress string `json:"gateway_address"`
+ InteractiveSessionStarted bool `json:"interactive_session_started"`
}
// ContainerRequest is an arvados#container_request resource.
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 52fc79e2c..49be3df55 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -77,6 +77,7 @@ class Container < ArvadosModel
t.add :runtime_auth_scopes
t.add :lock_count
t.add :gateway_address
+ t.add :interactive_session_started
end
# Supported states for a container
@@ -481,6 +482,9 @@ class Container < ArvadosModel
if self.state_changed?
permitted.push :started_at, :gateway_address
end
+ if !self.interactive_session_started_was
+ permitted.push :interactive_session_started
+ end
when Complete
if self.state_was == Running
diff --git a/services/api/db/migrate/20210126183521_add_interactive_session_started_to_containers.rb b/services/api/db/migrate/20210126183521_add_interactive_session_started_to_containers.rb
new file mode 100644
index 000000000..3fe23f64d
--- /dev/null
+++ b/services/api/db/migrate/20210126183521_add_interactive_session_started_to_containers.rb
@@ -0,0 +1,9 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+class AddInteractiveSessionStartedToContainers < ActiveRecord::Migration[5.2]
+ def change
+ add_column :containers, :interactive_session_started, :boolean, null: false, default: false
+ end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 249ec67ac..14eca609e 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -522,7 +522,8 @@ CREATE TABLE public.containers (
runtime_auth_scopes jsonb,
runtime_token text,
lock_count integer DEFAULT 0 NOT NULL,
- gateway_address character varying
+ gateway_address character varying,
+ interactive_session_started boolean DEFAULT false NOT NULL
);
@@ -3189,6 +3190,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20201103170213'),
('20201105190435'),
('20201202174753'),
-('20210108033940');
+('20210108033940'),
+('20210126183521');
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 98e60e057..7853b6f6a 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -797,6 +797,8 @@ class ContainerTest < ActiveSupport::TestCase
[Container::Running, {priority: 123456789}],
[Container::Running, {runtime_status: {'error' => 'oops'}}],
[Container::Running, {cwd: '/'}],
+ [Container::Running, {gateway_address: "172.16.0.1:12345"}],
+ [Container::Running, {interactive_session_started: true}],
[Container::Complete, {state: Container::Cancelled}],
[Container::Complete, {priority: 123456789}],
[Container::Complete, {runtime_status: {'error' => 'oops'}}],
commit 0d4be3c57b4e6f5be9699cc9fe445c2d5af0174e
Author: Tom Clegg <tom at curii.com>
Date: Tue Jan 26 11:25:48 2021 -0500
17170: Add config keys to enable shell access for users/admins.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 2aa53a432..7748ea4aa 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -870,6 +870,26 @@ Clusters:
# period.
LogUpdateSize: 32MiB
+ ShellAccess:
+ # An admin user can use "arvados-client shell" to start an
+ # interactive shell (with any user ID) in any running
+ # container.
+ Admin: false
+
+ # Any user can use "arvados-client shell" to start an
+ # interactive shell (with any user ID) in any running
+ # container that they started, provided it isn't also
+ # associated with a different user's container request.
+ #
+ # Interactive sessions make it easy to alter the container's
+ # runtime environment in ways that aren't recorded or
+ # reproducible. Consider the implications for automatic
+ # container reuse before enabling and using this feature. In
+ # particular, note that starting an interactive session does
+ # not disqualify a container from being reused by a different
+ # user/workflow in the future.
+ User: false
+
SLURM:
PrioritySpread: 0
SbatchArgumentsList: []
diff --git a/lib/config/export.go b/lib/config/export.go
index e4917032f..2d666e638 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -121,6 +121,9 @@ var whitelist = map[string]bool{
"Containers.MaxRetryAttempts": true,
"Containers.MinRetryPeriod": true,
"Containers.ReserveExtraRAM": true,
+ "Containers.ShellAccess": true,
+ "Containers.ShellAccess.Admin": true,
+ "Containers.ShellAccess.User": true,
"Containers.SLURM": false,
"Containers.StaleLockTimeout": false,
"Containers.SupportedDockerImageFormats": true,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 34f0a0c92..fd5723b13 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -876,6 +876,26 @@ Clusters:
# period.
LogUpdateSize: 32MiB
+ ShellAccess:
+ # An admin user can use "arvados-client shell" to start an
+ # interactive shell (with any user ID) in any running
+ # container.
+ Admin: false
+
+ # Any user can use "arvados-client shell" to start an
+ # interactive shell (with any user ID) in any running
+ # container that they started, provided it isn't also
+ # associated with a different user's container request.
+ #
+ # Interactive sessions make it easy to alter the container's
+ # runtime environment in ways that aren't recorded or
+ # reproducible. Consider the implications for automatic
+ # container reuse before enabling and using this feature. In
+ # particular, note that starting an interactive session does
+ # not disqualify a container from being reused by a different
+ # user/workflow in the future.
+ User: false
+
SLURM:
PrioritySpread: 0
SbatchArgumentsList: []
diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go
index ca968cf20..b8d85516a 100644
--- a/lib/controller/localdb/container_gateway.go
+++ b/lib/controller/localdb/container_gateway.go
@@ -38,7 +38,11 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt
if err != nil {
return
}
- if !user.IsAdmin {
+ if !user.IsAdmin || !conn.cluster.Containers.ShellAccess.Admin {
+ if !conn.cluster.Containers.ShellAccess.User {
+ err = httpserver.ErrorWithStatus(errors.New("shell access is disabled in config"), http.StatusServiceUnavailable)
+ return
+ }
ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
var crs arvados.ContainerRequestList
crs, err = conn.railsProxy.ContainerRequestList(ctxRoot, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"container_uuid", "=", opts.UUID}}})
diff --git a/lib/controller/localdb/container_gateway_test.go b/lib/controller/localdb/container_gateway_test.go
index 336d5b634..54c1b2149 100644
--- a/lib/controller/localdb/container_gateway_test.go
+++ b/lib/controller/localdb/container_gateway_test.go
@@ -74,6 +74,50 @@ func (s *ContainerGatewaySuite) SetUpSuite(c *check.C) {
c.Assert(err, check.IsNil)
}
+func (s *ContainerGatewaySuite) SetUpTest(c *check.C) {
+ s.cluster.Containers.ShellAccess.Admin = true
+ s.cluster.Containers.ShellAccess.User = true
+}
+
+func (s *ContainerGatewaySuite) TestConfig(c *check.C) {
+ for _, trial := range []struct {
+ configAdmin bool
+ configUser bool
+ sendToken string
+ errorCode int
+ }{
+ {true, true, arvadostest.ActiveTokenV2, 0},
+ {true, false, arvadostest.ActiveTokenV2, 503},
+ {false, true, arvadostest.ActiveTokenV2, 0},
+ {false, false, arvadostest.ActiveTokenV2, 503},
+ {true, true, arvadostest.AdminToken, 0},
+ {true, false, arvadostest.AdminToken, 0},
+ {false, true, arvadostest.AdminToken, 403},
+ {false, false, arvadostest.AdminToken, 503},
+ } {
+ c.Logf("trial %#v", trial)
+ s.cluster.Containers.ShellAccess.Admin = trial.configAdmin
+ s.cluster.Containers.ShellAccess.User = trial.configUser
+ ctx := auth.NewContext(s.ctx, &auth.Credentials{Tokens: []string{trial.sendToken}})
+ sshconn, err := s.localdb.ContainerSSH(ctx, arvados.ContainerSSHOptions{UUID: s.ctrUUID})
+ if trial.errorCode == 0 {
+ if !c.Check(err, check.IsNil) {
+ continue
+ }
+ if !c.Check(sshconn.Conn, check.NotNil) {
+ continue
+ }
+ sshconn.Conn.Close()
+ } else {
+ c.Check(err, check.NotNil)
+ err, ok := err.(interface{ HTTPStatus() int })
+ if c.Check(ok, check.Equals, true) {
+ c.Check(err.HTTPStatus(), check.Equals, trial.errorCode)
+ }
+ }
+ }
+}
+
func (s *ContainerGatewaySuite) TestConnect(c *check.C) {
c.Logf("connecting to %s", s.gw.Address)
sshconn, err := s.localdb.ContainerSSH(s.ctx, arvados.ContainerSSHOptions{UUID: s.ctrUUID})
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 9dc9e17dd..8222035a3 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -429,6 +429,10 @@ type ContainersConfig struct {
LogUpdatePeriod Duration
LogUpdateSize ByteSize
}
+ ShellAccess struct {
+ Admin bool
+ User bool
+ }
SLURM struct {
PrioritySpread int64
SbatchArgumentsList []string
diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 6a0f7d9f4..953021f0e 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -802,6 +802,10 @@ def setup_config():
"GitInternalDir": os.path.join(SERVICES_SRC_DIR, 'api', 'tmp', 'internal.git'),
},
"SupportedDockerImageFormats": {"v1": {}},
+ "ShellAccess": {
+ "Admin": True,
+ "User": True,
+ },
},
"Volumes": {
"zzzzz-nyw5e-%015d"%n: {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list