Remove UI gating in GitOps mode for excepted entities#42486
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #42486 +/- ##
========================================
Coverage 66.70% 66.70%
========================================
Files 2535 2536 +1
Lines 203449 203456 +7
Branches 9231 9117 -114
========================================
+ Hits 135716 135722 +6
- Misses 55456 55457 +1
Partials 12277 12277
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sgress454
force-pushed
the
sgress454/gitops-exceptions-pr4-ui-gating
branch
from
5da9174 to
014aca7
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR introduces GitOps mode exceptions for three entity types: labels, software, and secrets. When GitOps mode is globally enabled, individual entity types can now be exempted via configuration flags. The implementation adds an optional Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/pages/labels/ManageLabelsPage/ManageLabelsPage.tsx (1)
104-120:⚠️ Potential issue | 🟡 MinorMissing dependencies in
useCallback.The
renderTablecallback referenceslabelsGitOpsManagedandconfig?.gitops.repository_urlbut these aren't included in the dependency array. If these values change (e.g., config update), the table won't re-render with updated GitOps state.Proposed fix
- }, [currentUser, error, isLoading, labels, onClickAction]); + }, [currentUser, error, isLoading, labels, onClickAction, labelsGitOpsManaged, config?.gitops.repository_url]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/labels/ManageLabelsPage/ManageLabelsPage.tsx` around lines 104 - 120, The renderTable useCallback is missing dependencies for labelsGitOpsManaged and the repo URL from config, so update the dependency array of renderTable (the useCallback that returns <LabelsTable ... />) to include labelsGitOpsManaged and the config value used (e.g., config or config?.gitops.repository_url) so the callback re-runs when those change; keep existing dependencies currentUser, error, isLoading, labels, onClickAction and add the new ones.frontend/pages/labels/ManageLabelsPage/LabelsTable/LabelsTableConfig.tsx (1)
96-109:⚠️ Potential issue | 🟠 MajorGate the "Edit" option for GitOps-managed labels to match "Delete" behavior.
Currently, "Edit" remains enabled (
disabled: falseat line 100) while "Delete" is gated withdisabled: labelsGitOpsManaged(line 107). The edit page also lacks GitOps validation. Either gate "Edit" in the dropdown at line 97-102, or confirm backend API prevents updates on GitOps-managed labels. The current state creates an inconsistency where users can navigate to edit a managed label even though deletion is blocked.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/labels/ManageLabelsPage/LabelsTable/LabelsTableConfig.tsx` around lines 96 - 109, The "Edit" option is inconsistently enabled for GitOps-managed labels; in the block that builds options (inside the hasEditPermission(currentUser, label) branch) change the Edit option to respect the labelsGitOpsManaged flag like Delete does — set disabled: labelsGitOpsManaged (instead of false) and add tooltipContent: gitOpsTooltip so the dropdown prevents edits and shows the same GitOps tooltip; keep the existing guard that skips host_vitals (label.label_membership_type !== "host_vitals").
🧹 Nitpick comments (2)
frontend/hooks/useSoftwareInstallerMeta.ts (1)
128-134: Consider consistency in accessing gitops config properties.Lines 128-131 destructure from
config?.gitops, while line 133 accessesconfig?.gitops.exceptions?.softwaredirectly. This works correctly due to optional chaining, but for consistency you could destructureexceptionsas well.♻️ Optional refactor for consistency
const { gitops_mode_enabled: configGitOpsModeEnabled, repository_url: repoURL, + exceptions, } = config?.gitops || {}; -const softwareExcepted = !!config?.gitops.exceptions?.software; +const softwareExcepted = !!exceptions?.software; const gitOpsModeEnabled = !!configGitOpsModeEnabled && !softwareExcepted;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/hooks/useSoftwareInstallerMeta.ts` around lines 128 - 134, Destructure the exceptions property from config?.gitops instead of accessing config?.gitops.exceptions?.software inline: add exceptions (or exceptions?.software) to the existing destructure alongside gitops_mode_enabled and repository_url, compute softwareExcepted from that local variable, and keep the existing gitOpsModeEnabled logic (use the destructured exceptions to derive softwareExcepted and then gitOpsModeEnabled) so all gitops properties are accessed consistently; reference the existing identifiers gitops_mode_enabled, repository_url, exceptions, softwareExcepted, and gitOpsModeEnabled when making the change.frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/EditSoftwareModal.tsx (1)
56-56: Remove unusedgitOpsModeEnabledprop from interface and component signature.The prop is defined in
IEditSoftwareModalPropsbut never destructured in the component function. The component computesgitOpsModeEnabledlocally from the app context (lines 82-84), and the parent component (SoftwareSummaryCard) does not pass this prop when rendering the component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/EditSoftwareModal.tsx` at line 56, Remove the unused gitOpsModeEnabled prop: delete gitOpsModeEnabled from the IEditSoftwareModalProps interface and from the EditSoftwareModal component parameter list (the function signature), and then run TypeScript checks to remove any resulting unused-variable warnings or imports; ensure the component continues to compute gitOpsModeEnabled locally (via app context in the body of EditSoftwareModal) and that no other code expects this prop (e.g., SoftwareSummaryCard) so type signatures remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/pages/labels/ManageLabelsPage/ManageLabelsPage.tsx`:
- Around line 41-42: The expression computing labelsGitOpsManaged can throw if
config.gitops.exceptions is undefined; update the check in the
labelsGitOpsManaged assignment to safely access exceptions (use optional
chaining on exceptions) so it evaluates like checking
config?.gitops?.exceptions?.labels, ensuring you reference the existing
labelsGitOpsManaged constant and the config.gitops.exceptions path when making
the change.
---
Outside diff comments:
In `@frontend/pages/labels/ManageLabelsPage/LabelsTable/LabelsTableConfig.tsx`:
- Around line 96-109: The "Edit" option is inconsistently enabled for
GitOps-managed labels; in the block that builds options (inside the
hasEditPermission(currentUser, label) branch) change the Edit option to respect
the labelsGitOpsManaged flag like Delete does — set disabled:
labelsGitOpsManaged (instead of false) and add tooltipContent: gitOpsTooltip so
the dropdown prevents edits and shows the same GitOps tooltip; keep the existing
guard that skips host_vitals (label.label_membership_type !== "host_vitals").
In `@frontend/pages/labels/ManageLabelsPage/ManageLabelsPage.tsx`:
- Around line 104-120: The renderTable useCallback is missing dependencies for
labelsGitOpsManaged and the repo URL from config, so update the dependency array
of renderTable (the useCallback that returns <LabelsTable ... />) to include
labelsGitOpsManaged and the config value used (e.g., config or
config?.gitops.repository_url) so the callback re-runs when those change; keep
existing dependencies currentUser, error, isLoading, labels, onClickAction and
add the new ones.
---
Nitpick comments:
In `@frontend/hooks/useSoftwareInstallerMeta.ts`:
- Around line 128-134: Destructure the exceptions property from config?.gitops
instead of accessing config?.gitops.exceptions?.software inline: add exceptions
(or exceptions?.software) to the existing destructure alongside
gitops_mode_enabled and repository_url, compute softwareExcepted from that local
variable, and keep the existing gitOpsModeEnabled logic (use the destructured
exceptions to derive softwareExcepted and then gitOpsModeEnabled) so all gitops
properties are accessed consistently; reference the existing identifiers
gitops_mode_enabled, repository_url, exceptions, softwareExcepted, and
gitOpsModeEnabled when making the change.
In
`@frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/EditSoftwareModal.tsx`:
- Line 56: Remove the unused gitOpsModeEnabled prop: delete gitOpsModeEnabled
from the IEditSoftwareModalProps interface and from the EditSoftwareModal
component parameter list (the function signature), and then run TypeScript
checks to remove any resulting unused-variable warnings or imports; ensure the
component continues to compute gitOpsModeEnabled locally (via app context in the
body of EditSoftwareModal) and that no other code expects this prop (e.g.,
SoftwareSummaryCard) so type signatures remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 49a4914f-83ef-4300-b8ea-c108e163a429
📒 Files selected for processing (26)
frontend/__mocks__/configMock.tsfrontend/components/EnrollSecrets/EnrollSecretModal/EnrollSecretModal.tsxfrontend/components/EnrollSecrets/EnrollSecretTable/EnrollSecretRow/EnrollSecretRow.tsxfrontend/components/GitOpsModeTooltipWrapper/GitOpsModeTooltipWrapper.tests.tsxfrontend/components/GitOpsModeTooltipWrapper/GitOpsModeTooltipWrapper.tsxfrontend/hooks/useSoftwareInstallerMeta.tsfrontend/interfaces/config.tsfrontend/pages/ManageControlsPage/Secrets/Secrets.tsxfrontend/pages/SoftwarePage/SoftwareAddPage/SoftwareCustomPackage/SoftwareCustomPackage.tsxfrontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/FleetMaintainedAppDetailsPage/FleetAppDetailsForm/FleetAppDetailsForm.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/AddPatchPolicyModal/AddPatchPolicyModal.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/EditSoftwareModal.tsxfrontend/pages/SoftwarePage/components/cards/SoftwareDetailsSummary/SoftwareDetailsSummary.tsxfrontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tsxfrontend/pages/SoftwarePage/components/forms/SoftwareAndroidForm/SoftwareAndroidForm.tsxfrontend/pages/SoftwarePage/components/forms/SoftwareVppForm/SoftwareVppForm.tsxfrontend/pages/SoftwarePage/components/modals/ManageSoftwareAutomationsModal/ManageSoftwareAutomationsModal.tsxfrontend/pages/admin/IntegrationsPage/cards/MdmSettings/components/EndUserMigrationSection/EndUserMigrationSection.tests.tsxfrontend/pages/admin/OrgSettingsPage/cards/FleetDesktop/FleetDesktop.tests.tsxfrontend/pages/hosts/ManageHostsPage/components/HostsFilterBlock/HostsFilterBlock.tsxfrontend/pages/labels/ManageLabelsPage/LabelsTable/LabelsTable.tsxfrontend/pages/labels/ManageLabelsPage/LabelsTable/LabelsTableConfig.tsxfrontend/pages/labels/ManageLabelsPage/ManageLabelsPage.tsxfrontend/pages/labels/NewLabelPage/NewLabelPage.tsxfrontend/pages/labels/components/LabelForm/LabelForm.tsx
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
sgress454
force-pushed
the
sgress454/gitops-exceptions-pr4-ui-gating
branch
from
014aca7 to
78e7e8f
Compare
There was a problem hiding this comment.
Pull request overview
Updates the frontend GitOps-mode UI gating so labels, software, and secrets controls remain interactive when their corresponding GitOps exception is enabled, primarily by extending GitOpsModeTooltipWrapper to understand per-entity exceptions.
Changes:
- Added
gitops.exceptionstyping to config and introducedGitOpsEntityTypefor exception-aware gating. - Extended
GitOpsModeTooltipWrapperwith anentityTypeprop to bypass disabling when that entity is excepted. - Applied exception-aware gating across labels/software/secrets UI controls and updated related tests/mocks.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/pages/labels/components/LabelForm/LabelForm.tsx | Wraps “Save” with exception-aware GitOps tooltip wrapper for labels. |
| frontend/pages/labels/NewLabelPage/NewLabelPage.tsx | Wraps “Save” with exception-aware GitOps tooltip wrapper for labels. |
| frontend/pages/labels/ManageLabelsPage/ManageLabelsPage.tsx | Computes labels GitOps-managed state and passes it (and repo URL) down to the labels table. |
| frontend/pages/labels/ManageLabelsPage/LabelsTable/LabelsTableConfig.tsx | Disables label deletion in actions dropdown when labels are GitOps-managed and adds tooltip content. |
| frontend/pages/labels/ManageLabelsPage/LabelsTable/LabelsTable.tsx | Plumbs GitOps-managed state + repo URL into table header generation. |
| frontend/pages/hosts/ManageHostsPage/components/HostsFilterBlock/HostsFilterBlock.tsx | Wraps label edit/delete icon buttons with exception-aware GitOps tooltip wrapper. |
| frontend/pages/admin/OrgSettingsPage/cards/FleetDesktop/FleetDesktop.tests.tsx | Updates config mock usage to include gitops.exceptions. |
| frontend/pages/admin/IntegrationsPage/cards/MdmSettings/components/EndUserMigrationSection/EndUserMigrationSection.tests.tsx | Updates config mock usage to include gitops.exceptions. |
| frontend/pages/SoftwarePage/components/modals/ManageSoftwareAutomationsModal/ManageSoftwareAutomationsModal.tsx | Treats GitOps mode as disabled for software when software is excepted; passes entityType="software" to wrapper. |
| frontend/pages/SoftwarePage/components/forms/SoftwareVppForm/SoftwareVppForm.tsx | Treats GitOps mode as disabled for software when software is excepted; passes entityType="software" to wrapper. |
| frontend/pages/SoftwarePage/components/forms/SoftwareAndroidForm/SoftwareAndroidForm.tsx | Treats GitOps mode as disabled for software when software is excepted; passes entityType="software" to wrapper. |
| frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tsx | Treats GitOps mode as disabled for software when software is excepted. |
| frontend/pages/SoftwarePage/components/cards/SoftwareDetailsSummary/SoftwareDetailsSummary.tsx | Treats GitOps mode as disabled for software when software is excepted. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/EditSoftwareModal.tsx | Treats GitOps mode as disabled for software when software is excepted. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditAutoUpdateConfigModal/EditAutoUpdateConfigModal.tsx | Treats GitOps mode as disabled for software when software is excepted; passes entityType="software" to wrapper. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/AddPatchPolicyModal/AddPatchPolicyModal.tsx | Passes entityType="software" to wrapper. |
| frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareFleetMaintained/FleetMaintainedAppDetailsPage/FleetAppDetailsForm/FleetAppDetailsForm.tsx | Passes entityType="software" to wrapper. |
| frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareCustomPackage/SoftwareCustomPackage.tsx | Treats GitOps mode as disabled for software when software is excepted. |
| frontend/pages/ManageControlsPage/Secrets/Secrets.tsx | Passes entityType="secrets" to wrapper in secrets UI. |
| frontend/interfaces/config.ts | Adds IGitOpsExceptions, GitOpsEntityType, and gitops.exceptions to config typings. |
| frontend/hooks/useSoftwareInstallerMeta.ts | Treats GitOps mode as disabled for software when software is excepted. |
| frontend/components/GitOpsModeTooltipWrapper/GitOpsModeTooltipWrapper.tsx | Adds entityType and exception-aware bypass behavior. |
| frontend/components/GitOpsModeTooltipWrapper/GitOpsModeTooltipWrapper.tests.tsx | Adds tests for exception-aware behavior. |
| frontend/components/EnrollSecrets/EnrollSecretTable/EnrollSecretRow/EnrollSecretRow.tsx | Passes entityType="secrets" to wrapper for enrollment secret row actions. |
| frontend/components/EnrollSecrets/EnrollSecretModal/EnrollSecretModal.tsx | Passes entityType="secrets" to wrapper in enrollment secret modal actions. |
| frontend/mocks/configMock.ts | Adds default gitops.exceptions to mock config. |
Comments suppressed due to low confidence (1)
frontend/pages/labels/ManageLabelsPage/LabelsTable/LabelsTableConfig.tsx:103
- In GitOps-managed mode (
labelsGitOpsManaged), the dropdown still offers an enabled "Edit" action. That allows navigating into the edit UI even though edits can’t be saved, which conflicts with the intended UI gating behavior. Disable the "Edit" option (and attach the same GitOps tooltip content) whenlabelsGitOpsManagedis true.
if (hasEditPermission(currentUser, label)) {
if (label.label_membership_type !== "host_vitals") {
options.push({
label: "Edit",
disabled: false,
value: "edit",
});
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
iansltx
left a comment
iansltx
left a comment
There was a problem hiding this comment.
Feels like the GitOps mode duplication is now a lot heavier on software UIs that have that check explicitly rather than as the component. Maybe that should be extracted?
| const gitOpsModeEnabled = config?.gitops.gitops_mode_enabled || false; | ||
| const softwareExcepted = !!config?.gitops.exceptions?.software; | ||
| const gitOpsModeEnabled = | ||
| (config?.gitops.gitops_mode_enabled && !softwareExcepted) || false; |
There was a problem hiding this comment.
I know this is a holdover from previously, but do we actually need the || false here?
sgress454
force-pushed
the
sgress454/gitops-exceptions-pr4-ui-gating
branch
from
78e7e8f to
0f1de82
Compare
Good call. Made a |
iansltx
left a comment
iansltx
left a comment
There was a problem hiding this comment.
Yep, that's a lot cleaner, thanks!
Related issue: Resolves #42184
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes
Testing
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
For unreleased bug fixes in a release candidate, one of:
Database migrations
COLLATE utf8mb4_unicode_ci).New Fleet configuration settings
If you didn't check the box above, follow this checklist for GitOps-enabled settings:
fleetctl generate-gitopsfleetd/orbit/Fleet Desktop
runtime.GOOSis used as needed to isolate changesSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests