Remove ability to run spack monitor without auth (#27888)

spack monitor now requires authentication as each build must be associated
with a user, so it does not make sense to allow the --monitor-no-auth flag
and this commit will remove it

lib/spack/spack/cmd/analyze.py

@@ -110,7 +110,6 @@ def analyze(parser, args, **kwargs):
         monitor = spack.monitor.get_client(
             host=args.monitor_host,
             prefix=args.monitor_prefix,
-            disable_auth=args.monitor_disable_auth,
         )
 
     # Run the analysis

lib/spack/spack/cmd/containerize.py

@@ -50,7 +50,6 @@ def containerize(parser, args):
     # If we have a monitor request, add monitor metadata to config
     if args.use_monitor:
         config['spack']['monitor'] = {
-            "disable_auth": args.monitor_disable_auth,
             "host": args.monitor_host,
             "keep_going": args.monitor_keep_going,
             "prefix": args.monitor_prefix,

lib/spack/spack/cmd/install.py

@@ -305,7 +305,6 @@ def install(parser, args, **kwargs):
         monitor = spack.monitor.get_client(
             host=args.monitor_host,
             prefix=args.monitor_prefix,
-            disable_auth=args.monitor_disable_auth,
             tags=args.monitor_tags,
             save_local=args.monitor_save_local,
         )
@@ -467,7 +466,6 @@ def get_tests(specs):
 
         # Update install_args with the monitor args, needed for build task
         kwargs.update({
-            "monitor_disable_auth": args.monitor_disable_auth,
             "monitor_keep_going": args.monitor_keep_going,
             "monitor_host": args.monitor_host,
             "use_monitor": args.use_monitor,

lib/spack/spack/cmd/monitor.py

@@ -27,7 +27,6 @@ def monitor(parser, args, **kwargs):
         monitor = spack.monitor.get_client(
             host=args.monitor_host,
             prefix=args.monitor_prefix,
-            disable_auth=args.monitor_disable_auth,
         )
 
         # Upload the directory

lib/spack/spack/container/writers/__init__.py

@@ -183,19 +183,18 @@ def paths(self):
     def monitor(self):
         """Enable using spack monitor during build."""
         Monitor = collections.namedtuple('Monitor', [
-            'enabled', 'host', 'disable_auth', 'prefix', 'keep_going', 'tags'
+            'enabled', 'host', 'prefix', 'keep_going', 'tags'
         ])
         monitor = self.config.get("monitor")
 
         # If we don't have a monitor group, cut out early.
         if not monitor:
-            return Monitor(False, None, None, None, None, None)
+            return Monitor(False, None, None, None, None)
 
         return Monitor(
             enabled=True,
             host=monitor.get('host'),
             prefix=monitor.get('prefix'),
-            disable_auth=monitor.get("disable_auth"),
             keep_going=monitor.get("keep_going"),
             tags=monitor.get('tags')
         )

lib/spack/spack/monitor.py

@@ -38,8 +38,7 @@
 cli = None
 
 
-def get_client(host, prefix="ms1", disable_auth=False, allow_fail=False, tags=None,
+def get_client(host, prefix="ms1", allow_fail=False, tags=None, save_local=False):
-               save_local=False):
     """
     Get a monitor client for a particular host and prefix.
 
@@ -57,8 +56,8 @@ def get_client(host, prefix="ms1", disable_auth=False, allow_fail=False, tags=No
     cli = SpackMonitorClient(host=host, prefix=prefix, allow_fail=allow_fail,
                              tags=tags, save_local=save_local)
 
-    # If we don't disable auth, environment credentials are required
+    # Auth is always required unless we are saving locally
-    if not disable_auth and not save_local:
+    if not save_local:
         cli.require_auth()
 
     # We will exit early if the monitoring service is not running, but
@@ -92,9 +91,6 @@ def get_monitor_group(subparser):
     monitor_group.add_argument(
         '--monitor-save-local', action='store_true', dest='monitor_save_local',
         default=False, help="save monitor results to .spack instead of server.")
-    monitor_group.add_argument(
-        '--monitor-no-auth', action='store_true', dest='monitor_disable_auth',
-        default=False, help="the monitoring server does not require auth.")
     monitor_group.add_argument(
         '--monitor-tags', dest='monitor_tags', default=None,
         help="One or more (comma separated) tags for a build.")

lib/spack/spack/test/monitor.py

@@ -18,18 +18,13 @@
 install = SpackCommand('install')
 
 
-def get_client(host, prefix="ms1", disable_auth=False, allow_fail=False, tags=None,
+def get_client(host, prefix="ms1", allow_fail=False, tags=None, save_local=False):
-               save_local=False):
     """
     We replicate this function to not generate a global client.
     """
     cli = SpackMonitorClient(host=host, prefix=prefix, allow_fail=allow_fail,
                              tags=tags, save_local=save_local)
 
-    # If we don't disable auth, environment credentials are required
-    if not disable_auth and not save_local:
-        cli.require_auth()
-
     # We will exit early if the monitoring service is not running, but
     # only if we aren't doing a local save
     if not save_local:
@@ -131,20 +126,17 @@ def mock_do_request(self, endpoint, *args, **kwargs):
 
 
 def test_spack_monitor_auth(mock_monitor_request):
-    with pytest.raises(SystemExit):
-        get_client(host="http://127.0.0.1")
-
     os.environ["SPACKMON_TOKEN"] = "xxxxxxxxxxxxxxxxx"
     os.environ["SPACKMON_USER"] = "spackuser"
     get_client(host="http://127.0.0.1")
 
 
 def test_spack_monitor_without_auth(mock_monitor_request):
-    get_client(host="hostname", disable_auth=True)
+    get_client(host="hostname")
 
 
 def test_spack_monitor_build_env(mock_monitor_request, install_mockery_mutable_config):
-    monitor = get_client(host="hostname", disable_auth=True)
+    monitor = get_client(host="hostname")
     assert hasattr(monitor, "build_environment")
     for key in ["host_os", "platform", "host_target", "hostname", "spack_version",
                 "kernel_version"]:
@@ -157,7 +149,7 @@ def test_spack_monitor_build_env(mock_monitor_request, install_mockery_mutable_c
 
 
 def test_spack_monitor_basic_auth(mock_monitor_request):
-    monitor = get_client(host="hostname", disable_auth=True)
+    monitor = get_client(host="hostname")
 
     # Headers should be empty
     assert not monitor.headers
@@ -167,7 +159,7 @@ def test_spack_monitor_basic_auth(mock_monitor_request):
 
 
 def test_spack_monitor_new_configuration(mock_monitor_request, install_mockery):
-    monitor = get_client(host="hostname", disable_auth=True)
+    monitor = get_client(host="hostname")
     spec = spack.spec.Spec("dttop")
     spec.concretize()
     response = monitor.new_configuration([spec])
@@ -178,7 +170,7 @@ def test_spack_monitor_new_configuration(mock_monitor_request, install_mockery):
 
 def test_spack_monitor_new_build(mock_monitor_request, install_mockery_mutable_config,
                                  install_mockery):
-    monitor = get_client(host="hostname", disable_auth=True)
+    monitor = get_client(host="hostname")
     spec = spack.spec.Spec("dttop")
     spec.concretize()
     response = monitor.new_build(spec)
@@ -190,7 +182,7 @@ def test_spack_monitor_new_build(mock_monitor_request, install_mockery_mutable_c
 
 def test_spack_monitor_update_build(mock_monitor_request, install_mockery,
                                     install_mockery_mutable_config):
-    monitor = get_client(host="hostname", disable_auth=True)
+    monitor = get_client(host="hostname")
     spec = spack.spec.Spec("dttop")
     spec.concretize()
     response = monitor.update_build(spec, status="SUCCESS")
@@ -200,7 +192,7 @@ def test_spack_monitor_update_build(mock_monitor_request, install_mockery,
 
 def test_spack_monitor_fail_task(mock_monitor_request, install_mockery,
                                  install_mockery_mutable_config):
-    monitor = get_client(host="hostname", disable_auth=True)
+    monitor = get_client(host="hostname")
     spec = spack.spec.Spec("dttop")
     spec.concretize()
     response = monitor.fail_task(spec)
@@ -215,7 +207,7 @@ def test_spack_monitor_send_analyze_metadata(monkeypatch, mock_monitor_request,
     def buildid(*args, **kwargs):
         return 1
     monkeypatch.setattr(spack.monitor.SpackMonitorClient, "get_build_id", buildid)
-    monitor = get_client(host="hostname", disable_auth=True)
+    monitor = get_client(host="hostname")
     spec = spack.spec.Spec("dttop")
     spec.concretize()
     response = monitor.send_analyze_metadata(spec.package, metadata={"boop": "beep"})
@@ -226,7 +218,7 @@ def buildid(*args, **kwargs):
 def test_spack_monitor_send_phase(mock_monitor_request, install_mockery,
                                   install_mockery_mutable_config):
 
-    monitor = get_client(host="hostname", disable_auth=True)
+    monitor = get_client(host="hostname")
 
     def get_build_id(*args, **kwargs):
         return 1

share/spack/spack-completion.bash

@@ -362,7 +362,7 @@ _spack_add() {
 _spack_analyze() {
     if $list_options
     then
-        SPACK_COMPREPLY="-h --help --monitor --monitor-save-local --monitor-no-auth --monitor-tags --monitor-keep-going --monitor-host --monitor-prefix"
+        SPACK_COMPREPLY="-h --help --monitor --monitor-save-local --monitor-tags --monitor-keep-going --monitor-host --monitor-prefix"
     else
         SPACK_COMPREPLY="list-analyzers run"
     fi
@@ -798,7 +798,7 @@ _spack_config_revert() {
 }
 
 _spack_containerize() {
-    SPACK_COMPREPLY="-h --help --monitor --monitor-save-local --monitor-no-auth --monitor-tags --monitor-keep-going --monitor-host --monitor-prefix --list-os --last-stage"
+    SPACK_COMPREPLY="-h --help --monitor --monitor-save-local --monitor-tags --monitor-keep-going --monitor-host --monitor-prefix --list-os --last-stage"
 }
 
 _spack_create() {
@@ -1162,7 +1162,7 @@ _spack_info() {
 _spack_install() {
     if $list_options
     then
-        SPACK_COMPREPLY="-h --help --only -u --until -j --jobs --reuse --overwrite --fail-fast --keep-prefix --keep-stage --dont-restage --use-cache --no-cache --cache-only --monitor --monitor-save-local --monitor-no-auth --monitor-tags --monitor-keep-going --monitor-host --monitor-prefix --include-build-deps --no-check-signature --require-full-hash-match --show-log-on-error --source -n --no-checksum --deprecated -v --verbose --fake --only-concrete --no-add -f --file --clean --dirty --test --run-tests --log-format --log-file --help-cdash --cdash-upload-url --cdash-build --cdash-site --cdash-track --cdash-buildstamp -y --yes-to-all"
+        SPACK_COMPREPLY="-h --help --only -u --until -j --jobs --reuse --overwrite --fail-fast --keep-prefix --keep-stage --dont-restage --use-cache --no-cache --cache-only --monitor --monitor-save-local --monitor-tags --monitor-keep-going --monitor-host --monitor-prefix --include-build-deps --no-check-signature --require-full-hash-match --show-log-on-error --source -n --no-checksum --deprecated -v --verbose --fake --only-concrete --no-add -f --file --clean --dirty --test --run-tests --log-format --log-file --help-cdash --cdash-upload-url --cdash-build --cdash-site --cdash-track --cdash-buildstamp -y --yes-to-all"
     else
         _all_packages
     fi
@@ -1423,7 +1423,7 @@ _spack_module_tcl_setdefault() {
 }
 
 _spack_monitor() {
-    SPACK_COMPREPLY="-h --help --monitor --monitor-save-local --monitor-no-auth --monitor-tags --monitor-keep-going --monitor-host --monitor-prefix"
+    SPACK_COMPREPLY="-h --help --monitor --monitor-save-local --monitor-tags --monitor-keep-going --monitor-host --monitor-prefix"
 }
 
 _spack_patch() {