From 659b9c43c5396804c2e44353fc92e05e5dca2772 Mon Sep 17 00:00:00 2001 From: Volodymyr Kolesnykov Date: Fri, 9 Oct 2026 17:43:03 +0300 Subject: [PATCH 1/2] fix: clarify phpMyAdmin access permissions by platform Address PLTFRM-1650 in both Node.js and Go command notes and help text. --- __tests__/commands/phpmyadmin.ts | 21 +++++ cmd/vip-next/commands/db_phpmyadmin.go | 8 +- cmd/vip-next/commands/db_phpmyadmin_test.go | 12 +++ internal/parity/phpmyadmin_scenario_test.go | 92 ++++++++++++++++++++- src/bin/vip-db-phpmyadmin.ts | 3 +- src/bin/vip-db.ts | 4 +- src/commands/phpmyadmin.ts | 2 +- testdata/parity/phpmyadmin-error.yaml | 2 +- testdata/parity/phpmyadmin-print.yaml | 2 +- testdata/parity/phpmyadmin-silent.yaml | 4 +- 10 files changed, 136 insertions(+), 14 deletions(-) diff --git a/__tests__/commands/phpmyadmin.ts b/__tests__/commands/phpmyadmin.ts index 98383aa19..8b5135bd7 100644 --- a/__tests__/commands/phpmyadmin.ts +++ b/__tests__/commands/phpmyadmin.ts @@ -70,6 +70,27 @@ describe( 'commands/PhpMyAdminCommand', () => { openUrl.mockReset(); } ); + it.each( [ false, true ] )( + 'prints the platform access note unless silent=%s', + async silent => { + const noteCmd = new PhpMyAdminCommand( app, env, tracker, silent ); + const consoleSpy = jest.spyOn( console, 'log' ).mockImplementation( () => {} ); + try { + await noteCmd.run( { print: true } ); + const notes = consoleSpy.mock.calls.filter( + ( [ message ] ) => + typeof message === 'string' && + message.includes( + 'Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud.' + ) + ); + expect( notes ).toHaveLength( silent ? 0 : 1 ); + } finally { + consoleSpy.mockRestore(); + } + } + ); + it( 'should open the generated URL in browser', async () => { await cmd.run(); expect( pmaEnabledQueryMockTrue ).toHaveBeenCalledWith( { diff --git a/cmd/vip-next/commands/db_phpmyadmin.go b/cmd/vip-next/commands/db_phpmyadmin.go index 2e988f809..9369552a2 100644 --- a/cmd/vip-next/commands/db_phpmyadmin.go +++ b/cmd/vip-next/commands/db_phpmyadmin.go @@ -27,8 +27,8 @@ var openURLFn = browser.OpenURL func DBPhpmyadminCmd() *cobra.Command { cmd := &cobra.Command{ Use: "phpmyadmin", - Short: "Generate access to a read-only phpMyAdmin web interface", - Long: "Generate access to a read-only phpMyAdmin web interface for the environment's database.\n\n" + + Short: "Generate access to a phpMyAdmin web interface", + Long: "Generate access to a phpMyAdmin web interface for the environment's database.\n\n" + "By default the URL is opened in your browser. Use --print to write it to stdout instead.", } cmd.Flags().BoolP("print", "p", false, "Print the phpMyAdmin URL to stdout instead of opening it in a browser.") @@ -90,12 +90,12 @@ func runDBPhpmyadmin(cmd *cobra.Command, args []string) error { "env": ae.Env.ID, }) - // Node prints a yellow warning that PMA sessions are read-only before + // Node prints a yellow note describing platform-specific access before // kicking the progress tracker. We match that here, but on stderr to // keep stdout clean for --print consumers. if !silent { fmt.Fprintln(cmd.ErrOrStderr(), color.YellowString( - "Note: PHPMyAdmin sessions are read-only. If you run a query that writes to DB, it will fail.")) + "Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud.")) } res, err := phpmyadmin.Run(cmd.Context(), cfg.GQLClient, ae.App.ID, ae.Env.ID, phpmyadmin.RunOpts{ diff --git a/cmd/vip-next/commands/db_phpmyadmin_test.go b/cmd/vip-next/commands/db_phpmyadmin_test.go index 0f474c995..de9563c9c 100644 --- a/cmd/vip-next/commands/db_phpmyadmin_test.go +++ b/cmd/vip-next/commands/db_phpmyadmin_test.go @@ -102,6 +102,18 @@ func TestDBPhpmyadminPrintWritesURLToStdout(t *testing.T) { if opened != "" { t.Errorf("--print must not open browser; got %q", opened) } + if !strings.Contains(stderr.String(), "Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud.") { + t.Errorf("missing platform-specific access note: %q", stderr.String()) + } +} + +func TestDBPhpmyadminHelpIsPlatformNeutral(t *testing.T) { + cmd := DBPhpmyadminCmd() + for _, description := range []string{cmd.Short, cmd.Long} { + if strings.Contains(description, "read-only") || !strings.Contains(description, "phpMyAdmin web interface") { + t.Errorf("misleading command description: %q", description) + } + } } func TestDBPhpmyadminPrintSilentSuppressesStderr(t *testing.T) { diff --git a/internal/parity/phpmyadmin_scenario_test.go b/internal/parity/phpmyadmin_scenario_test.go index b8e477353..6d261ce0e 100644 --- a/internal/parity/phpmyadmin_scenario_test.go +++ b/internal/parity/phpmyadmin_scenario_test.go @@ -128,7 +128,7 @@ func TestPhpmyadminPrintParity(t *testing.T) { } // TestPhpmyadminSilentParity exercises --print --silent: URL still lands on -// stdout, stderr stays empty (no progress lines, no read-only warning). +// stdout, stderr stays empty (no progress lines or access note). func TestPhpmyadminSilentParity(t *testing.T) { mux, _ := phpmyadminMux(t, "phpmyadmin-silent") srv := httptest.NewServer(mux) @@ -171,6 +171,96 @@ func TestPhpmyadminSilentParity(t *testing.T) { } } +func TestPhpmyadminSessionNoteDifferential(t *testing.T) { + rig, skip := differentialAvailable(t) + if skip != "" { + t.Skip(LoudSkip("TestPhpmyadminSessionNoteDifferential — the phpMyAdmin Node-vs-Go note", skip)) + } + + const note = "Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud." + for _, tc := range []struct { + name string + wantNote bool + }{ + {name: "phpmyadmin-print", wantNote: true}, + {name: "phpmyadmin-silent", wantNote: false}, + } { + t.Run(tc.name, func(t *testing.T) { + scenario, err := LoadScenario("../../testdata/parity/" + tc.name + ".yaml") + if err != nil { + t.Fatalf("LoadScenario: %v", err) + } + scenario.Env = rig.scenarioEnv(scenario) + + for _, side := range []struct { + name string + bin string + }{ + {name: "Node", bin: rig.nodeBin}, + {name: "Go", bin: rig.goBin}, + } { + res, _ := rig.runSide(t, scenario, side.bin, phpmyadminSurfaceMux) + if res.ExitCode != 0 { + t.Errorf("%s exit = %d; stdout=%q stderr=%q", side.name, res.ExitCode, res.Stdout, res.Stderr) + } + output := res.Stdout + res.Stderr + count := 0 + for _, line := range strings.Split(output, "\n") { + if strings.TrimSpace(line) == note { + count++ + } + } + if tc.wantNote && count != 1 { + t.Errorf("%s note count = %d, want 1; stdout=%q stderr=%q", side.name, count, res.Stdout, res.Stderr) + } + if !tc.wantNote && (count != 0 || strings.Contains(output, "Note:")) { + t.Errorf("%s --silent printed a note; stdout=%q stderr=%q", side.name, res.Stdout, res.Stderr) + } + if strings.Contains(output, "Note: PHPMyAdmin sessions are read-only.") { + t.Errorf("%s printed the obsolete blanket warning: %q", side.name, output) + } + } + }) + } + + for _, help := range []struct { + name string + argv []string + }{ + {name: "db-help", argv: []string{"db", "--help"}}, + {name: "phpmyadmin-help", argv: []string{"db", "phpmyadmin", "--help"}}, + } { + t.Run(help.name, func(t *testing.T) { + for _, side := range []struct { + name string + bin string + }{ + {name: "Node", bin: rig.nodeBin}, + {name: "Go", bin: rig.goBin}, + } { + res, err := Run(RunSpec{ + Binary: side.bin, + Argv: help.argv, + Env: FixtureEnv(rig.scenarioEnv(&Scenario{})), + }) + if err != nil { + t.Fatalf("run %s help: %v", side.name, err) + } + if res.ExitCode != 0 { + t.Errorf("%s help exit = %d; stdout=%q stderr=%q", side.name, res.ExitCode, res.Stdout, res.Stderr) + } + output := strings.ToLower(res.Stdout + res.Stderr) + if !strings.Contains(output, "phpmyadmin") { + t.Errorf("%s help missing phpMyAdmin description: stdout=%q stderr=%q", side.name, res.Stdout, res.Stderr) + } + if strings.Contains(output, "read-only phpmyadmin") { + t.Errorf("%s help retains blanket read-only description: stdout=%q stderr=%q", side.name, res.Stdout, res.Stderr) + } + } + }) + } +} + // TestPhpmyadminErrorParity: enable mutation returns a GraphQL error; // the CLI must exit non-zero. func TestPhpmyadminErrorParity(t *testing.T) { diff --git a/src/bin/vip-db-phpmyadmin.ts b/src/bin/vip-db-phpmyadmin.ts index d52dd9b99..7491b2fe4 100755 --- a/src/bin/vip-db-phpmyadmin.ts +++ b/src/bin/vip-db-phpmyadmin.ts @@ -15,8 +15,7 @@ import { makeCommandTracker } from '../lib/tracker'; const examples = [ { usage: 'vip @example-app.develop db phpmyadmin', - description: - "Generate access to a read-only phpMyAdmin web interface for the environment's database.", + description: "Generate access to a phpMyAdmin web interface for the environment's database.", }, { usage: 'vip @example-app.develop db phpmyadmin --print', diff --git a/src/bin/vip-db.ts b/src/bin/vip-db.ts index 2e3d75d12..a7af73c95 100755 --- a/src/bin/vip-db.ts +++ b/src/bin/vip-db.ts @@ -13,11 +13,11 @@ import { trackEvent } from '../lib/tracker'; void command( { usage: 'vip db' } ) .command( 'phpmyadmin', - 'Generate access to a read-only phpMyAdmin web interface for an environment database.' + 'Generate access to a phpMyAdmin web interface for an environment database.' ) .example( 'vip @example-app.develop db phpmyadmin', - "Generate access to a read-only phpMyAdmin web interface for the environment's database." + "Generate access to a phpMyAdmin web interface for the environment's database." ) .argv( process.argv, async () => { await trackEvent( 'vip_db_command_execute' ); diff --git a/src/commands/phpmyadmin.ts b/src/commands/phpmyadmin.ts index e0684f1f7..889b27c89 100644 --- a/src/commands/phpmyadmin.ts +++ b/src/commands/phpmyadmin.ts @@ -232,7 +232,7 @@ export class PhpMyAdminCommand { if ( ! this.silent ) { const message = - 'Note: PHPMyAdmin sessions are read-only. If you run a query that writes to DB, it will fail.'; + 'Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud.'; console.log( chalk.yellow( message ) ); } diff --git a/testdata/parity/phpmyadmin-error.yaml b/testdata/parity/phpmyadmin-error.yaml index 23aaca6eb..dfd4c2288 100644 --- a/testdata/parity/phpmyadmin-error.yaml +++ b/testdata/parity/phpmyadmin-error.yaml @@ -13,7 +13,7 @@ recording: phpmyadmin-error expect: exit_code: 1 expected_drift: - signature: "574bd9a5ca09ffbbe366a192669e62efcd01849318d28fd11b06aa22cac38f85" + signature: "d2f22910c5410309a936634f319e89b9bc38f424d383a61444638f562b04f12a" reason: >- parity register 1.10 and 1.26 (KEEP): vip-next sends phpMyAdmin progress to stderr and emits a clean error without Node's runtime Debug line; both map diff --git a/testdata/parity/phpmyadmin-print.yaml b/testdata/parity/phpmyadmin-print.yaml index d4eb7b7aa..18cc69980 100644 --- a/testdata/parity/phpmyadmin-print.yaml +++ b/testdata/parity/phpmyadmin-print.yaml @@ -13,7 +13,7 @@ recording: phpmyadmin-print expect: exit_code: 0 expected_drift: - signature: "879aafb8109cb49122e457224e5e9359dce7ab6b810e81d86360d6e41fb4f2f5" + signature: "d0eec49aee54cb06a0b1759bf3e8308dabeeac97f4bca7978ff9d91cec94481b" reason: >- parity register 1.26 (KEEP): vip-next keeps --print stdout URL-only and sends human progress to stderr; Node interleaves progress and the URL on diff --git a/testdata/parity/phpmyadmin-silent.yaml b/testdata/parity/phpmyadmin-silent.yaml index 8efc02264..c0896a27e 100644 --- a/testdata/parity/phpmyadmin-silent.yaml +++ b/testdata/parity/phpmyadmin-silent.yaml @@ -2,8 +2,8 @@ name: phpmyadmin-silent description: | `vip @parityapp.develop db phpmyadmin --print --silent`. The URL must still land on stdout (it's the documented output channel for --print), - but the progress lines and the yellow "read-only" warning must be - suppressed on stderr. + but the progress lines and the environment-specific access note must be + suppressed from output. argv: ["@parityapp.develop", "db", "phpmyadmin", "--print", "--silent"] env: DO_NOT_TRACK: "1" From a84a2207ea1c2593e3e4ddc92b3a00a1793c5886 Mon Sep 17 00:00:00 2001 From: Volodymyr Kolesnykov Date: Fri, 9 Oct 2026 17:53:28 +0300 Subject: [PATCH 2/2] fix: show phpMyAdmin warning only for read-only environments --- __tests__/commands/phpmyadmin.ts | 36 +++-- cmd/vip-next/commands/db_phpmyadmin.go | 9 +- cmd/vip-next/commands/db_phpmyadmin_test.go | 50 ++++++ internal/appctx/app_resolver.go | 2 + internal/appctx/app_resolver_test.go | 62 +++++++ internal/appctx/context.go | 5 +- internal/gql/generated.go | 16 ++ internal/gql/operations/app_resolve.graphql | 2 + internal/parity/phpmyadmin_scenario_test.go | 171 +++++++++++++++----- src/bin/vip-db-phpmyadmin.ts | 1 + src/commands/phpmyadmin.ts | 8 +- 11 files changed, 308 insertions(+), 54 deletions(-) diff --git a/__tests__/commands/phpmyadmin.ts b/__tests__/commands/phpmyadmin.ts index 8b5135bd7..d3c180a1c 100644 --- a/__tests__/commands/phpmyadmin.ts +++ b/__tests__/commands/phpmyadmin.ts @@ -70,21 +70,37 @@ describe( 'commands/PhpMyAdminCommand', () => { openUrl.mockReset(); } ); - it.each( [ false, true ] )( - 'prints the platform access note unless silent=%s', - async silent => { - const noteCmd = new PhpMyAdminCommand( app, env, tracker, silent ); + it.each( + [ + { + platform: { isK8sResident: true }, + note: 'Note: phpMyAdmin sessions are read-only. If you run a query that writes to DB, it will fail.', + }, + { + platform: { isK8sResident: false }, + note: '', + }, + { + platform: { isK8sResident: null }, + note: 'Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud.', + }, + { + platform: {}, + note: 'Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud.', + }, + ].flatMap( testCase => [ false, true ].map( silent => ( { ...testCase, silent } ) ) ) + )( + 'prints the appropriate access note for $platform with silent=$silent', + async ( { platform, note, silent } ) => { + const noteCmd = new PhpMyAdminCommand( app, { ...env, ...platform }, tracker, silent ); const consoleSpy = jest.spyOn( console, 'log' ).mockImplementation( () => {} ); try { await noteCmd.run( { print: true } ); const notes = consoleSpy.mock.calls.filter( - ( [ message ] ) => - typeof message === 'string' && - message.includes( - 'Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud.' - ) + ( [ message ] ) => typeof message === 'string' && message.includes( 'Note:' ) ); - expect( notes ).toHaveLength( silent ? 0 : 1 ); + const expectedNote = expect.stringContaining( note ); + expect( notes ).toEqual( silent || ! note ? [] : [ [ expectedNote ] ] ); } finally { consoleSpy.mockRestore(); } diff --git a/cmd/vip-next/commands/db_phpmyadmin.go b/cmd/vip-next/commands/db_phpmyadmin.go index 9369552a2..891de39c5 100644 --- a/cmd/vip-next/commands/db_phpmyadmin.go +++ b/cmd/vip-next/commands/db_phpmyadmin.go @@ -93,9 +93,12 @@ func runDBPhpmyadmin(cmd *cobra.Command, args []string) error { // Node prints a yellow note describing platform-specific access before // kicking the progress tracker. We match that here, but on stderr to // keep stdout clean for --print consumers. - if !silent { - fmt.Fprintln(cmd.ErrOrStderr(), color.YellowString( - "Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud.")) + if !silent && (ae.Env.IsK8sResident == nil || *ae.Env.IsK8sResident) { + note := "Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud." + if ae.Env.IsK8sResident != nil { + note = "Note: phpMyAdmin sessions are read-only. If you run a query that writes to DB, it will fail." + } + fmt.Fprintln(cmd.ErrOrStderr(), color.YellowString(note)) } res, err := phpmyadmin.Run(cmd.Context(), cfg.GQLClient, ae.App.ID, ae.Env.ID, phpmyadmin.RunOpts{ diff --git a/cmd/vip-next/commands/db_phpmyadmin_test.go b/cmd/vip-next/commands/db_phpmyadmin_test.go index de9563c9c..665fef5a0 100644 --- a/cmd/vip-next/commands/db_phpmyadmin_test.go +++ b/cmd/vip-next/commands/db_phpmyadmin_test.go @@ -2,6 +2,7 @@ package commands import ( "bytes" + "context" "io" "net/http" "net/http/httptest" @@ -12,6 +13,7 @@ import ( "github.com/Khan/genqlient/graphql" + "github.com/Automattic/vip/internal/appctx" "github.com/Automattic/vip/internal/phpmyadmin" ) @@ -107,6 +109,54 @@ func TestDBPhpmyadminPrintWritesURLToStdout(t *testing.T) { } } +func TestDBPhpmyadminNoteFollowsK8sResidence(t *testing.T) { + trueValue, falseValue := true, false + for _, tc := range []struct { + name string + k8s *bool + want string + }{ + {"kubernetes", &trueValue, "Note: phpMyAdmin sessions are read-only. If you run a query that writes to DB, it will fail."}, + {"wp-cloud", &falseValue, ""}, + {"unknown", nil, "Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud."}, + } { + t.Run(tc.name, func(t *testing.T) { + stub := successStub() + srv := httptest.NewServer(stub.handler(t)) + defer srv.Close() + cleanup := setupPhpmyadminConfig(srv, nil) + defer cleanup() + + cmd := DBPhpmyadminCmd() + _ = cmd.Flags().Set("print", "true") + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetContext(appctx.WithAppEnv(context.Background(), &appctx.AppEnv{ + App: appctx.App{ID: 1}, + Env: appctx.Env{ID: 2, IsK8sResident: tc.k8s}, + })) + + if err := runDBPhpmyadmin(cmd, nil); err != nil { + t.Fatalf("runDBPhpmyadmin: %v", err) + } + if strings.TrimSpace(stdout.String()) != "https://pma.example/abc" { + t.Errorf("stdout = %q, want URL", stdout.String()) + } + var got string + for _, line := range strings.Split(stderr.String(), "\n") { + if strings.HasPrefix(line, "Note:") { + got = line + break + } + } + if got != tc.want { + t.Errorf("note = %q, want %q", got, tc.want) + } + }) + } +} + func TestDBPhpmyadminHelpIsPlatformNeutral(t *testing.T) { cmd := DBPhpmyadminCmd() for _, description := range []string{cmd.Short, cmd.Long} { diff --git a/internal/appctx/app_resolver.go b/internal/appctx/app_resolver.go index 6f5d7a072..4828f73ae 100644 --- a/internal/appctx/app_resolver.go +++ b/internal/appctx/app_resolver.go @@ -76,6 +76,7 @@ type envGetter interface { GetUniqueLabel() *string GetDefaultDomain() *string GetIsMultisite() *bool + GetIsK8sResident() *bool } func resolveApp(cmd *cobra.Command, client graphql.Client, appKey, envKey string) (App, []Env, error) { @@ -168,6 +169,7 @@ func envsFromGetters(getters []envGetter) []Env { if multisite := g.GetIsMultisite(); multisite != nil { e.IsMultisite = *multisite } + e.IsK8sResident = g.GetIsK8sResident() if e.ID == 0 && e.Name == "" && e.Type == "" { continue } diff --git a/internal/appctx/app_resolver_test.go b/internal/appctx/app_resolver_test.go index 773d786a5..721db8af5 100644 --- a/internal/appctx/app_resolver_test.go +++ b/internal/appctx/app_resolver_test.go @@ -2,6 +2,7 @@ package appctx import ( "context" + "io" "net/http" "net/http/httptest" "strings" @@ -97,6 +98,67 @@ func TestWithAppContextResolvesByID(t *testing.T) { } } +func TestWithAppContextCarriesNullableK8sResidence(t *testing.T) { + for _, lookup := range []struct { + name string + appKey string + data func(string) string + }{ + {"name", "myapp", func(env string) string { + return `{"data":{"apps":{"edges":[{"id":42,"name":"myapp","environments":[` + env + `]}]}}}` + }}, + {"id", "42", func(env string) string { + return `{"data":{"app":{"id":42,"name":"myapp","environments":[` + env + `]}}}` + }}, + } { + for _, field := range []struct { + name string + json string + want *bool + }{ + {"true", `,"isK8sResident":true`, func() *bool { v := true; return &v }()}, + {"false", `,"isK8sResident":false`, func() *bool { v := false; return &v }()}, + {"null", `,"isK8sResident":null`, nil}, + {"missing", "", nil}, + } { + t.Run(lookup.name+"/"+field.name, func(t *testing.T) { + env := `{"id":7,"name":"develop","type":"develop"` + field.json + `}` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("read request: %v", err) + } + if !strings.Contains(string(body), "isK8sResident") { + t.Errorf("request did not select isK8sResident: %s", body) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(lookup.data(env))) + })) + defer srv.Close() + + cmd := makeAppCmd(lookup.appKey, "", true) + run := WithAppContext(AppContextConfig{Client: gqlClientForServer(srv)})(func(cmd *cobra.Command, _ []string) error { + envs := FromContext(cmd.Context()).AvailableEnvs() + if len(envs) != 1 { + t.Fatalf("AvailableEnvs = %+v, want one env", envs) + } + got := envs[0].IsK8sResident + if got == nil && field.want == nil { + return nil + } + if got == nil || field.want == nil || *got != *field.want { + t.Errorf("IsK8sResident = %v, want %v", got, field.want) + } + return nil + }) + if err := run(cmd, nil); err != nil { + t.Fatalf("run: %v", err) + } + }) + } + } +} + func TestWithAppContextMissingAppNonInteractive(t *testing.T) { cmd := makeAppCmd("", "", true) mw := WithAppContext(AppContextConfig{Client: nil}) diff --git a/internal/appctx/context.go b/internal/appctx/context.go index 8b294362e..ac76076d5 100644 --- a/internal/appctx/context.go +++ b/internal/appctx/context.go @@ -34,8 +34,9 @@ type Env struct { DefaultDomain string // UniqueLabel is the env's dashboard slug (e.g. "develop"); used in // dashboard URLs by export sql and app deploy. - UniqueLabel string - IsMultisite bool + UniqueLabel string + IsMultisite bool + IsK8sResident *bool } // AppEnv pairs the resolved App with its target Env. Either field may be diff --git a/internal/gql/generated.go b/internal/gql/generated.go index 928d6b6b8..8fb10ce65 100644 --- a/internal/gql/generated.go +++ b/internal/gql/generated.go @@ -4727,6 +4727,8 @@ type ResolveAppByIDAppEnvironmentsAppEnvironment struct { DefaultDomain *string `json:"defaultDomain"` // Whether the environment is a multisite install. IsMultisite *bool `json:"isMultisite"` + // Whether the environment runs on Kubernetes. + IsK8sResident *bool `json:"isK8sResident"` } // GetId returns ResolveAppByIDAppEnvironmentsAppEnvironment.Id, and is useful for accessing the field via an interface. @@ -4752,6 +4754,11 @@ func (v *ResolveAppByIDAppEnvironmentsAppEnvironment) GetDefaultDomain() *string // GetIsMultisite returns ResolveAppByIDAppEnvironmentsAppEnvironment.IsMultisite, and is useful for accessing the field via an interface. func (v *ResolveAppByIDAppEnvironmentsAppEnvironment) GetIsMultisite() *bool { return v.IsMultisite } +// GetIsK8sResident returns ResolveAppByIDAppEnvironmentsAppEnvironment.IsK8sResident, and is useful for accessing the field via an interface. +func (v *ResolveAppByIDAppEnvironmentsAppEnvironment) GetIsK8sResident() *bool { + return v.IsK8sResident +} + // ResolveAppByIDResponse is returned by ResolveAppByID on success. type ResolveAppByIDResponse struct { // Retrieve a single application. @@ -4828,6 +4835,8 @@ type ResolveAppByNameAppsAppListEdgesAppEnvironmentsAppEnvironment struct { DefaultDomain *string `json:"defaultDomain"` // Whether the environment is a multisite install. IsMultisite *bool `json:"isMultisite"` + // Whether the environment runs on Kubernetes. + IsK8sResident *bool `json:"isK8sResident"` } // GetId returns ResolveAppByNameAppsAppListEdgesAppEnvironmentsAppEnvironment.Id, and is useful for accessing the field via an interface. @@ -4863,6 +4872,11 @@ func (v *ResolveAppByNameAppsAppListEdgesAppEnvironmentsAppEnvironment) GetIsMul return v.IsMultisite } +// GetIsK8sResident returns ResolveAppByNameAppsAppListEdgesAppEnvironmentsAppEnvironment.IsK8sResident, and is useful for accessing the field via an interface. +func (v *ResolveAppByNameAppsAppListEdgesAppEnvironmentsAppEnvironment) GetIsK8sResident() *bool { + return v.IsK8sResident +} + // ResolveAppByNameResponse is returned by ResolveAppByName on success. type ResolveAppByNameResponse struct { // Retrieve a paginated list of applications. @@ -9472,6 +9486,7 @@ query ResolveAppByID ($id: Int!) { uniqueLabel defaultDomain isMultisite + isK8sResident } } } @@ -9519,6 +9534,7 @@ query ResolveAppByName ($name: String!) { uniqueLabel defaultDomain isMultisite + isK8sResident } } } diff --git a/internal/gql/operations/app_resolve.graphql b/internal/gql/operations/app_resolve.graphql index d78909e7a..925dbc546 100644 --- a/internal/gql/operations/app_resolve.graphql +++ b/internal/gql/operations/app_resolve.graphql @@ -13,6 +13,7 @@ query ResolveAppByName($name: String!) { uniqueLabel defaultDomain isMultisite + isK8sResident } } } @@ -32,6 +33,7 @@ query ResolveAppByID($id: Int!) { uniqueLabel defaultDomain isMultisite + isK8sResident } } } diff --git a/internal/parity/phpmyadmin_scenario_test.go b/internal/parity/phpmyadmin_scenario_test.go index 6d261ce0e..bc241ac1c 100644 --- a/internal/parity/phpmyadmin_scenario_test.go +++ b/internal/parity/phpmyadmin_scenario_test.go @@ -3,6 +3,7 @@ package parity import ( + "bytes" "io" "net/http" "net/http/httptest" @@ -10,6 +11,8 @@ import ( "strings" "sync/atomic" "testing" + + json "encoding/json/v2" ) // phpmyadminMux returns a shared HTTP handler that answers the operations the @@ -87,6 +90,84 @@ func phpmyadminMux(t *testing.T, recordingDir string) (http.Handler, func() (en, return mux, hits } +func phpmyadminResidencyMux(t *testing.T, recordingDir, residency string) (http.Handler, func() map[string]int32) { + t.Helper() + base, baseHits := phpmyadminMux(t, recordingDir) + body, err := os.ReadFile("../../testdata/parity/recordings/" + recordingDir + "/resolve-app.json") + if err != nil { + t.Fatalf("read resolve-app.json: %v", err) + } + var response map[string]any + if err := json.Unmarshal(body, &response); err != nil { + t.Fatalf("decode resolve-app.json: %v", err) + } + data := response["data"].(map[string]any) + apps := data["apps"].(map[string]any) + app := apps["edges"].([]any)[0].(map[string]any) + env := app["environments"].([]any)[0].(map[string]any) + switch residency { + case "true": + env["isK8sResident"] = true + case "false": + env["isK8sResident"] = false + case "null": + env["isK8sResident"] = nil + case "missing": + delete(env, "isK8sResident") + default: + t.Fatalf("unknown residency fixture %q", residency) + } + resolveBody, err := json.Marshal(response) + if err != nil { + t.Fatalf("encode resolve-app.json: %v", err) + } + + var resolves, missingSelection, malformedRequests int32 + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + atomic.AddInt32(&malformedRequests, 1) + http.Error(w, "read request", http.StatusBadRequest) + return + } + var request struct { + OperationName string `json:"operationName"` + Query string `json:"query"` + } + if err := json.Unmarshal(body, &request); err != nil { + atomic.AddInt32(&malformedRequests, 1) + http.Error(w, "decode request", http.StatusBadRequest) + return + } + switch request.OperationName { + case "App", "ResolveAppByName", "ResolveAppByID": + atomic.AddInt32(&resolves, 1) + if !strings.Contains(request.Query, "isK8sResident") { + atomic.AddInt32(&missingSelection, 1) + http.Error(w, "isK8sResident not selected", http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(resolveBody) + default: + r.Body = io.NopCloser(bytes.NewReader(body)) + base.ServeHTTP(w, r) + } + }) + hits := func() map[string]int32 { + enable, status, generate := baseHits() + return map[string]int32{ + "ResolveApp": atomic.LoadInt32(&resolves), + "MissingSelection": atomic.LoadInt32(&missingSelection), + "MalformedRequest": atomic.LoadInt32(&malformedRequests), + "Enable": enable, + "Status": status, + "Generate": generate, + } + } + return handler, hits +} + // TestPhpmyadminPrintParity exercises the happy path with --print: the // generated URL must land on stdout, exit code 0. func TestPhpmyadminPrintParity(t *testing.T) { @@ -177,48 +258,64 @@ func TestPhpmyadminSessionNoteDifferential(t *testing.T) { t.Skip(LoudSkip("TestPhpmyadminSessionNoteDifferential — the phpMyAdmin Node-vs-Go note", skip)) } - const note = "Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud." for _, tc := range []struct { - name string - wantNote bool + residency string + note string }{ - {name: "phpmyadmin-print", wantNote: true}, - {name: "phpmyadmin-silent", wantNote: false}, + {residency: "true", note: "Note: phpMyAdmin sessions are read-only. If you run a query that writes to DB, it will fail."}, + {residency: "false", note: ""}, + {residency: "null", note: "Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud."}, + {residency: "missing", note: "Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud."}, } { - t.Run(tc.name, func(t *testing.T) { - scenario, err := LoadScenario("../../testdata/parity/" + tc.name + ".yaml") - if err != nil { - t.Fatalf("LoadScenario: %v", err) - } - scenario.Env = rig.scenarioEnv(scenario) - - for _, side := range []struct { - name string - bin string - }{ - {name: "Node", bin: rig.nodeBin}, - {name: "Go", bin: rig.goBin}, - } { - res, _ := rig.runSide(t, scenario, side.bin, phpmyadminSurfaceMux) - if res.ExitCode != 0 { - t.Errorf("%s exit = %d; stdout=%q stderr=%q", side.name, res.ExitCode, res.Stdout, res.Stderr) + t.Run(tc.residency, func(t *testing.T) { + for _, silent := range []bool{false, true} { + name := "phpmyadmin-print" + if silent { + name = "phpmyadmin-silent" } - output := res.Stdout + res.Stderr - count := 0 - for _, line := range strings.Split(output, "\n") { - if strings.TrimSpace(line) == note { - count++ + t.Run(name, func(t *testing.T) { + scenario, err := LoadScenario("../../testdata/parity/" + name + ".yaml") + if err != nil { + t.Fatalf("LoadScenario: %v", err) } - } - if tc.wantNote && count != 1 { - t.Errorf("%s note count = %d, want 1; stdout=%q stderr=%q", side.name, count, res.Stdout, res.Stderr) - } - if !tc.wantNote && (count != 0 || strings.Contains(output, "Note:")) { - t.Errorf("%s --silent printed a note; stdout=%q stderr=%q", side.name, res.Stdout, res.Stderr) - } - if strings.Contains(output, "Note: PHPMyAdmin sessions are read-only.") { - t.Errorf("%s printed the obsolete blanket warning: %q", side.name, output) - } + scenario.Env = rig.scenarioEnv(scenario) + mux := func(t *testing.T, recordingDir string) (http.Handler, func() map[string]int32) { + return phpmyadminResidencyMux(t, recordingDir, tc.residency) + } + + for _, side := range []struct { + name string + bin string + }{ + {name: "Node", bin: rig.nodeBin}, + {name: "Go", bin: rig.goBin}, + } { + res, hits := rig.runSide(t, scenario, side.bin, mux) + if res.ExitCode != 0 { + t.Errorf("%s exit = %d; stdout=%q stderr=%q", side.name, res.ExitCode, res.Stdout, res.Stderr) + } + if hits["ResolveApp"] == 0 || hits["MissingSelection"] != 0 || hits["MalformedRequest"] != 0 { + t.Errorf("%s app resolution hits = %v, want a query selecting isK8sResident", side.name, hits) + } + output := res.Stdout + res.Stderr + noteCount, matchingCount := 0, 0 + for _, line := range strings.Split(output, "\n") { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "Note:") { + noteCount++ + } + if line == tc.note { + matchingCount++ + } + } + if !silent && tc.note != "" && (noteCount != 1 || matchingCount != 1) { + t.Errorf("%s note count = %d, exact matches = %d, want 1/1; stdout=%q stderr=%q", side.name, noteCount, matchingCount, res.Stdout, res.Stderr) + } + if (silent || tc.note == "") && noteCount != 0 { + t.Errorf("%s printed an unexpected note; stdout=%q stderr=%q", side.name, res.Stdout, res.Stderr) + } + } + }) } }) } diff --git a/src/bin/vip-db-phpmyadmin.ts b/src/bin/vip-db-phpmyadmin.ts index 7491b2fe4..3c1461b39 100755 --- a/src/bin/vip-db-phpmyadmin.ts +++ b/src/bin/vip-db-phpmyadmin.ts @@ -32,6 +32,7 @@ const appQuery = ` name type uniqueLabel + isK8sResident } `; diff --git a/src/commands/phpmyadmin.ts b/src/commands/phpmyadmin.ts index 889b27c89..5510f4670 100644 --- a/src/commands/phpmyadmin.ts +++ b/src/commands/phpmyadmin.ts @@ -230,9 +230,13 @@ export class PhpMyAdminCommand { exit.withError( 'No environment was specified' ); } - if ( ! this.silent ) { - const message = + if ( ! this.silent && this.env.isK8sResident !== false ) { + let message = 'Note: phpMyAdmin sessions are read-only on VIP Kubernetes and read-write on WP Cloud.'; + if ( this.env.isK8sResident === true ) { + message = + 'Note: phpMyAdmin sessions are read-only. If you run a query that writes to DB, it will fail.'; + } console.log( chalk.yellow( message ) ); }