Merge branch 'main' into PMM-15058-seamless-pmm-ui-redirects

Author: matejkubinec

.github/workflows/linkspector.yml

@@ -31,7 +31,12 @@ jobs:
         uses: browser-actions/setup-chrome@48ad923757ca74d66703209fe939badbdf80f2f4 # v2.2.0
 
       - name: Configure Chrome path for puppeteer
-        run: echo "PUPPETEER_EXECUTABLE_PATH=${{ steps.setup-chrome.outputs.chrome-path }}" >> "$GITHUB_ENV"
+        run: |
+          # ubuntu-24.04 restricts unprivileged user namespaces via AppArmor, which
+          # leaves Chrome with no usable sandbox. linkspector hardcodes its puppeteer
+          # launch args, so restore the namespaces rather than disable the sandbox.
+          sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0
+          echo "PUPPETEER_EXECUTABLE_PATH=${{ steps.setup-chrome.outputs.chrome-path }}" >> "$GITHUB_ENV"
 
       - name: Run linkspector
         uses: umbrelladocs/action-linkspector@568ec8d29fa92b31fd9ea5381e155c51e922af83 # v1.5.5

admin/commands/inventory/change_agent_valkey_exporter.go

@@ -62,7 +62,7 @@ func (res *changeAgentValkeyExporterResult) String() string {
 // ChangeAgentValkeyExporterCommand is used by Kong for CLI flags and commands.
 type ChangeAgentValkeyExporterCommand struct {
 	// Embedded flags
-	flags.LogLevelFatalChangeFlags
+	flags.LogLevelNoFatalChangeFlags
 
 	AgentID string `arg:"" help:"Valkey Exporter Agent ID"`
 

admin/commands/inventory/change_agent_valkey_exporter_test.go

@@ -43,7 +43,7 @@ func TestValkeyExporterChangeAgent(t *testing.T) {
 				Password:    new("redis_pass"),
 				TLS:         new(true),
 				PushMetrics: new(false),
-				LogLevelFatalChangeFlags: flags.LogLevelFatalChangeFlags{
+				LogLevelNoFatalChangeFlags: flags.LogLevelNoFatalChangeFlags{
 					LogLevel: new(flags.LogLevel("debug")),
 				},
 				CustomLabels: &map[string]string{"environment": "test"},
@@ -268,18 +268,25 @@ Configuration changes applied:
 			assert.Contains(t, strings.ToLower(err.Error()), "agent-id")
 		})
 
-		t.Run("InvalidLogLevel", func(t *testing.T) {
-			t.Parallel()
-
-			cli := []string{"change-agent", "valkey-exporter", "test-agent-id", "--log-level=invalid"}
-
-			var cmd ChangeAgentValkeyExporterCommand
-			parser, err := kong.New(&cmd)
-			require.NoError(t, err)
-
-			_, err = parser.Parse(cli[2:])
-			require.Error(t, err)
-			assert.Contains(t, strings.ToLower(err.Error()), "log-level")
-		})
+		// valkey_exporter has no fatal level, so the flag must reject it the same way
+		// `pmm-admin inventory add agent valkey-exporter` does.
+		for name, level := range map[string]string{
+			"InvalidLogLevel":       "invalid",
+			"FatalLogLevelRejected": "fatal",
+		} {
+			t.Run(name, func(t *testing.T) {
+				t.Parallel()
+
+				cli := []string{"change-agent", "valkey-exporter", "test-agent-id", "--log-level=" + level}
+
+				var cmd ChangeAgentValkeyExporterCommand
+				parser, err := kong.New(&cmd)
+				require.NoError(t, err)
+
+				_, err = parser.Parse(cli[2:])
+				require.Error(t, err)
+				assert.Contains(t, strings.ToLower(err.Error()), "log-level")
+			})
+		}
 	})
 }

api-tests/server/updates_test.go

@@ -16,6 +16,7 @@
 package server
 
 import (
+	"net/url"
 	"strings"
 	"testing"
 	"time"
@@ -182,3 +183,33 @@ func TestListUpdates(t *testing.T) {
 		pmmapitests.AssertAPIErrorf(t, err, 400, codes.FailedPrecondition, `PMM updates are disabled`)
 	})
 }
+
+// TestUpdateStatus covers the endpoint pre-3.9 clients poll after triggering an update: on a server
+// that has finished initializing it must report the update as done, without authentication.
+func TestUpdateStatus(t *testing.T) {
+	baseURL, err := url.Parse(pmmapitests.BaseURL.String())
+	require.NoError(t, err)
+	baseURL.User = nil
+	noAuthClient := serverClient.New(pmmapitests.Transport(baseURL, true), nil)
+
+	for _, tc := range []struct {
+		name string
+		body server.UpdateStatusBody
+	}{
+		{"with a token issued by the previous instance", server.UpdateStatusBody{AuthToken: "unverifiable", LogOffset: 1024}},
+		{"without a token", server.UpdateStatusBody{}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			res, err := noAuthClient.ServerService.UpdateStatus(&server.UpdateStatusParams{
+				Body:    tc.body,
+				Context: pmmapitests.Context,
+			})
+			require.NoError(t, err)
+			assert.True(t, res.Payload.Done)
+			// Pre-3.9 clients join log_lines unconditionally, so it must marshal as an empty array.
+			assert.NotNil(t, res.Payload.LogLines)
+			assert.Empty(t, res.Payload.LogLines)
+			assert.Zero(t, res.Payload.LogOffset)
+		})
+	}
+}

api/AGENTS.md

@@ -70,6 +70,21 @@ domain/v1/
 - **`v1`** — stable API, backward-compatible changes only
 - **`v1beta1`** — beta API, may have breaking changes (e.g., `dump/v1beta1/`, `accesscontrol/v1beta1/`, `ha/v1beta1/`)
 
+## REST Path Naming
+
+PMM follows [AIP-122](https://google.aip.dev/122) and [AIP-136](https://google.aip.dev/136) for `google.api.http` paths.
+
+- **Collection identifiers are `lowerCamelCase`** — `/v1/management/enrollmentTokens`, `/v1/advisors/failedServices`. Not kebab-case, not snake_case.
+- **Custom methods use `:verb` in `lowerCamelCase`** — `/v1/inventory/services:getTypes`, `/v1/realtimeanalytics/sessions:start`.
+- **Path parameters keep the proto field name**, which is `snake_case` — `/v1/management/nodes/{node_id}`.
+
+Two deliberate departures from AIP-136:
+
+- **A POST-based read keeps a `get` prefix** — `/v1/qan:getLabels`, `/v1/inventory/services:getTypes`. AIP-136 bars standard method verbs (`get`, `list`, …) from custom methods, but without the prefix `POST /v1/qan:labels` reads like a create. Marking the read intent wins over the rule here.
+- **The URI verb is chosen for HTTP readability, not to mirror the RPC name** — `ListActiveServiceTypes` is exposed as `:getTypes`. AIP-136 requires the two to match; PMM optimizes the path for REST clients instead.
+
+Nothing enforces this: `buf lint` checks proto identifiers, not the path strings inside `google.api.http` annotations. Two older paths predate the convention and are kebab-case (`/v1/backups/{artifact_id}/compatible-services` and `/v1/backups/artifacts/{artifact_id}/pitr-timeranges`). Don't copy them, and don't rename them either — a released path cannot change without breaking clients.
+
 ## Patterns and Conventions
 
 ### Do
@@ -80,6 +95,7 @@ domain/v1/
 - Use `google.api.http` annotations for REST endpoint mapping
 - Use gRPC status codes (`codes.NotFound`, `codes.InvalidArgument`, etc.) not HTTP status codes
 - Follow RESTful conventions for HTTP mappings (GET for reads, POST for creates, PUT for updates, DELETE for deletes)
+- Name REST paths per [REST Path Naming](#rest-path-naming) above
 - Add comments to proto messages and fields — they become API documentation
 
 ### Don't