PMM-15360: Add the OpenManager settings flag - #5866
Conversation
Adds Settings.OpenManager.Enabled to pmm-managed and an Advanced Settings toggle for it, default off like every other technical-preview flag (Azure Discover, Access Control). PMM_ENABLE_OM seeds it the same way PMM_ENABLE_NOMAD/PMM_ENABLE_AZURE_DISCOVER do, and reuses validateChangeSettingsRequest's existing env-var-wins precedence check rather than adding a new mechanism. This only exposes and stores the flag; nothing reads it yet to gate OpenManager's own behavior. That's the follow-up PR, once this and PMM-15326-om-backend / PMM-15326-om-ui-nav are all available to stack on. Signed-off-by: Pawel Lebioda <pawel.lebioda@percona.com>
68eea37 to
839e3dc
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## PMM-15299-open-manager #5866 +/- ##
==========================================================
- Coverage 46.98% 46.94% -0.05%
==========================================================
Files 430 430
Lines 45395 45409 +14
==========================================================
- Hits 21331 21318 -13
- Misses 22004 22030 +26
- Partials 2060 2061 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1de5337 to
be40598
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
WalkthroughThe change adds OpenManager configuration to the v1 API and server settings model. It supports updates through Merge Risk: 🟡 Moderate · up to The OpenManager setting is exposed through the API and UI, but the current update path does not persist the toggle, so users may see a successful change without the server state changing. The persistence mapping should be fixed before merging; the remaining accessibility and URL cleanup are non-blocking. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Warning Some tools did not complete. Review the errors below. 🔧 Buf (1.72.0)api/server/v1/server.protofatal: unable to access 'https://github.com/percona/pmm.git/': Failed to connect to github.com port 443 via 127.0.0.1 after 0 ms: Could not connect to server Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
ui/apps/pmm/src/pages/settings/Settings.messages.ts (1)
51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
openManagerLinkto the shared URL constants.This line hardcodes a URL in
Settings.messages.ts. Define the URL insrc/lib/constants.tsand reference that constant here.As per coding guidelines,
ui/apps/pmm/**/*.{js,jsx,ts,tsx}must not hardcode URLs; use constants fromsrc/lib/constants.ts.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: adfff729-cedd-4345-a7dd-1199eea48e13
⛔ Files ignored due to path filters (1)
api/server/v1/server.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (19)
api/server/v1/json/client/server_service/change_settings_responses.goapi/server/v1/json/client/server_service/get_read_only_settings_responses.goapi/server/v1/json/client/server_service/get_settings_responses.goapi/server/v1/json/v1.jsonapi/server/v1/server.pb.validate.goapi/server/v1/server.protoapi/swagger/swagger-dev.jsonapi/swagger/swagger.jsonmanaged/models/settings.gomanaged/models/settings_helpers.gomanaged/models/settings_helpers_test.gomanaged/services/server/server.gomanaged/utils/envvars/parser.goui/apps/pmm/src/pages/settings/Settings.messages.tsui/apps/pmm/src/pages/settings/components/advanced/AdvancedSettingsForm.schema.tsui/apps/pmm/src/pages/settings/components/advanced/AdvancedSettingsForm.tsxui/apps/pmm/src/pages/settings/components/advanced/AdvancedSettingsForm.utils.tsui/apps/pmm/src/types/settings.types.tsui/apps/pmm/src/utils/testUtils.tsx
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
percona/pmm-qa(manual)percona/pmm(manual)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.
…abled make gen does more than buf generate -- it also regenerates the swagger/OpenAPI JSON (api/server/v1/json/v1.json, the merged api/swagger/swagger*.json) and the generated JSON API client (api/server/v1/json/client/server_service/*.go) from the same proto. The earlier commit only ran buf generate directly and hand-curated its output against unrelated toolchain drift elsewhere in api/, which missed these -- they're make gen's job, not buf generate's. Also picks up the same protoc-gen-go import-grouping drift on server.pb.go/server.pb.validate.go that affects unrelated proto packages across the tree; taking make gen's own output here (rather my earlier hand-curated one) is what actually matches what CI's own gen step produces, so this and CI won't disagree again. The generated client's field names are OmEnabled/EnableOm here, matching this branch's own api/Makefile: --additional-initialism=om was added to the swagger client generator's flag list by PMM-15326-om-api, but that commit hasn't reached this branch's base (PMM-15299-open-manager) yet, so "om" isn't a registered initialism here and go-swagger capitalizes it as an ordinary word. Branches that already have that flag (PMM-15326-om-backend and its descendants) will correctly want OMEnabled/EnableOM instead -- each PR's generated code has to match its own base's Makefile, not a single "right" answer across the stack. Found because CI's "make gen; git diff --exit-code" check failed on this exact mismatch, on all three of this ticket's PRs. Signed-off-by: Pawel Lebioda <pawel.lebioda@percona.com>
be40598 to
04f695f
Compare
…witch-flag # Conflicts: # managed/utils/envvars/parser.go
2 participants
Recreated after #5851 was merged by mistake and reverted on
PMM-15299-open-manager(see #5851 for the original description and discussion).Same two commits as the original PR, unchanged: