Skip to content

Adding the logic to make controls even when it's empty - #50836

Open
kitarp29 wants to merge 1 commit into
fleetdm:mainfrom
kitarp29:main
Open

Adding the logic to make controls even when it's empty#50836
kitarp29 wants to merge 1 commit into
fleetdm:mainfrom
kitarp29:main

Conversation

@kitarp29

@kitarp29 kitarp29 commented Aug 8, 2026

Copy link
Copy Markdown

Related issue: Resolves #50300, Resolves #50276

Description

This PR fixes two scoped issues with GitOps configuration generation (fleetctl generate-gitops):

  1. Fixes fleetctl generate-gitops emits org-level windows_enabled_and_configured / android_enabled_and_configured into per-fleet YAML files #50300: Moved the emission of windows_enabled_and_configured and android_enabled_and_configured inside the global-only guard block (teamId == nil || *teamId == 0) in generateControls. This prevents org-level MDM configurations from improperly leaking into per-fleet YAML files.
  2. Fixes fleetctl generate-gitops omits controls key, causing validation failure on apply #50276: Ensured the controls dictionary is always initialized and appended to the global GitOps output (even as an empty map {}). This prevents fleetctl gitops from failing validation with 'controls' must be set on global config when applying the generated files.

Checklist for submitter

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/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.

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually

For unreleased bug fixes in a release candidate, one of:

  • Confirmed that the fix is not expected to adversely impact load test results

New Fleet configuration settings

If you didn't check the box above, follow this checklist for GitOps-enabled settings:

  • Verified that the setting is exported via fleetctl generate-gitops

Summary by CodeRabbit

  • New Features

    • GitOps generation now includes controls for every processed fleet, including premium global fleets.
    • Generated controls are consistently written to the corresponding output file.
    • Windows and Android configuration settings are included only for applicable global or unassigned fleets.
  • Bug Fixes

    • Control-generation errors now stop the process instead of being skipped.
    • Empty GitOps values are cleaned up while preserving valid empty controls configurations.

Signed-off-by: Kitarp29 <kitarpsinghrajpoot@gmail.com>
@kitarp29
kitarp29 requested a review from a team as a code owner August 8, 2026 19:23
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The head commit changed during the review from 792bd6d to 53824d8.

Walkthrough

fleetctl generate-gitops now generates controls for every fleet and always writes the result. Shared YAML post-processing removes generic empty values while preserving controls: {}. Windows and Android configuration flags are emitted only for global or unassigned fleets. Tests and premium fixtures cover these behaviors.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address both linked issues by scoping Windows and Android controls globally and preserving an empty global controls map.
Out of Scope Changes check ✅ Passed The implementation, regression tests, and fixture updates directly support the two linked issue objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly describes the main change: generating the controls configuration even when it is empty.
Description check ✅ Passed The description explains both fixes, links related issues, and documents testing and GitOps verification, although the automated-tests checkbox is not checked.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/fleetctl/fleetctl/generate_gitops.go`:
- Around line 564-569: Update the serialization/rewrite logic following
generateControls so an empty controls map remains serialized as controls: {}
rather than being replaced with a blank value. Preserve the existing behavior
for non-empty controls, and add a regression case covering a team with no
generated controls.
- Around line 564-569: Update generateControls so the organization-level status
field assignments around the teamId checks are performed only when teamId is nil
or points to team 0. Preserve the existing control generation for regular team
fleets, but prevent global organization-level fields from being emitted for
non-global team outputs.

In `@cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.yml`:
- Line 258: Update the Android status fixture to match generateControls: remove
android_enabled_and_configured when AndroidEnabledAndConfigured is false, or
adjust the fixture and corresponding global/no-team controls to emit both status
fields explicitly only when the generator does so. Ensure the generated output
cannot include this key with a false value.
🪄 Autofix

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 Plus

Run ID: 2830faaf-5e54-4d18-a07e-4d2935868598

📥 Commits

Reviewing files that changed from the base of the PR and between 5b6a23d and 53824d8.

📒 Files selected for processing (2)
  • cmd/fleetctl/fleetctl/generate_gitops.go
  • cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.yml

Comment thread cmd/fleetctl/fleetctl/generate_gitops.go
@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.53%. Comparing base (5b6a23d) to head (53824d8).

Files with missing lines Patch % Lines
cmd/fleetctl/fleetctl/generate_gitops.go 40.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #50836      +/-   ##
==========================================
- Coverage   68.53%   68.53%   -0.01%     
==========================================
  Files        3977     3977              
  Lines      256142   256141       -1     
  Branches    13818    13818              
==========================================
- Hits       175553   175541      -12     
- Misses      64973    64983      +10     
- Partials    15616    15617       +1     
Flag Coverage Δ
backend 69.63% <40.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant