From df42b08093c924e65da094864a0451b8b70a8258 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Wed, 14 Apr 2021 17:10:02 -0300 Subject: [PATCH 01/16] fix(database): don't enforce client encoding --- arvados/files/default/config.tmpl.jinja | 1 - pillar.example | 3 +-- test/integration/api/controls/config_spec.rb | 15 +++++++++++++++ test/salt/pillar/arvados.sls | 5 ++--- test/salt/pillar/arvados_dev.sls | 4 ++-- 5 files changed, 20 insertions(+), 8 deletions(-) diff --git a/arvados/files/default/config.tmpl.jinja b/arvados/files/default/config.tmpl.jinja index c52186e..9c9db2f 100644 --- a/arvados/files/default/config.tmpl.jinja +++ b/arvados/files/default/config.tmpl.jinja @@ -73,7 +73,6 @@ Clusters: host: {{ arvados.cluster.database.host }} password: {{ arvados.cluster.database.password | yaml_encode }} user: {{ arvados.cluster.database.user }} - client_encoding: {{ arvados.cluster.database.client_encoding }} {%- if arvados.cluster.database.extra_conn_params is defined %} {{ arvados.cluster.database.extra_conn_params | yaml(False) | indent(8) }} {%- endif %} diff --git a/pillar.example b/pillar.example index 4ff35cd..233b492 100644 --- a/pillar.example +++ b/pillar.example @@ -65,13 +65,12 @@ arvados: host: 127.0.0.1 password: changeme_arvados user: arvados - encoding: en_US.utf8 - client_encoding: UTF8 # You can pass extra database connections parameters here, # which will be rendered as yaml. # extra_conn_params: # sslmode: prefer # verify-ca: false + # client_encoding: UTF8 tls: diff --git a/test/integration/api/controls/config_spec.rb b/test/integration/api/controls/config_spec.rb index 57fffc1..0f64cb2 100644 --- a/test/integration/api/controls/config_spec.rb +++ b/test/integration/api/controls/config_spec.rb @@ -10,6 +10,20 @@ rails_stanza = <<-RAILS_STANZA http://api.internal:8004: {} RAILS_STANZA +database_stanza = <<-DATABASE_STANZA + ### DATABASE CONFIGURATION + PostgreSQL: + ConnectionPool: 32 + Connection: + # All parameters here are passed to the PG client library in a connection string; + # see https://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-PARAMKEYWORDS + dbname: arvados + host: 127.0.0.1 + password: "changeme_arvados" + user: arvados + client_encoding: UTF8 +DATABASE_STANZA + group = case os[:name] when 'centos' 'nginx' @@ -32,5 +46,6 @@ control 'arvados configuration' do end its('content') { should include(api_stanza) } its('content') { should include(rails_stanza) } + its('content') { should include(database_stanza) } end end diff --git a/test/salt/pillar/arvados.sls b/test/salt/pillar/arvados.sls index 73efb61..a063823 100644 --- a/test/salt/pillar/arvados.sls +++ b/test/salt/pillar/arvados.sls @@ -39,14 +39,13 @@ arvados: host: 127.0.0.1 password: changeme_arvados user: arvados - encoding: en_US.utf8 - client_encoding: UTF8 + extra_conn_params: + client_encoding: UTF8 # Centos7 does not enable SSL by default, so we disable # it here just for testing of the formula purposes only. # You should not do this in production, and should # configure Postgres certificates correctly {%- if grains.os_family in ('RedHat',) %} - extra_conn_params: sslmode: disable {%- endif %} diff --git a/test/salt/pillar/arvados_dev.sls b/test/salt/pillar/arvados_dev.sls index e1c42cd..35a5bff 100644 --- a/test/salt/pillar/arvados_dev.sls +++ b/test/salt/pillar/arvados_dev.sls @@ -65,8 +65,8 @@ arvados: host: 127.0.0.1 password: changeme_arvados user: arvados - encoding: en_US.utf8 - client_encoding: UTF8 + extra_conn_params: + client_encoding: UTF8 tls: # certificate: '' -- 2.30.2 From 5ce013435374b9697cb6d6a3fc3a3c7f35ae3279 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Wed, 14 Apr 2021 17:20:13 -0300 Subject: [PATCH 02/16] test(dependencies): add ssl certs dependencies so nginx does not fail --- kitchen.yml | 4 ++-- test/salt/states/examples/single_host/host_entries.sls | 5 +++++ test/salt/states/examples/single_host/snakeoil_certs.sls | 9 +++++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/kitchen.yml b/kitchen.yml index d0f50c5..e47ea24 100644 --- a/kitchen.yml +++ b/kitchen.yml @@ -236,7 +236,7 @@ suites: source: https://github.com/saltstack-formulas/postgres-formula.git - name: nginx repo: git - source: https://github.com/netmanagers/nginx-formula.git + source: https://github.com/saltstack-formulas/nginx-formula.git state_top: base: '*': @@ -297,7 +297,7 @@ suites: path: test/salt/states/examples - name: nginx repo: git - source: https://github.com/netmanagers/nginx-formula.git + source: https://github.com/saltstack-formulas/nginx-formula.git state_top: base: '*': diff --git a/test/salt/states/examples/single_host/host_entries.sls b/test/salt/states/examples/single_host/host_entries.sls index 855757e..177ad18 100644 --- a/test/salt/states/examples/single_host/host_entries.sls +++ b/test/salt/states/examples/single_host/host_entries.sls @@ -3,6 +3,9 @@ {%- from "arvados/map.jinja" import arvados with context %} {%- set tpldir = curr_tpldir %} +include: + - nginx.config + arvados_test_salt_states_examples_single_host_etc_hosts_host_present: host.present: - ip: {{ grains.get('ipv4')[0] }} @@ -26,3 +29,5 @@ arvados_test_salt_states_examples_single_host_etc_hosts_host_present: - {{ entry }}.internal - {{ entry }}.{{ arvados.cluster.name }}.{{ arvados.cluster.domain }} {%- endfor %} + - require_in: + - file: nginx_config diff --git a/test/salt/states/examples/single_host/snakeoil_certs.sls b/test/salt/states/examples/single_host/snakeoil_certs.sls index f6e69b8..6f4e932 100644 --- a/test/salt/states/examples/single_host/snakeoil_certs.sls +++ b/test/salt/states/examples/single_host/snakeoil_certs.sls @@ -4,6 +4,8 @@ {%- set tpldir = curr_tpldir %} include: + - nginx.passenger + - nginx.config - nginx.service {%- set arvados_ca_cert_file = '/etc/ssl/certs/arvados-snakeoil-ca.pem' %} @@ -145,5 +147,8 @@ arvados_test_salt_states_examples_single_host_snakeoil_certs_nginx_snakeoil_file ssl_certificate_key {{ arvados_key_file }}; - watch_in: - service: nginx_service - - + - require: + - pkg: passenger_install + - cmd: arvados_test_salt_states_examples_single_host_snakeoil_certs_certs_permissions_cmd_run + - require_in: + - file: nginx_config -- 2.30.2 From ff7ed36638e4985336ff4fe0fa483312b057f198 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Fri, 16 Apr 2021 17:54:14 -0300 Subject: [PATCH 03/16] test(dependencies): add hosts entries for testing --- test/salt/states/examples/single_host/host_entries.sls | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/salt/states/examples/single_host/host_entries.sls b/test/salt/states/examples/single_host/host_entries.sls index 177ad18..fc35678 100644 --- a/test/salt/states/examples/single_host/host_entries.sls +++ b/test/salt/states/examples/single_host/host_entries.sls @@ -3,9 +3,6 @@ {%- from "arvados/map.jinja" import arvados with context %} {%- set tpldir = curr_tpldir %} -include: - - nginx.config - arvados_test_salt_states_examples_single_host_etc_hosts_host_present: host.present: - ip: {{ grains.get('ipv4')[0] }} @@ -19,6 +16,7 @@ arvados_test_salt_states_examples_single_host_etc_hosts_host_present: 'controller', 'download', 'keep', + 'keepweb', 'keep0', 'shell', 'workbench', @@ -26,8 +24,10 @@ arvados_test_salt_states_examples_single_host_etc_hosts_host_present: 'ws', ] %} + - {{ entry }} - {{ entry }}.internal - {{ entry }}.{{ arvados.cluster.name }}.{{ arvados.cluster.domain }} {%- endfor %} - require_in: - file: nginx_config + - service: nginx_service -- 2.30.2 From 4871254408f8a9330de08fde2c21cb60ae1e5aa6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Thu, 29 Apr 2021 19:56:55 -0300 Subject: [PATCH 04/16] fix(config): ensure AnonymousUserToken is set * added tests to verify it's set either in the `tokens` or the `Users` dicts --- arvados/config/file.sls | 2 ++ kitchen.yml | 5 ++--- pillar.example | 2 ++ test/integration/api/controls/config_spec.rb | 6 ++++++ test/integration/keepstore/controls/config_spec.rb | 6 ++++++ test/integration/shell/controls/config_spec.rb | 8 ++++++++ test/salt/pillar/arvados.sls | 2 +- test/salt/pillar/arvados_dev.sls | 9 ++++++++- 8 files changed, 35 insertions(+), 5 deletions(-) diff --git a/arvados/config/file.sls b/arvados/config/file.sls index 5a9c1f0..fb9ab67 100644 --- a/arvados/config/file.sls +++ b/arvados/config/file.sls @@ -6,6 +6,8 @@ {%- from tplroot ~ "/map.jinja" import arvados with context %} {%- from tplroot ~ "/libtofs.jinja" import files_switch with context %} +{%- do arvados.cluster.Users.update({'AnonymousUserToken': arvados.cluster.tokens.anonymous_user }) if arvados.cluster.Users.AnonymousUserToken is not defined %} + include: - .package diff --git a/kitchen.yml b/kitchen.yml index e47ea24..cb35319 100644 --- a/kitchen.yml +++ b/kitchen.yml @@ -325,6 +325,7 @@ suites: # yamllint enable rule:line-length verifier: inspec_tests: + - path: test/integration/repo - path: test/integration/workbench - path: test/integration/workbench2 #### shell @@ -360,15 +361,13 @@ suites: - arvados._mapdata - arvados.repo - arvados.keepstore - # - arvados.clean pillars: top.sls: base: '*': - arvados pillars_from_files: - arvados.sls: test/salt/pillar/arvados.sls + arvados.sls: test/salt/pillar/arvados_dev.sls verifier: inspec_tests: - - path: test/integration/repo - path: test/integration/keepstore diff --git a/pillar.example b/pillar.example index 233b492..b8acf45 100644 --- a/pillar.example +++ b/pillar.example @@ -86,6 +86,8 @@ arvados: # See https://dev.arvados.org/issues/17150 system_root: changemesystemroottoken management: changememanagementtoken + # The AnonymousUserToken can be set here or in the + # USers dictionary below. The latter will be used if set. anonymous_user: changemeanonymoususertoken ### KEYS diff --git a/test/integration/api/controls/config_spec.rb b/test/integration/api/controls/config_spec.rb index 0f64cb2..7131096 100644 --- a/test/integration/api/controls/config_spec.rb +++ b/test/integration/api/controls/config_spec.rb @@ -1,5 +1,10 @@ # frozen_string_literal: true +users_stanza = <<-USERS_STANZA + Users: + AnonymousUserToken: anonymoususertokensetinthetokensdict +USERS_STANZA + api_stanza = <<-API_STANZA API: API_STANZA @@ -47,5 +52,6 @@ control 'arvados configuration' do its('content') { should include(api_stanza) } its('content') { should include(rails_stanza) } its('content') { should include(database_stanza) } + its('content') { should include(users_stanza) } end end diff --git a/test/integration/keepstore/controls/config_spec.rb b/test/integration/keepstore/controls/config_spec.rb index d44e673..7113e2b 100644 --- a/test/integration/keepstore/controls/config_spec.rb +++ b/test/integration/keepstore/controls/config_spec.rb @@ -1,5 +1,10 @@ # frozen_string_literal: true +users_stanza = <<-USERS_STANZA + Users: + AnonymousUserToken: anonymoususertokensetintheusersdict +USERS_STANZA + keepstore_stanza = <<-KEEPSTORE_STANZA Keepstore: InternalURLs: @@ -33,5 +38,6 @@ control 'arvados configuration' do end its('content') { should include(keepstore_stanza) } its('content') { should include(volumes_stanza) } + its('content') { should include(users_stanza) } end end diff --git a/test/integration/shell/controls/config_spec.rb b/test/integration/shell/controls/config_spec.rb index 603e337..f0e8a7e 100644 --- a/test/integration/shell/controls/config_spec.rb +++ b/test/integration/shell/controls/config_spec.rb @@ -1,5 +1,13 @@ # frozen_string_literal: true +control 'arvados configuration file' do + title 'should not exist' + + describe file('/etc/arvados/config.yml') do + it { should_not exist} + end +end + control 'shellinabox configuration' do title 'should match desired lines' diff --git a/test/salt/pillar/arvados.sls b/test/salt/pillar/arvados.sls index a063823..635b894 100644 --- a/test/salt/pillar/arvados.sls +++ b/test/salt/pillar/arvados.sls @@ -59,7 +59,7 @@ arvados: tokens: system_root: changemesystemroottoken management: changememanagementtoken - anonymous_user: changemeanonymoususertoken + anonymous_user: anonymoususertokensetinthetokensdict ### KEYS secrets: diff --git a/test/salt/pillar/arvados_dev.sls b/test/salt/pillar/arvados_dev.sls index 35a5bff..1dcc78b 100644 --- a/test/salt/pillar/arvados_dev.sls +++ b/test/salt/pillar/arvados_dev.sls @@ -67,6 +67,13 @@ arvados: user: arvados extra_conn_params: client_encoding: UTF8 + # Centos7 does not enable SSL by default, so we disable + # it here just for testing of the formula purposes only. + # You should not do this in production, and should + # configure Postgres certificates correctly + {%- if grains.os_family in ('RedHat',) %} + sslmode: disable + {%- endif %} tls: # certificate: '' @@ -78,7 +85,6 @@ arvados: tokens: system_root: changemesystemroottoken management: changememanagementtoken - anonymous_user: changemeanonymoususertoken ### KEYS secrets: @@ -108,6 +114,7 @@ arvados: Root: /tmp Users: + AnonymousUserToken: anonymoususertokensetintheusersdict NewUsersAreActive: true AutoAdminFirstUser: true AutoSetupNewUsers: true -- 2.30.2 From ee822734b85d93c7f5aedf25abbb8eb9c0baf46c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Mon, 3 May 2021 19:05:19 -0300 Subject: [PATCH 05/16] feat(config): enable strict config-check before deploying * modify pillars and examples to pass config-check BREAKING CHANGE: the configuration file now is checked before deployment to make sure it's valid. As keys and tokens now are checked to make sure they comply with Arvados' requirements, old configurations might fail to deploy --- arvados/config/file.sls | 2 +- arvados/defaults.yaml | 3 +- arvados/files/default/config.tmpl.jinja | 2 - pillar.example | 65 +++++++++++-------- .../workbench/controls/config_spec.rb | 2 +- test/salt/pillar/arvados.sls | 12 ++-- test/salt/pillar/arvados_dev.sls | 12 ++-- 7 files changed, 49 insertions(+), 49 deletions(-) diff --git a/arvados/config/file.sls b/arvados/config/file.sls index fb9ab67..4d99721 100644 --- a/arvados/config/file.sls +++ b/arvados/config/file.sls @@ -25,6 +25,6 @@ arvados-config-file-file-managed: - template: jinja - context: arvados: {{ arvados | json }} - - check_cmd: /usr/bin/arvados-server config-dump -config + - check_cmd: {{ arvados.config.check_command }} - require: - pkg: arvados-config-package-install-pkg-installed diff --git a/arvados/defaults.yaml b/arvados/defaults.yaml index da441d0..d737ee4 100644 --- a/arvados/defaults.yaml +++ b/arvados/defaults.yaml @@ -37,14 +37,13 @@ arvados: user: root group: root mode: 640 + check_command: /usr/bin/arvados-server config-check -config # Experimental feature # only available when 'release: development' auto_reload_config: false cluster: - force_legacy_api14: false - database: connection_pool_max: 32 diff --git a/arvados/files/default/config.tmpl.jinja b/arvados/files/default/config.tmpl.jinja index 9c9db2f..017c672 100644 --- a/arvados/files/default/config.tmpl.jinja +++ b/arvados/files/default/config.tmpl.jinja @@ -19,8 +19,6 @@ Clusters: SystemRootToken: {{ arvados.cluster.tokens.system_root | yaml_encode }} ManagementToken: {{ arvados.cluster.tokens.management | yaml_encode }} - ForceLegacyAPI14: {{ arvados.cluster.force_legacy_api14 }} - API: {%- if 'API' in arvados.cluster %} {{ arvados.cluster.API | default('') | yaml(False) | indent(6) }} diff --git a/pillar.example b/pillar.example index b8acf45..4105b66 100644 --- a/pillar.example +++ b/pillar.example @@ -8,23 +8,23 @@ arvados: ### GENERAL CONFIG # version: '2.1.0' # release: production - ## It makes little sense to disable this flag, but you can, if you want :) + ### It makes little sense to disable this flag, but you can, if you want :) # use_upstream_repo: true - ## Repo URL is built with grains values. If desired, it can be completely - ## overwritten with the pillar parameter 'repo_url' + ### Repo URL is built with grains values. If desired, it can be completely + ### overwritten with the pillar parameter 'repo_url' # repo: # humanname: Arvados Official Repository - ## IMPORTANT!!!!! - ## api, workbench and shell require some gems, so you need to make sure ruby - ## and deps are installed in order to install and compile the gems. - ## We default to `false` in these two variables as it's expected you already - ## manage OS packages with some other tool and you don't want us messing up - ## with your setup. + # IMPORTANT!!!!! + # api, workbench and shell require some gems, so you need to make sure ruby + # and deps are installed in order to install and compile the gems. + # We default to `false` in these two variables as it's expected you already + # manage OS packages with some other tool and you don't want us messing up + # with your setup. ruby: - ## We set these to `true` here for testing purposes. - ## They both default to `false`. + # We set these to `true` here for testing purposes. + # They both default to `false`. manage_ruby: true use_rvm: false # If you want to use rvm. Defaults to true for centos-7 # pkg: ruby # Can specify a version like ruby-2.5.7 for rvm @@ -47,11 +47,26 @@ arvados: # config: # file: /etc/arvados/config.yml # user: root - ## IMPORTANT!!!!! - ## If you're intalling any of the rails apps (api, workbench), the group - ## should be set to that of the web server, usually `www-data` + ### IMPORTANT!!!!! + ### If you're intalling any of the rails apps (api, workbench), the group + ### should be set to that of the web server, usually `www-data` # group: root # mode: 640 + # + ### This is the command run to verify the configuration is correct before + ### deploying it. By default it uses `-strict=true`, so it will error on + ### warnings (ie, unknown/deprecated parameters) + # + # check_command: /usr/bin/arvados-server config-check -config + # + ### To fail only on errors, you can use + # + # check_command: /usr/bin/arvados-server config-check -strict=false -config + # + ### and to disable configuration checking (not recommended), just set it to + ### any command that returns true + # + # check_command: /bin/true ### ARVADOS CLUSTER CONFIG cluster: @@ -68,9 +83,9 @@ arvados: # You can pass extra database connections parameters here, # which will be rendered as yaml. # extra_conn_params: - # sslmode: prefer - # verify-ca: false - # client_encoding: UTF8 + # sslmode: prefer + # verify-ca: false + # client_encoding: UTF8 tls: @@ -84,25 +99,21 @@ arvados: # Secrets and tokens have to be +32 alphanumeric, # it does not accept underscores or special characters. # See https://dev.arvados.org/issues/17150 - system_root: changemesystemroottoken - management: changememanagementtoken + system_root: systemroottokenmushaveatleast32characters + management: managementtokenmushaveatleast32characters # The AnonymousUserToken can be set here or in the - # USers dictionary below. The latter will be used if set. - anonymous_user: changemeanonymoususertoken + # Users dictionary below. The latter will be used if set. + anonymous_user: anonymoususertokenmushaveatleast32characters ### KEYS secrets: - blob_signing_key: changemeblobsigningkey - workbench_secret_key: changemeworkbenchsecretkey + blob_signing_key: blobsigningkeymushaveatleast32characters + workbench_secret_key: workbenchsecretkeymushaveatleast32characters dispatcher_access_key: changemedispatcheraccesskey dispatcher_secret_key: changemedispatchersecretkey keep_access_key: changemekeepaccesskey keep_secret_key: changemekeepsecretkey - AuditLogs: - Section_to_ignore: - - some_random_value - ### VOLUMES ## This should usually match all your `keepstore` instances Volumes: diff --git a/test/integration/workbench/controls/config_spec.rb b/test/integration/workbench/controls/config_spec.rb index 9a14383..8e33e84 100644 --- a/test/integration/workbench/controls/config_spec.rb +++ b/test/integration/workbench/controls/config_spec.rb @@ -2,7 +2,7 @@ workbench_config = <<-WORKBENCH_STANZA Workbench: - SecretKeyBase: "changemeworkbenchsecretkey" + SecretKeyBase: "workbenchsecretkeymushaveatleast32characters" SiteName: FIXME WORKBENCH_STANZA diff --git a/test/salt/pillar/arvados.sls b/test/salt/pillar/arvados.sls index 635b894..d8117c6 100644 --- a/test/salt/pillar/arvados.sls +++ b/test/salt/pillar/arvados.sls @@ -57,23 +57,19 @@ arvados: ### TOKENS tokens: - system_root: changemesystemroottoken - management: changememanagementtoken + system_root: systemroottokenmushaveatleast32characters + management: managementtokenmushaveatleast32characters anonymous_user: anonymoususertokensetinthetokensdict ### KEYS secrets: - blob_signing_key: changemeblobsigningkey - workbench_secret_key: changemeworkbenchsecretkey + blob_signing_key: blobsigningkeymushaveatleast32characters + workbench_secret_key: workbenchsecretkeymushaveatleast32characters dispatcher_access_key: changemedispatcheraccesskey dispatcher_secret_key: changemedispatchersecretkey keep_access_key: changemekeepaccesskey keep_secret_key: changemekeepsecretkey - AuditLogs: - Section_to_ignore: - - some_random_value - ### VOLUMES ## This should usually match all your `keepstore` instances Volumes: diff --git a/test/salt/pillar/arvados_dev.sls b/test/salt/pillar/arvados_dev.sls index 1dcc78b..2160b93 100644 --- a/test/salt/pillar/arvados_dev.sls +++ b/test/salt/pillar/arvados_dev.sls @@ -83,22 +83,18 @@ arvados: ### TOKENS tokens: - system_root: changemesystemroottoken - management: changememanagementtoken + system_root: systemroottokenmushaveatleast32characters + management: managementtokenmushaveatleast32characters ### KEYS secrets: - blob_signing_key: changemeblobsigningkey - workbench_secret_key: changemeworkbenchsecretkey + blob_signing_key: blobsigningkeymushaveatleast32characters + workbench_secret_key: workbenchsecretkeymushaveatleast32characters dispatcher_access_key: changemedispatcheraccesskey dispatcher_secret_key: changemedispatchersecretkey keep_access_key: changemekeepaccesskey keep_secret_key: changemekeepsecretkey - AuditLogs: - Section_to_ignore: - - some_random_value - ### VOLUMES ## This should usually match all your `keepstore` instances Volumes: -- 2.30.2 From ea03991ffd010f914ed016021e393a47654f8c8b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Wed, 5 May 2021 16:27:06 -0300 Subject: [PATCH 06/16] fix(shellinabox): renamed pam file also removed the messed `libpam-arvados-go` references, as they were not functional as they were written. --- arvados/defaults.yaml | 3 --- arvados/shell/config/clean.sls | 4 ++-- arvados/shell/config/file.sls | 8 ++++---- ...s.tmpl.jinja => shell-pam-shellinabox.tmpl.jinja} | 0 arvados/shell/package/install.sls | 1 - pillar.example | 5 +---- test/integration/shell/controls/config_spec.rb | 12 ++++++------ test/integration/shell/controls/packages_spec.rb | 1 - 8 files changed, 13 insertions(+), 21 deletions(-) rename arvados/shell/config/files/default/{shell-libpam-arvados.tmpl.jinja => shell-pam-shellinabox.tmpl.jinja} (100%) diff --git a/arvados/defaults.yaml b/arvados/defaults.yaml index d737ee4..07780b6 100644 --- a/arvados/defaults.yaml +++ b/arvados/defaults.yaml @@ -115,7 +115,6 @@ arvados: name: - arvados-client - arvados-src - - libpam-arvados-go - python3-arvados-fuse - python3-arvados-python-client - python3-arvados-cwl-runner @@ -129,8 +128,6 @@ arvados: service: name: shellinabox port: 4200 - libpam_arvados: - config: /etc/pam.d/arvados ##### WORKBENCH workbench: diff --git a/arvados/shell/config/clean.sls b/arvados/shell/config/clean.sls index caceeca..1c7848c 100644 --- a/arvados/shell/config/clean.sls +++ b/arvados/shell/config/clean.sls @@ -11,8 +11,8 @@ arvados-shell-config-clean-file-shellinabox-absent: - watch_in: - sls: {{ sls_service_clean }} -arvados-shell-config-clean-file-libpam-arvados-absent: +arvados-shell-config-clean-file-pam-shellinabox-absent: file.absent: - - name: {{ arvados.shell.libpam-arvados.config }} + - name: /etc/pam.d/shellinabox - watch_in: - sls: {{ sls_service_clean }} diff --git a/arvados/shell/config/file.sls b/arvados/shell/config/file.sls index c20ef99..e8bf644 100644 --- a/arvados/shell/config/file.sls +++ b/arvados/shell/config/file.sls @@ -28,11 +28,11 @@ arvados-shell-config-file-shellinabox-file-managed: - context: arvados: {{ arvados | json }} -arvados-shell-config-file-libpam-arvados-file-managed: +arvados-shell-config-file-pam-shellinabox-file-managed: file.managed: - - name: {{ arvados.shell.libpam_arvados.config }} - - source: {{ files_switch(['shell-libpam-arvados.tmpl.jinja'], - lookup='arvados-shell-config-file-libpam-arvados-file-managed', + - name: /etc/pam.d/shellinabox + - source: {{ files_switch(['shell-pam-shellinabox.tmpl.jinja'], + lookup='arvados-shell-config-file-pam-shellinabox-file-managed', use_subpath=True ) }} diff --git a/arvados/shell/config/files/default/shell-libpam-arvados.tmpl.jinja b/arvados/shell/config/files/default/shell-pam-shellinabox.tmpl.jinja similarity index 100% rename from arvados/shell/config/files/default/shell-libpam-arvados.tmpl.jinja rename to arvados/shell/config/files/default/shell-pam-shellinabox.tmpl.jinja diff --git a/arvados/shell/package/install.sls b/arvados/shell/package/install.sls index b1ad75e..c5d4206 100644 --- a/arvados/shell/package/install.sls +++ b/arvados/shell/package/install.sls @@ -21,7 +21,6 @@ arvados-shell-package-install-pkg-installed: {%- if package in [ 'arvados-client', 'arvados-src', - 'libpam-arvados-go', 'python3-arvados-fuse', 'python3-arvados-python-client', 'python3-arvados-cwl-runner', diff --git a/pillar.example b/pillar.example index 4105b66..e5278dd 100644 --- a/pillar.example +++ b/pillar.example @@ -48,7 +48,7 @@ arvados: # file: /etc/arvados/config.yml # user: root ### IMPORTANT!!!!! - ### If you're intalling any of the rails apps (api, workbench), the group + ### If you're installing any of the rails apps (api, workbench), the group ### should be set to that of the web server, usually `www-data` # group: root # mode: 640 @@ -238,7 +238,6 @@ arvados: # name: # - arvados-client # - arvados-src -# - libpam-arvados-go # - python3-arvados-fuse # - python3-arvados-python-client # - python3-arvados-cwl-runner @@ -251,8 +250,6 @@ arvados: # service: # name: shellinabox # port: 4200 -# libpam_arvados: -# config: /etc/pam.d/arvados # #### WORKBENCH # workbench: # pkg: diff --git a/test/integration/shell/controls/config_spec.rb b/test/integration/shell/controls/config_spec.rb index f0e8a7e..54852fa 100644 --- a/test/integration/shell/controls/config_spec.rb +++ b/test/integration/shell/controls/config_spec.rb @@ -45,14 +45,14 @@ control 'shellinabox configuration' do end end -control 'libpam-arvados configuration' do +control 'pam-shellinabox-arvados configuration' do title 'should match desired lines' - libpam_stanza = <<~LIBPAM_STANZA + pamshellinabox_stanza = <<~PAMSHELLINABOX_STANZA auth [success=1 default=ignore] /usr/lib/pam_arvados.so fixme.example.net shell.fixme.example.net - LIBPAM_STANZA + PAMSHELLINABOX_STANZA - describe file('/etc/pam.d/arvados') do + describe file('/etc/pam.d/shellinabox') do it { should be_file } it { should be_owned_by 'root' } it { should be_grouped_into 'root' } @@ -60,10 +60,10 @@ control 'libpam-arvados configuration' do its('content') do should include( # rubocop:disable Layout/LineLength - 'File managed by Salt at .' + 'File managed by Salt at .' # rubocop:enable Layout/LineLength ) end - its('content') { should include(libpam_stanza) } + its('content') { should include(pamshellinabox_stanza) } end end diff --git a/test/integration/shell/controls/packages_spec.rb b/test/integration/shell/controls/packages_spec.rb index 0b69de9..50fd1a5 100644 --- a/test/integration/shell/controls/packages_spec.rb +++ b/test/integration/shell/controls/packages_spec.rb @@ -3,7 +3,6 @@ packages_list = %w[ arvados-client arvados-src - libpam-arvados-go python3-arvados-fuse python3-arvados-python-client python3-arvados-cwl-runner -- 2.30.2 From abd6fde224cc7c272edfa44266b53eca900f8602 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Wed, 12 May 2021 13:11:28 -0300 Subject: [PATCH 07/16] fix(conditionals): wrong onlyif specification --- arvados/api/package/clean.sls | 4 ++-- arvados/api/package/install.sls | 2 +- arvados/ruby/package/clean.sls | 4 ++-- arvados/shell/package/clean.sls | 4 ++-- arvados/shell/package/install.sls | 2 +- arvados/workbench/package/clean.sls | 4 ++-- arvados/workbench/package/install.sls | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/arvados/api/package/clean.sls b/arvados/api/package/clean.sls index 52e3650..6f9e620 100644 --- a/arvados/api/package/clean.sls +++ b/arvados/api/package/clean.sls @@ -16,7 +16,7 @@ arvados-api-package-clean-gem-{{ gm }}-removed: arvados-api-package-clean-gems-deps-pkg-removed: pkg.removed: - pkgs: {{ arvados.ruby.gems_deps | json }} - - only_if: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" arvados-api-package-clean-pkg-removed: pkg.removed: @@ -25,4 +25,4 @@ arvados-api-package-clean-pkg-removed: arvados-api-package-clean-ruby-pkg-removed: pkg.removed: - name: {{ arvados.ruby.pkg }} - - only_if: test "{{ arvados.ruby.manage_ruby | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_ruby | lower }}" = "true" diff --git a/arvados/api/package/install.sls b/arvados/api/package/install.sls index 068d4a1..54e20ef 100644 --- a/arvados/api/package/install.sls +++ b/arvados/api/package/install.sls @@ -19,7 +19,7 @@ include: arvados-api-package-install-gems-deps-pkg-installed: pkg.installed: - pkgs: {{ arvados.ruby.gems_deps | unique | json }} - - only_if: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" {%- for gm in arvados.api.gem.name | unique %} arvados-api-package-install-gem-{{ gm }}-installed: diff --git a/arvados/ruby/package/clean.sls b/arvados/ruby/package/clean.sls index cd5f32a..9fb78af 100644 --- a/arvados/ruby/package/clean.sls +++ b/arvados/ruby/package/clean.sls @@ -16,7 +16,7 @@ arvados-shell-package-clean-gem-{{ gm }}-removed: arvados-shell-package-clean-gems-deps-pkg-removed: pkg.removed: - pkgs: {{ arvados.ruby.gems_deps | json }} - - only_if: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" arvados-shell-package-clean-pkg-removed: pkg.removed: @@ -25,4 +25,4 @@ arvados-shell-package-clean-pkg-removed: arvados-shell-package-clean-ruby-pkg-removed: pkg.removed: - name: {{ arvados.ruby.pkg }} - - only_if: test "{{ arvados.ruby.manage_ruby | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_ruby | lower }}" = "true" diff --git a/arvados/shell/package/clean.sls b/arvados/shell/package/clean.sls index cd5f32a..9fb78af 100644 --- a/arvados/shell/package/clean.sls +++ b/arvados/shell/package/clean.sls @@ -16,7 +16,7 @@ arvados-shell-package-clean-gem-{{ gm }}-removed: arvados-shell-package-clean-gems-deps-pkg-removed: pkg.removed: - pkgs: {{ arvados.ruby.gems_deps | json }} - - only_if: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" arvados-shell-package-clean-pkg-removed: pkg.removed: @@ -25,4 +25,4 @@ arvados-shell-package-clean-pkg-removed: arvados-shell-package-clean-ruby-pkg-removed: pkg.removed: - name: {{ arvados.ruby.pkg }} - - only_if: test "{{ arvados.ruby.manage_ruby | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_ruby | lower }}" = "true" diff --git a/arvados/shell/package/install.sls b/arvados/shell/package/install.sls index c5d4206..6d1300f 100644 --- a/arvados/shell/package/install.sls +++ b/arvados/shell/package/install.sls @@ -34,7 +34,7 @@ arvados-shell-package-install-pkg-installed: arvados-shell-package-install-gems-deps-pkg-installed: pkg.installed: - pkgs: {{ arvados.ruby.gems_deps | json }} - - only_if: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" {%- for gm in arvados.shell.gem.name %} arvados-shell-package-install-gem-{{ gm }}-installed: diff --git a/arvados/workbench/package/clean.sls b/arvados/workbench/package/clean.sls index fcfd2ba..f7970da 100644 --- a/arvados/workbench/package/clean.sls +++ b/arvados/workbench/package/clean.sls @@ -8,7 +8,7 @@ arvados-workbench-package-clean-gems-deps-pkg-removed: pkg.removed: - pkgs: {{ arvados.ruby.gems_deps | json }} - - only_if: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" arvados-workbench-package-clean-pkg-removed: pkg.removed: @@ -17,4 +17,4 @@ arvados-workbench-package-clean-pkg-removed: arvados-workbench-package-clean-ruby-pkg-removed: pkg.removed: - name: {{ arvados.ruby.pkg }} - - only_if: test "{{ arvados.ruby.manage_ruby | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_ruby | lower }}" = "true" diff --git a/arvados/workbench/package/install.sls b/arvados/workbench/package/install.sls index cbda268..dff47f6 100644 --- a/arvados/workbench/package/install.sls +++ b/arvados/workbench/package/install.sls @@ -19,7 +19,7 @@ include: arvados-workbench-package-install-gems-deps-pkg-installed: pkg.installed: - pkgs: {{ arvados.ruby.gems_deps | json }} - - only_if: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" + - onlyif: test "{{ arvados.ruby.manage_gems_deps | lower }}" = "true" arvados-workbench-package-install-pkg-installed: pkg.installed: -- 2.30.2 From b2a5dc4e4da45de6f3357eeb341ab8b3a9113c9a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Wed, 12 May 2021 13:30:53 -0300 Subject: [PATCH 08/16] feat(resources): add state to register virtual_machines --- arvados/api/init.sls | 1 + arvados/api/resources/init.sls | 5 +++ arvados/api/resources/virtual_machines.sls | 43 ++++++++++++++++++ arvados/defaults.yaml | 2 + kitchen.yml | 4 +- pillar.example | 44 ++++++++++++------- .../api/controls/resources_spec.rb | 28 ++++++++++++ .../integration/shell/controls/config_spec.rb | 2 +- test/salt/pillar/arvados.sls | 8 ++++ .../examples/nginx_webshell_configuration.sls | 1 + 10 files changed, 121 insertions(+), 17 deletions(-) create mode 100644 arvados/api/resources/init.sls create mode 100644 arvados/api/resources/virtual_machines.sls create mode 100644 test/integration/api/controls/resources_spec.rb diff --git a/arvados/api/init.sls b/arvados/api/init.sls index 02a98b8..24f84d0 100644 --- a/arvados/api/init.sls +++ b/arvados/api/init.sls @@ -5,3 +5,4 @@ include: - .package - ..config - .service + - .resources diff --git a/arvados/api/resources/init.sls b/arvados/api/resources/init.sls new file mode 100644 index 0000000..f3f78a3 --- /dev/null +++ b/arvados/api/resources/init.sls @@ -0,0 +1,5 @@ +# -*- coding: utf-8 -*- +# vim: ft=sls + +include: + - .virtual_machines diff --git a/arvados/api/resources/virtual_machines.sls b/arvados/api/resources/virtual_machines.sls new file mode 100644 index 0000000..a2af30a --- /dev/null +++ b/arvados/api/resources/virtual_machines.sls @@ -0,0 +1,43 @@ +# -*- coding: utf-8 -*- +# vim: ft=sls + +{#- Get the `tplroot` from `tpldir` #} +{%- set tplroot = tpldir.split('/')[0] %} +{%- set sls_config_file = tplroot ~ '.config.file' %} +{%- from tplroot ~ "/map.jinja" import arvados with context %} +{%- from tplroot ~ "/libtofs.jinja" import files_switch with context %} + +{%- set virtual_machines = arvados.cluster.resources.virtual_machines | default({}) %} +{%- set api_token = arvados.cluster.tokens.system_root | yaml_encode %} +{%- set api_host = arvados.cluster.Services.Controller.ExternalURL | regex_replace('^http(s?)://', '', ignorecase=true) %} + +include: + - ..package + - {{ sls_config_file }} + - ..service + +{%- for vm, vm_params in virtual_machines.items() %} + {%- set vm_name = vm_params.name | default(vm) %} + {%- set vm_backend = vm_params.backend | default(vm_name) %} + {%- set vm_port = vm_params.port | default(4200) %} + +arvados-api-resources-virtual-machines-{{ vm }}-record-cmd-run: + cmd.run: + - env: + - ARVADOS_API_TOKEN: {{ api_token }} + - ARVADOS_API_HOST: {{ api_host }} + - name: | + arv --format=uuid \ + virtual_machine \ + create \ + --virtual-machine '{"hostname":"{{ vm_name }}" }' + - onlyif: | + ARVADOS_API_TOKEN={{ api_token }} \ + ARVADOS_API_HOST={{ api_host }} \ + arv --short \ + virtual_machine \ + list \ + --filters '[["hostname", "=", "{{ vm_name }}"]]' | \ + /bin/grep -qE "fixme-2x53u-[a-z0-9_]{15}" && \ + false +{%- endfor %} diff --git a/arvados/defaults.yaml b/arvados/defaults.yaml index 07780b6..bceb84e 100644 --- a/arvados/defaults.yaml +++ b/arvados/defaults.yaml @@ -52,6 +52,8 @@ arvados: key: '' insecure: false + resources: {} + ### THESE ARE THE PACKAGES AND DAEMONS BASIC CONFIGS ##### API api: diff --git a/kitchen.yml b/kitchen.yml index cb35319..a511ce8 100644 --- a/kitchen.yml +++ b/kitchen.yml @@ -287,7 +287,7 @@ suites: - path: test/integration/keepproxy - path: test/integration/keepweb - path: test/integration/controller - #### workbench,workbench2 + #### workbench,workbench2,webshell - name: workbench driver: hostname: workbench.fixme.example.net @@ -316,12 +316,14 @@ suites: - example_nginx - example_nginx_workbench - example_nginx_workbench2 + - example_nginx_webshell pillars_from_files: # yamllint disable rule:line-length arvados.sls: test/salt/pillar/arvados.sls example_nginx.sls: test/salt/pillar/examples/nginx_passenger.sls example_nginx_workbench.sls: test/salt/pillar/examples/nginx_workbench_configuration.sls example_nginx_workbench2.sls: test/salt/pillar/examples/nginx_workbench2_configuration.sls + example_nginx_webshell.sls: test/salt/pillar/examples/nginx_webshell_configuration.sls # yamllint enable rule:line-length verifier: inspec_tests: diff --git a/pillar.example b/pillar.example index e5278dd..ba5f228 100644 --- a/pillar.example +++ b/pillar.example @@ -87,7 +87,6 @@ arvados: # verify-ca: false # client_encoding: UTF8 - tls: # certificate: '' # key: '' @@ -114,6 +113,35 @@ arvados: keep_access_key: changemekeepaccesskey keep_secret_key: changemekeepsecretkey + ### ARVADOS RESOURCES + # This dict allows you to create various resources in the Arvados + # database so they're ready to use. + # Check the `arvados.api.resources.* states to see which can be + # currently managed + + ### SHELL / WEBSHELL REGISTRATION + # In order to use shell nodes via webshell, Arvados needs to know of + # their existence and they need to be configured as upstreams in nginx + # (see https://doc.arvados.org/v2.0/install/install-webshell.html) + # This could be achieved in various ways (ie, through salt mine if you + # want them to be dinamically created), but that's outside the scope of + # this formula. The following dict is just an example that will be used + # by the `arvados.api.resources.virtual_machines` state to add entries + # in Arvados' database of the cluster's resources' + # It's additionally used in the `test/salt/pillar/examples/nginx_webshell_configuration.sls` + # pillar to add the corresponding `location` entries in nginx's webshell vhosts & upstreams + resources: + virtual_machines: + shell1: + name: webshell1 # if not set, will match the one of the dict key above + backend: 1.2.3.4 # upstream host ip/name that has the shell role + port: 4200 # port where shellinabox is listening + # when no other parameter is set: + # `name` will match the name of the key + # backend` will match `name` + # `port` will default to shellinabox's 4200 + webshell2: {} + ### VOLUMES ## This should usually match all your `keepstore` instances Volumes: @@ -225,13 +253,6 @@ arvados: # service: # name: keepstore # port: 25107 -# #### GIT-HTTPD -# githttpd: -# pkg: -# name: arvados-git-httpd -# service: -# name: arvados-git-httpd -# port: 9001 # #### SHELL # shell: # pkg: @@ -269,13 +290,6 @@ arvados: # service: # name: arvados-ws # port: 8005 -# #### SSO -# sso: -# pkg: -# name: arvados-sso -# service: -# name: arvados-sso -# port: 8900 # ## SALTSTACK FORMULAS TOFS configuration # https://template-formula.readthedocs.io/en/latest/TOFS_pattern.html diff --git a/test/integration/api/controls/resources_spec.rb b/test/integration/api/controls/resources_spec.rb new file mode 100644 index 0000000..c4e2f6b --- /dev/null +++ b/test/integration/api/controls/resources_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +test_cmd = <<~TEST_CMD + su -l kitchen -c \ + "ARVADOS_API_TOKEN=\\"systemroottokenmushaveatleast32characters\\" \ + ARVADOS_API_HOST=\\"fixme.example.net\\" \ + arv virtual_machine list --filters '[[\\"hostname\\", \\"=\\", \\"%s\\"]]'" +TEST_CMD + +control 'arvados api resources' do + impact 0.5 + title 'should be created' + + %w[ + webshell1 + webshell2 + ].each do |vm| + describe "virtual machine #{vm}" do + subject do + command(test_cmd % vm) + end + its('stdout') { should match(/"uuid":"fixme-2x53u-[a-z0-9_]{15}"/) } + its('stdout') { should match(/"hostname":"#{vm}"/) } + its('stderr') { should eq '' } + its('exit_status') { should eq 0 } + end + end +end diff --git a/test/integration/shell/controls/config_spec.rb b/test/integration/shell/controls/config_spec.rb index 54852fa..0ada81c 100644 --- a/test/integration/shell/controls/config_spec.rb +++ b/test/integration/shell/controls/config_spec.rb @@ -4,7 +4,7 @@ control 'arvados configuration file' do title 'should not exist' describe file('/etc/arvados/config.yml') do - it { should_not exist} + it { should_not exist } end end diff --git a/test/salt/pillar/arvados.sls b/test/salt/pillar/arvados.sls index d8117c6..1abe76c 100644 --- a/test/salt/pillar/arvados.sls +++ b/test/salt/pillar/arvados.sls @@ -55,6 +55,14 @@ arvados: # required to test with snakeoil certs insecure: true + resources: + virtual_machines: + shell1: + name: webshell1 + backend: 1.2.3.4 + port: 4200 + webshell2: {} + ### TOKENS tokens: system_root: systemroottokenmushaveatleast32characters diff --git a/test/salt/pillar/examples/nginx_webshell_configuration.sls b/test/salt/pillar/examples/nginx_webshell_configuration.sls index 022cd36..e28fb9a 100644 --- a/test/salt/pillar/examples/nginx_webshell_configuration.sls +++ b/test/salt/pillar/examples/nginx_webshell_configuration.sls @@ -7,6 +7,7 @@ nginx: ### STREAMS http: + {%- for shell_node, params in %} upstream webshell_upstream: - server: 'shell.internal:4200 fail_timeout=10s' -- 2.30.2 From 7f41ee686a38f70d40d2713b44d480e5490ec3d1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Wed, 12 May 2021 16:05:21 -0300 Subject: [PATCH 09/16] style(sls): fix `salt-lint` errors --- arvados/config/file.sls | 3 ++- arvados/dispatcher/service/file.sls | 2 ++ test/salt/states/examples/single_host/host_entries.sls | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/arvados/config/file.sls b/arvados/config/file.sls index 4d99721..2249103 100644 --- a/arvados/config/file.sls +++ b/arvados/config/file.sls @@ -6,7 +6,8 @@ {%- from tplroot ~ "/map.jinja" import arvados with context %} {%- from tplroot ~ "/libtofs.jinja" import files_switch with context %} -{%- do arvados.cluster.Users.update({'AnonymousUserToken': arvados.cluster.tokens.anonymous_user }) if arvados.cluster.Users.AnonymousUserToken is not defined %} +{%- do arvados.cluster.Users.update({'AnonymousUserToken': arvados.cluster.tokens.anonymous_user }) + if arvados.cluster.Users.AnonymousUserToken is not defined %} include: - .package diff --git a/arvados/dispatcher/service/file.sls b/arvados/dispatcher/service/file.sls index 411848f..504f423 100644 --- a/arvados/dispatcher/service/file.sls +++ b/arvados/dispatcher/service/file.sls @@ -73,4 +73,6 @@ arvados-dispatcher-service-file-file-managed-crunch-dispatch-local-service: - service: arvados-dispatcher-service-running-service-running - require: - file: arvados-dispatcher-service-file-file-managed-crunch-dispatch-local-service + - onchanges: + - file: arvados-dispatcher-service-file-file-managed-crunch-dispatch-local-service {%- endif %} diff --git a/test/salt/states/examples/single_host/host_entries.sls b/test/salt/states/examples/single_host/host_entries.sls index fc35678..7670da0 100644 --- a/test/salt/states/examples/single_host/host_entries.sls +++ b/test/salt/states/examples/single_host/host_entries.sls @@ -5,7 +5,7 @@ arvados_test_salt_states_examples_single_host_etc_hosts_host_present: host.present: - - ip: {{ grains.get('ipv4')[0] }} + - ip: {{ grains['ipv4'][0] }} - names: - {{ arvados.cluster.name }}.{{ arvados.cluster.domain }} # FIXME! This just works for our testings. -- 2.30.2 From fa6d569ffe200b11354cac28452645a9a868ea4a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Wed, 12 May 2021 16:08:27 -0300 Subject: [PATCH 10/16] test(snakeoil_certs): ensure states are not run every time --- test/salt/states/examples/single_host/snakeoil_certs.sls | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/salt/states/examples/single_host/snakeoil_certs.sls b/test/salt/states/examples/single_host/snakeoil_certs.sls index 6f4e932..6ac8f87 100644 --- a/test/salt/states/examples/single_host/snakeoil_certs.sls +++ b/test/salt/states/examples/single_host/snakeoil_certs.sls @@ -131,9 +131,10 @@ arvados_test_salt_states_examples_single_host_snakeoil_certs_ssl_cert_pkg_instal - sls: postgres arvados_test_salt_states_examples_single_host_snakeoil_certs_certs_permissions_cmd_run: - cmd.run: - - name: | - chown root:ssl-cert {{ arvados_key_file }} + file.managed: + - name: {{ arvados_key_file }} + - owner: root + - group: ssl-cert - require: - cmd: arvados_test_salt_states_examples_single_host_snakeoil_certs_arvados_snake_oil_cert_cmd_run - pkg: arvados_test_salt_states_examples_single_host_snakeoil_certs_ssl_cert_pkg_installed @@ -149,6 +150,6 @@ arvados_test_salt_states_examples_single_host_snakeoil_certs_nginx_snakeoil_file - service: nginx_service - require: - pkg: passenger_install - - cmd: arvados_test_salt_states_examples_single_host_snakeoil_certs_certs_permissions_cmd_run + - file: arvados_test_salt_states_examples_single_host_snakeoil_certs_certs_permissions_cmd_run - require_in: - file: nginx_config -- 2.30.2 From 42913b9bb0e18563a40ec56c233121d3c1530a1a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Wed, 12 May 2021 16:09:08 -0300 Subject: [PATCH 11/16] fix(virtual_machines): ensure uuid is not added every time --- arvados/api/resources/virtual_machines.sls | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/arvados/api/resources/virtual_machines.sls b/arvados/api/resources/virtual_machines.sls index a2af30a..07a215d 100644 --- a/arvados/api/resources/virtual_machines.sls +++ b/arvados/api/resources/virtual_machines.sls @@ -31,13 +31,12 @@ arvados-api-resources-virtual-machines-{{ vm }}-record-cmd-run: virtual_machine \ create \ --virtual-machine '{"hostname":"{{ vm_name }}" }' - - onlyif: | + - unless: | ARVADOS_API_TOKEN={{ api_token }} \ - ARVADOS_API_HOST={{ api_host }} \ + ARVADOS_API_HOST="{{ api_host }}" \ arv --short \ virtual_machine \ list \ --filters '[["hostname", "=", "{{ vm_name }}"]]' | \ - /bin/grep -qE "fixme-2x53u-[a-z0-9_]{15}" && \ - false + /bin/grep -qE "fixme-2x53u-[a-z0-9_]{15}" {%- endfor %} -- 2.30.2 From 9cabd51263de6dadf5000b488ee62d3c32af50c0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Thu, 13 May 2021 12:50:21 -0300 Subject: [PATCH 12/16] feat(scoped_tokens): create scoped_tokens for webshell instances --- arvados/api/resources/virtual_machines.sls | 52 +++++++++++++++---- .../api/controls/resources_spec.rb | 27 ++++++++-- test/salt/pillar/arvados.sls | 3 ++ 3 files changed, 70 insertions(+), 12 deletions(-) diff --git a/arvados/api/resources/virtual_machines.sls b/arvados/api/resources/virtual_machines.sls index 07a215d..0bb468d 100644 --- a/arvados/api/resources/virtual_machines.sls +++ b/arvados/api/resources/virtual_machines.sls @@ -16,11 +16,20 @@ include: - {{ sls_config_file }} - ..service +arvados-api-resources-virtual-machines-jq-pkg-installed: + pkg.installed: + - name: jq + {%- for vm, vm_params in virtual_machines.items() %} {%- set vm_name = vm_params.name | default(vm) %} - {%- set vm_backend = vm_params.backend | default(vm_name) %} - {%- set vm_port = vm_params.port | default(4200) %} + {%- set cmd_query_vm_uuid = 'ARVADOS_API_TOKEN=' ~ api_token ~ + ' ARVADOS_API_HOST=' ~ api_host ~ + ' arv --short virtual_machine list' ~ + ' --filters \'[["hostname", "=", "' ~ vm_name ~ '"]]\'' + %} + +# Create the virtual machine record arvados-api-resources-virtual-machines-{{ vm }}-record-cmd-run: cmd.run: - env: @@ -32,11 +41,36 @@ arvados-api-resources-virtual-machines-{{ vm }}-record-cmd-run: create \ --virtual-machine '{"hostname":"{{ vm_name }}" }' - unless: | - ARVADOS_API_TOKEN={{ api_token }} \ - ARVADOS_API_HOST="{{ api_host }}" \ - arv --short \ - virtual_machine \ - list \ - --filters '[["hostname", "=", "{{ vm_name }}"]]' | \ - /bin/grep -qE "fixme-2x53u-[a-z0-9_]{15}" + {{ cmd_query_vm_uuid }} | \ + /bin/grep -qE "fixme-2x53u-[a-z0-9]{15}" + + # As we need the UUID generated in the previous command, we need to + # iterate again in order to get them + {% set vm_uuid = salt['cmd.shell'](cmd_query_vm_uuid) %} + + {%- set scoped_token_url = '/arvados/v1/virtual_machines/' ~ vm_uuid ~ '/logins' %} + + # There's no direct way to query the scoped_token for a given virtual_machine + # so we need to parse the api_client_authorization list through some jq + {%- set cmd_query_scoped_token_url = 'ARVADOS_API_TOKEN=' ~ api_token ~ + ' ARVADOS_API_HOST=' ~ api_host ~ + ' arv api_client_authorization list |' ~ + ' jq -e \'.items[].scopes[] | select(. == "GET ' ~ + scoped_token_url ~ '")\'' + %} +# Create the VM scoped tokens +arvados-api-resources-virtual-machines-{{ vm }}-scoped-token-cmd-run: + cmd.run: + - env: + - ARVADOS_API_TOKEN: {{ api_token }} + - ARVADOS_API_HOST: {{ api_host }} + - name: | + arv --format=uuid \ + api_client_authorization \ + create \ + --api-client-authorization '{"scopes":["GET {{ scoped_token_url }}"]}' + - require: + - pkg: arvados-api-resources-virtual-machines-jq-pkg-installed + - unless: {{ cmd_query_scoped_token_url }} + {%- endfor %} diff --git a/test/integration/api/controls/resources_spec.rb b/test/integration/api/controls/resources_spec.rb index c4e2f6b..2d05a1e 100644 --- a/test/integration/api/controls/resources_spec.rb +++ b/test/integration/api/controls/resources_spec.rb @@ -1,11 +1,18 @@ # frozen_string_literal: true -test_cmd = <<~TEST_CMD +query_virtual_machines = <<~TEST_VM_CMD su -l kitchen -c \ "ARVADOS_API_TOKEN=\\"systemroottokenmushaveatleast32characters\\" \ ARVADOS_API_HOST=\\"fixme.example.net\\" \ arv virtual_machine list --filters '[[\\"hostname\\", \\"=\\", \\"%s\\"]]'" -TEST_CMD +TEST_VM_CMD + +query_scoped_token_urls = <<~TEST_STU_CMD + su -l kitchen -c \ + "ARVADOS_API_TOKEN=\\"systemroottokenmushaveatleast32characters\\" \ + ARVADOS_API_HOST=\\"fixme.example.net\\" \ + arv api_client_authorization list" +TEST_STU_CMD control 'arvados api resources' do impact 0.5 @@ -14,15 +21,29 @@ control 'arvados api resources' do %w[ webshell1 webshell2 + webshell3 ].each do |vm| describe "virtual machine #{vm}" do subject do - command(test_cmd % vm) + command(query_virtual_machines % vm) end its('stdout') { should match(/"uuid":"fixme-2x53u-[a-z0-9_]{15}"/) } its('stdout') { should match(/"hostname":"#{vm}"/) } its('stderr') { should eq '' } its('exit_status') { should eq 0 } end + + describe "scoped token for #{vm}" do + subject do + command(query_scoped_token_urls % vm) + end + its('stdout') do + should match( + %r{"GET /arvados/v1/virtual_machines/fixme-2x53u-[a-z0-9]{15}/logins"} + ) + end + its('stderr') { should eq '' } + its('exit_status') { should eq 0 } + end end end diff --git a/test/salt/pillar/arvados.sls b/test/salt/pillar/arvados.sls index 1abe76c..81d22d4 100644 --- a/test/salt/pillar/arvados.sls +++ b/test/salt/pillar/arvados.sls @@ -62,6 +62,9 @@ arvados: backend: 1.2.3.4 port: 4200 webshell2: {} + webshell3: + backend: 4.3.2.1 + port: 4500 ### TOKENS tokens: -- 2.30.2 From d708f6e05ad3593e6ebe4094ea8414110b1510c2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Fri, 14 May 2021 18:58:58 -0300 Subject: [PATCH 13/16] fix(arvados-controller): add a loop to verify the service is ready It's not enough to have the service up, but it needs to be responding correctly. --- arvados/controller/service/running.sls | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/arvados/controller/service/running.sls b/arvados/controller/service/running.sls index 3fca8f3..6c95b3c 100644 --- a/arvados/controller/service/running.sls +++ b/arvados/controller/service/running.sls @@ -18,3 +18,21 @@ arvados-controller-service-running-service-running: - sls: {{ sls_config_file }} - require: - pkg: arvados-controller-package-install-pkg-installed + +# Before being able to create resources, we need API to be up. When running the formula for +# the first time, it might be still being configured, so we add this workaround, as suggested at +# https://github.com/saltstack/salt/issues/19084#issuecomment-70317884 +arvados-controller-service-running-service-ready-cmd-run: + cmd.run: + - name: | + while ! (curl -s {{ arvados.cluster.Services.Controller.ExternalURL }} | \ + grep -qE "req-[a-z0-9]{20}.{5}error_token") do + echo 'waiting for API to be ready...' + sleep 1 + done + - timeout: 520 + - unless: | + curl -s {{ arvados.cluster.Services.Controller.ExternalURL }} | \ + grep -qE "req-[a-z0-9]{20}.{5}error_token" + - require: + - service: arvados-controller-service-running-service-running -- 2.30.2 From 1c81e527e7a3b3a965958e33d7073135071b3ff3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Fri, 14 May 2021 19:00:17 -0300 Subject: [PATCH 14/16] fix(scoped_tokens): resolved dependencies on virtual_machines Also added a state to check the vm uuid exists before creating the scoped token --- arvados/api/init.sls | 1 - arvados/controller/init.sls | 1 + .../{api => controller}/resources/init.sls | 0 .../resources/virtual_machines.sls | 44 +++++++++++++------ .../controls/resources_spec.rb | 0 5 files changed, 31 insertions(+), 15 deletions(-) rename arvados/{api => controller}/resources/init.sls (100%) rename arvados/{api => controller}/resources/virtual_machines.sls (53%) rename test/integration/{api => controller}/controls/resources_spec.rb (100%) diff --git a/arvados/api/init.sls b/arvados/api/init.sls index 24f84d0..02a98b8 100644 --- a/arvados/api/init.sls +++ b/arvados/api/init.sls @@ -5,4 +5,3 @@ include: - .package - ..config - .service - - .resources diff --git a/arvados/controller/init.sls b/arvados/controller/init.sls index 02a98b8..24f84d0 100644 --- a/arvados/controller/init.sls +++ b/arvados/controller/init.sls @@ -5,3 +5,4 @@ include: - .package - ..config - .service + - .resources diff --git a/arvados/api/resources/init.sls b/arvados/controller/resources/init.sls similarity index 100% rename from arvados/api/resources/init.sls rename to arvados/controller/resources/init.sls diff --git a/arvados/api/resources/virtual_machines.sls b/arvados/controller/resources/virtual_machines.sls similarity index 53% rename from arvados/api/resources/virtual_machines.sls rename to arvados/controller/resources/virtual_machines.sls index 0bb468d..7303dac 100644 --- a/arvados/api/resources/virtual_machines.sls +++ b/arvados/controller/resources/virtual_machines.sls @@ -16,7 +16,7 @@ include: - {{ sls_config_file }} - ..service -arvados-api-resources-virtual-machines-jq-pkg-installed: +arvados-controller-resources-virtual-machines-jq-pkg-installed: pkg.installed: - name: jq @@ -30,7 +30,7 @@ arvados-api-resources-virtual-machines-jq-pkg-installed: %} # Create the virtual machine record -arvados-api-resources-virtual-machines-{{ vm }}-record-cmd-run: +arvados-controller-resources-virtual-machines-{{ vm }}-record-cmd-run: cmd.run: - env: - ARVADOS_API_TOKEN: {{ api_token }} @@ -43,34 +43,50 @@ arvados-api-resources-virtual-machines-{{ vm }}-record-cmd-run: - unless: | {{ cmd_query_vm_uuid }} | \ /bin/grep -qE "fixme-2x53u-[a-z0-9]{15}" + - require: + - pkg: arvados-controller-package-install-pkg-installed + - cmd: arvados-controller-service-running-service-ready-cmd-run - # As we need the UUID generated in the previous command, we need to - # iterate again in order to get them - {% set vm_uuid = salt['cmd.shell'](cmd_query_vm_uuid) %} - - {%- set scoped_token_url = '/arvados/v1/virtual_machines/' ~ vm_uuid ~ '/logins' %} +# We need to use the UUID generated in the previous command to see if there's a +# scoped token for it. There's no easy way to pass the value from a shellout +# to another state, so we store it in a temp file and use that in the next +# command. Flaky, mostly because the `unless` clause is just checking thatg +# the file content is a token uuid :| +arvados-controller-resources-virtual-machines-{{ vm }}-get-vm_uuid-cmd-run: + cmd.run: + - name: {{ cmd_query_vm_uuid }} | head -1 | tee /tmp/{{ vm }} + - require: + - cmd: arvados-controller-resources-virtual-machines-{{ vm }}-record-cmd-run + - unless: + - /bin/grep -qE "fixme-2x53u-[a-z0-9]{15}" /tmp/{{ vm }} # There's no direct way to query the scoped_token for a given virtual_machine # so we need to parse the api_client_authorization list through some jq - {%- set cmd_query_scoped_token_url = 'ARVADOS_API_TOKEN=' ~ api_token ~ + {%- set cmd_query_scoped_token_url = 'VM_UUID=$(cat /tmp/' ~ vm ~ ') && ' ~ + ' ARVADOS_API_TOKEN=' ~ api_token ~ ' ARVADOS_API_HOST=' ~ api_host ~ ' arv api_client_authorization list |' ~ - ' jq -e \'.items[].scopes[] | select(. == "GET ' ~ - scoped_token_url ~ '")\'' + ' /usr/bin/jq -e \'.items[].scopes[] | select(. == "GET ' ~ + '/arvados/v1/virtual_machines/\'${VM_UUID}\'/logins")\' && ' ~ + 'unset VM_UUID' %} + # Create the VM scoped tokens -arvados-api-resources-virtual-machines-{{ vm }}-scoped-token-cmd-run: +arvados-controller-resources-virtual-machines-{{ vm }}-scoped-token-cmd-run: cmd.run: - env: - ARVADOS_API_TOKEN: {{ api_token }} - ARVADOS_API_HOST: {{ api_host }} - name: | + VM_UUID=$(cat /tmp/{{ vm }}) && arv --format=uuid \ api_client_authorization \ create \ - --api-client-authorization '{"scopes":["GET {{ scoped_token_url }}"]}' - - require: - - pkg: arvados-api-resources-virtual-machines-jq-pkg-installed + --api-client-authorization '{"scopes":["GET /arvados/v1/virtual_machines/'${VM_UUID}'/logins"]}' - unless: {{ cmd_query_scoped_token_url }} + - require: + - pkg: arvados-controller-package-install-pkg-installed + - pkg: arvados-controller-resources-virtual-machines-jq-pkg-installed + - cmd: arvados-controller-resources-virtual-machines-{{ vm }}-get-vm_uuid-cmd-run {%- endfor %} diff --git a/test/integration/api/controls/resources_spec.rb b/test/integration/controller/controls/resources_spec.rb similarity index 100% rename from test/integration/api/controls/resources_spec.rb rename to test/integration/controller/controls/resources_spec.rb -- 2.30.2 From ca0ebadc5ae1737f2b8aa9b467aa0ed7791ce287 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Mon, 17 May 2021 14:13:14 -0300 Subject: [PATCH 15/16] test(hosts): use the nic ip instead of localhost for dns --- test/salt/states/examples/single_host/host_entries.sls | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/salt/states/examples/single_host/host_entries.sls b/test/salt/states/examples/single_host/host_entries.sls index 7670da0..8dfb64e 100644 --- a/test/salt/states/examples/single_host/host_entries.sls +++ b/test/salt/states/examples/single_host/host_entries.sls @@ -5,7 +5,7 @@ arvados_test_salt_states_examples_single_host_etc_hosts_host_present: host.present: - - ip: {{ grains['ipv4'][0] }} + - ip: {{ grains['ipv4'][1] }} - names: - {{ arvados.cluster.name }}.{{ arvados.cluster.domain }} # FIXME! This just works for our testings. -- 2.30.2 From 257615eab47f9d4bf64694dc0aac9dfff4e8edc4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Javier=20B=C3=A9rtoli?= Date: Mon, 17 May 2021 14:15:20 -0300 Subject: [PATCH 16/16] test(examples): remove workbench upstream in nginx --- .../workbench/controls/services_spec.rb | 2 +- .../nginx_workbench_configuration.sls | 33 ++----------------- 2 files changed, 4 insertions(+), 31 deletions(-) diff --git a/test/integration/workbench/controls/services_spec.rb b/test/integration/workbench/controls/services_spec.rb index 006d96f..e7910c9 100644 --- a/test/integration/workbench/controls/services_spec.rb +++ b/test/integration/workbench/controls/services_spec.rb @@ -9,7 +9,7 @@ control 'arvados workbench service' do it { should be_running } end - describe port(9000) do + describe port(443) do proc = case os[:name] when 'centos' # Centos ps adds an extra colon and the end of the process diff --git a/test/salt/pillar/examples/nginx_workbench_configuration.sls b/test/salt/pillar/examples/nginx_workbench_configuration.sls index fbadc58..e34ad2d 100644 --- a/test/salt/pillar/examples/nginx_workbench_configuration.sls +++ b/test/salt/pillar/examples/nginx_workbench_configuration.sls @@ -12,15 +12,6 @@ arvados: ### NGINX nginx: - ### SERVER - server: - config: - - ### STREAMS - http: - upstream workbench_upstream: - - server: 'workbench.internal:9000 fail_timeout=10s' - ### SITES servers: managed: @@ -48,30 +39,12 @@ nginx: - 443 http2 ssl - index: index.html index.htm - location /: - - proxy_pass: 'http://workbench_upstream' - - proxy_read_timeout: 300 - - proxy_connect_timeout: 90 - - proxy_redirect: 'off' - - proxy_set_header: X-Forwarded-Proto https - - proxy_set_header: 'Host $http_host' - - proxy_set_header: 'X-Real-IP $remote_addr' - - proxy_set_header: 'X-Forwarded-For $proxy_add_x_forwarded_for' + - root: /var/www/arvados-workbench/current/public + - passenger_enabled: 'on' - include: 'snippets/ssl_hardening_default.conf' # - include: 'snippets/letsencrypt.conf' - include: 'snippets/ssl_snakeoil.conf' + # yamllint disable-line rule:line-length - access_log: /var/log/nginx/workbench.fixme.example.net.access.log combined - error_log: /var/log/nginx/workbench.fixme.example.net.error.log - arvados_workbench_upstream.conf: - enabled: true - overwrite: true - config: - - server: - - listen: 'workbench.internal:9000' - - server_name: workbench - - root: /var/www/arvados-workbench/current/public - - index: index.html index.htm - - passenger_enabled: 'on' - # yamllint disable-line rule:line-length - - access_log: /var/log/nginx/workbench.fixme.example.net-upstream.access.log combined - - error_log: /var/log/nginx/workbench.fixme.example.net-upstream.error.log -- 2.30.2