Validate log --service, --app and --worker names against the deployment

integration-tests/environment_log_test.go

@@ -35,7 +35,30 @@ func TestEnvironmentLogAPI(t *testing.T) {
 			"environments=/projects/"+projectID+"/environments"),
 		DefaultBranch: "main",
 	}})
-	apiHandler.SetEnvironments([]*mockapi.Environment{makeEnv(projectID, "main", "production", "active", nil)})
+	mainEnv := makeEnv(projectID, "main", "production", "active", nil)
+	app := mockapi.App{Type: "golang:1.23", Size: "M", Disk: 2048, Mounts: map[string]mockapi.Mount{}}
+	webApp := func(name string) mockapi.App {
+		a := app
+		a.Name = name
+		return a
+	}
+	worker := func(name string) mockapi.Worker {
+		return mockapi.Worker{App: webApp(name)}
+	}
+	mainEnv.SetCurrentDeployment(&mockapi.Deployment{
+		WebApps: map[string]mockapi.App{"app": webApp("app"), "admin": webApp("admin")},
+		Services: map[string]mockapi.App{
+			"db": {Name: "db", Type: "mariadb:11.4", Size: "S", Disk: 1024},
+		},
+		Workers: map[string]mockapi.Worker{
+			"app--queue":   worker("app--queue"),
+			"admin--queue": worker("admin--queue"),
+			"app--mail":    worker("app--mail"),
+		},
+		Routes: mockRoutes(),
+		Links:  mockapi.MakeHALLinks("self=/projects/" + projectID + "/environments/main/deployment/current"),
+	})
+	apiHandler.SetEnvironments([]*mockapi.Environment{mainEnv})
 
 	// 150 log lines, one per second, ending 20 seconds ago.
 	end := time.Now().Add(-20 * time.Second).UTC().Truncate(time.Second)
@@ -129,11 +152,11 @@ func TestEnvironmentLogAPI(t *testing.T) {
 	assert.Equal(t, "-1", last.Get("log_kinds_mode"))
 
 	// Filters.
-	f.Run("log", "-p", projectID, "-e", "main", "-s", "app,worker", "--severity", "warning", "--since", "5m")
+	f.Run("log", "-p", projectID, "-e", "main", "-s", "app,db", "--severity", "warning", "--since", "5m")
 	mu.Lock()
 	last = queries[len(queries)-1]
 	mu.Unlock()
-	assert.Equal(t, []string{"app", "worker"}, last["services[]"])
+	assert.Equal(t, []string{"app", "db"}, last["services[]"])
 	assert.Equal(t, []string{"EMERGENCY", "ALERT", "CRITICAL", "ERROR", "WARNING"}, last["severities[]"])
 	from, _ := strconv.ParseInt(last.Get("from"), 10, 64)
 	assert.InDelta(t, time.Now().Add(-5*time.Minute).Unix(), from, 60)
@@ -191,6 +214,77 @@ func TestEnvironmentLogAPI(t *testing.T) {
 		assert.Contains(t, stdErr, c.stdErr)
 	}
 
+	// Workers are selected with the logs API, by their qualified name.
+	for _, args := range [][]string{
+		{"--worker", "app--queue"},
+		{"--app", "app", "--worker", "queue"},
+		{"--worker", "mail"},
+	} {
+		_, stdErr, err := f.RunCombinedOutput(append([]string{"log", "-p", projectID, "-e", "main"}, args...)...)
+		require.NoError(t, err, stdErr)
+		assert.NotContains(t, stdErr, "Reading log file")
+		mu.Lock()
+		last = queries[len(queries)-1]
+		mu.Unlock()
+		expected := "app--queue"
+		if args[len(args)-1] == "mail" {
+			expected = "app--mail"
+		}
+		assert.Equal(t, []string{expected}, last["services[]"], args)
+	}
+
+	// Names that are not in the current deployment produce a warning, but are still queried.
+	validNames := "admin, admin--queue, app, app--mail, app--queue, db, router"
+	for _, c := range []struct {
+		args     []string
+		services []string
+		hint     bool
+	}{
+		{args: []string{"-s", "access"}, services: []string{"access"}, hint: true},
+		{args: []string{"-s", "db,old"}, services: []string{"db", "old"}},
+		{args: []string{"--app", "old"}, services: []string{"old"}},
+		{args: []string{"--worker", "app--old"}, services: []string{"app--old"}},
+		{args: []string{"--app", "app", "--worker", "old"}, services: []string{"app--old"}},
+	} {
+		_, stdErr, err := f.RunCombinedOutput(append([]string{"log", "-p", projectID, "-e", "main"}, c.args...)...)
+		require.NoError(t, err, stdErr)
+		unknown := c.services[len(c.services)-1]
+		assert.Contains(t, stdErr, "Warning: the service "+unknown+
+			" is not in the current deployment (expected one of: "+validNames+")")
+		if c.hint {
+			assert.Contains(t, stdErr, "To filter by log type, use it as an argument, e.g.: platform-test log "+unknown)
+		} else {
+			assert.NotContains(t, stdErr, "To filter by log type")
+		}
+		mu.Lock()
+		last = queries[len(queries)-1]
+		mu.Unlock()
+		assert.Equal(t, c.services, last["services[]"], c.args)
+	}
+
+	// Short worker names that cannot be qualified are errors.
+	for _, c := range []struct {
+		args   []string
+		stdErr string
+	}{
+		{args: []string{"--worker", "queue"}, stdErr: "Ambiguous worker name: queue (matches: admin--queue, app--queue)"},
+		{args: []string{"--worker", "missing"}, stdErr: "Worker not found: missing"},
+		{
+			args:   []string{"--app", "admin", "--worker", "app--queue"},
+			stdErr: `App name "admin" conflicts with worker name "app--queue"`,
+		},
+	} {
+		mu.Lock()
+		count := len(queries)
+		mu.Unlock()
+		_, stdErr, err := f.RunCombinedOutput(append([]string{"log", "-p", projectID, "-e", "main"}, c.args...)...)
+		assert.Error(t, err, c.args)
+		assert.Contains(t, stdErr, c.stdErr)
+		mu.Lock()
+		assert.Len(t, queries, count, "no query is sent: %v", c.args)
+		mu.Unlock()
+	}
+
 	// The app can be selected by an environment variable.
 	f.extraEnv = []string{EnvPrefix + "LOG_PROTOCOL=auto", "PLATFORM_APPLICATION_NAME=app"}
 	f.Run("log", "-p", projectID, "-e", "main")

legacy/src/Command/Environment/EnvironmentLogCommand.php

@@ -9,6 +9,7 @@
 use Platformsh\Cli\Console\Option;
 use Platformsh\Cli\Selector\Selection;
 use Platformsh\Cli\Selector\SelectorConfig;
+use Platformsh\Cli\Service\Api;
 use Platformsh\Cli\Service\Config;
 use Platformsh\Cli\Service\Io;
 use Platformsh\Cli\Service\Observability;
@@ -19,6 +20,7 @@
 use Platformsh\Cli\Command\CommandBase;
 use Platformsh\Cli\Util\OsUtil;
 use Platformsh\Cli\Util\StringUtil;
+use Platformsh\Client\Exception\EnvironmentStateException;
 use Symfony\Component\Console\Attribute\AsCommand;
 use Symfony\Component\Console\Exception\InvalidArgumentException;
 use Symfony\Component\Console\Formatter\OutputFormatter;
@@ -87,6 +89,7 @@ class EnvironmentLogCommand extends CommandBase
     private const WINDOW_GROWTH = 4;
 
     public function __construct(
+        private readonly Api $api,
         private readonly CacheProvider $cacheProvider,
         private readonly Config $config,
         private readonly Io $io,
@@ -137,7 +140,7 @@ protected function configure(): void
               Use --fields to choose the fields, or --format json to see everything.
               Logs take a few seconds to become available, so --tail shows them with a delay of about 15-20 seconds.
               Logs that become available later are shown when they arrive, unless they are over a minute late.
-              SSH is still used if the --worker, --instance or --task option is used,
+              SSH is still used if the --instance or --task option is used,
               or if the type is not listed above or ends in .log (e.g. "error.log").
             EOF);
         $this->addExample('Display a choice of logs that can be read');
@@ -212,7 +215,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
      */
     private function sshReason(InputInterface $input, ?string $logType): ?string
     {
-        foreach (['worker', 'instance', 'task'] as $option) {
+        foreach (['instance', 'task'] as $option) {
             if (Option::stringOrNull($input, $option) !== null) {
                 return sprintf('the --%s option is used', $option);
             }
@@ -434,13 +437,9 @@ private function buildFilters(InputInterface $input, Selection $selection, ?stri
             $params[] = ['severities_mode', '1'];
         }
 
-        $services = ArrayArgument::getOption($input, 'service');
-        // The app may also come from the project URL or an environment variable.
-        if (($app = $selection->getAppName()) !== null) {
-            $services[] = $app;
-        }
+        $services = $this->resolveServices($input, $selection);
         if ($services !== []) {
-            foreach (array_unique($services) as $service) {
+            foreach ($services as $service) {
                 $params[] = ['services[]', $service];
             }
             $params[] = ['services_mode', '1'];
@@ -449,6 +448,74 @@ private function buildFilters(InputInterface $input, Selection $selection, ?stri
         return $params;
     }
 
+    /**
+     * Returns the service names to filter logs by, from the --service, --app and --worker options.
+     *
+     * Names that are not in the environment's current deployment produce a
+     * warning, as logs of removed services can still be read.
+     *
+     * @return list<string>
+     */
+    private function resolveServices(InputInterface $input, Selection $selection): array
+    {
+        $services = ArrayArgument::getOption($input, 'service');
+        // The app may also come from the project URL or an environment variable.
+        $appName = $selection->getAppName();
+        $worker = $input->hasOption('worker') ? Option::stringOrNull($input, 'worker') : null;
+        if ($worker === '') {
+            $worker = null;
+        }
+        if ($services === [] && $appName === null && $worker === null) {
+            return [];
+        }
+
+        try {
+            $deployment = $this->api->getCurrentDeployment($selection->getEnvironment());
+        } catch (EnvironmentStateException $e) {
+            $this->io->debug('Not validating service names: ' . $e->getMessage());
+            $deployment = null;
+        }
+
+        if ($worker !== null) {
+            // The app name is only used to qualify the worker name.
+            $qualified = $this->selector->qualifyWorkerName($worker, $appName);
+            if ($qualified === null) {
+                if ($deployment === null) {
+                    throw new InvalidArgumentException(sprintf('The worker name must include the app name, e.g.: --worker <app>--%s', $worker));
+                }
+                $qualified = $this->selector->selectWorkerName($deployment, $input, $worker, null);
+            }
+            $services[] = $qualified;
+        } elseif ($appName !== null) {
+            $services[] = $appName;
+        }
+        $services = array_values(array_unique($services));
+
+        if ($deployment === null) {
+            return $services;
+        }
+
+        $validNames = array_merge(
+            array_map('strval', array_keys($deployment->webapps)),
+            array_map('strval', array_keys($deployment->workers)),
+            array_map('strval', array_keys($deployment->services)),
+            ['router'],
+        );
+        $validNames = array_values(array_unique($validNames));
+        sort($validNames);
+        foreach ($services as $service) {
+            if (in_array($service, $validNames, true)) {
+                continue;
+            }
+            $this->stdErr->writeln(sprintf('<comment>Warning:</comment> the service <comment>%s</comment> is not in the current deployment (expected one of: %s)', $service, implode(', ', $validNames)));
+            if ($service === 'error' || isset(self::LOG_KINDS[$service])) {
+                $this->stdErr->writeln(sprintf('To filter by log type, use it as an argument, e.g.: <info>%s log %s</info>', $this->config->getStr('application.executable'), $service));
+            }
+        }
+
+        return $services;
+    }
+
     /**
      * Queries a page of logs.
      *