Skip to content

Add "exceptions" GitOps config - #42013

Merged
sgress454 merged 12 commits into
mainfrom
worktree-gitops-exceptions
Mar 23, 2026
Merged

Add "exceptions" GitOps config#42013
sgress454 merged 12 commits into
mainfrom
worktree-gitops-exceptions

Conversation

@sgress454

@sgress454 sgress454 commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

Related issue: Resolves #42008

Details

Step one in #40171.

This PR adds a new exceptions subsection to the current GitOps config, with boolean keys for software, secrets and labels. For existing instances a migration is included to set labels and secrets to true. For new instances, only secrets will be true.

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/ or ee/fleetd-chrome/changes.
    See Changes files for more information.
    n/a, will put changelog in when more functionality is implemented.

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually
    (https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another)
    • ran migration and verified that app config had gitops.exceptions with software: false, secrets: true, labels: true
    • created a new instance and verified that that app config had gitops.exceptions with software: false, secrets: true, labels: false
    • verified that the PATCH /config API works and can update exceptions independently of other config

Database migrations

  • Checked schema for all modified table for columns that will auto-update timestamps during migration.
    n/a
  • Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
    n/a
  • Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).
    n/a

New Fleet configuration settings

  • Setting(s) is/are explicitly excluded from GitOps
    these will not be set in GitOps, since they're about how GitOps works.

Summary by CodeRabbit

Release Notes

  • New Features

    • GitOps configuration now supports exception settings for granular resource control. Administrators can configure which specific resource types (labels, software, and secrets) are included in or excluded from GitOps mode operations.
  • Improvements

    • Improved GitOps configuration handling to preserve exception settings during partial updates and system migrations.

@sgress454
sgress454 requested a review from a team as a code owner March 18, 2026 21:47
Copilot AI review requested due to automatic review settings March 18, 2026 21:47

Copilot AI 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.

Pull request overview

Adds a new gitops.exceptions configuration object (with labels, software, secrets booleans) to Fleet’s AppConfig, including defaults for new installs and a migration to backfill existing instances, plus integration/migration test coverage.

Changes:

  • Extend fleet.UIGitOpsModeConfig with a new Exceptions field and add new-install defaults.
  • Add MySQL migration to initialize exceptions for existing instances (labels/secrets enabled, software disabled).
  • Add integration test coverage for PATCH/GET semantics around GitOps exceptions and add a migration unit test.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
server/service/integration_enterprise_test.go Adds an end-to-end test validating PATCH merge behavior and GET response for gitops.exceptions.
server/fleet/app.go Introduces GitOpsExceptions and wires it into UIGitOpsModeConfig, plus new-install defaults.
server/datastore/mysql/migrations/tables/20260318210346_AddGitOpsExceptionsToAppConfig.go Migration to populate gitops.exceptions for existing instances.
server/datastore/mysql/migrations/tables/20260318210346_AddGitOpsExceptionsToAppConfig_test.go Unit test validating the migration sets the intended defaults on existing app config JSON.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread server/service/integration_enterprise_test.go Outdated
Comment thread server/fleet/app.go
@codecov

codecov Bot commented Mar 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.51%. Comparing base (351107c) to head (9096ca1).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...s/20260323144117_AddGitOpsExceptionsToAppConfig.go 81.81% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #42013      +/-   ##
==========================================
- Coverage   66.53%   66.51%   -0.03%     
==========================================
  Files        2519     2520       +1     
  Lines      202294   202330      +36     
  Branches     9049     9065      +16     
==========================================
- Hits       134590   134571      -19     
- Misses      55551    55606      +55     
  Partials    12153    12153              
Flag Coverage Δ
backend 68.30% <88.88%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@sgress454
sgress454 force-pushed the worktree-gitops-exceptions branch from ea61adf to 08c67dd Compare March 20, 2026 16:41
@sgress454
sgress454 marked this pull request as ready for review March 20, 2026 16:43

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

@sgress454

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 20, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR adds a new exceptions configuration to Fleet's GitOps app settings. The change introduces a GitOpsExceptions struct containing three boolean fields—labels, software, and secrets—and integrates it into the existing UIGitOpsModeConfig. A database migration sets initial exception values (labels and secrets enabled, software disabled) for existing instances. The API layer is updated to properly preserve exceptions during PATCH operations to the /config endpoint, and test fixtures and integration tests validate the complete workflow including partial updates and GET/PATCH behavior.

Possibly related PRs

  • #41303: Adds GitOps settings validation that directly uses the GitOpsExceptions structure and UIGitOpsModeConfig type introduced in this PR for parsing and validating GitOps organization settings.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding a new 'exceptions' subsection to the GitOps configuration.
Description check ✅ Passed The description is mostly complete with clear references to the linked issue #42008, implementation details about the migration defaults, and completed testing steps.
Linked Issues check ✅ Passed All coding requirements from issue #42008 are satisfied: GitOpsExceptions and UIGitOpsModeConfig structs added [#42008], GET/PATCH /config APIs support exceptions [#42008], and migrations set correct defaults [#42008].
Out of Scope Changes check ✅ Passed All code changes are within scope. Changes include the new config structures, migrations, tests, and API updates required by issue #42008 with no extraneous modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-gitops-exceptions

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 and usage tips.

Tip

CodeRabbit can enforce grammar and style rules using `languagetool`.

Configure the reviews.tools.languagetool setting to enable/disable rules and categories. Refer to the LanguageTool Community to learn more.

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/datastore/mysql/schema.sql`:
- Line 147: The schema snapshot contains an upgraded-instance default
(gitops.exceptions.labels=true) that conflicts with ApplyDefaultsForNewInstalls
in server/fleet/app.go which sets labels=false for new installs; update the
migration or default-generation code that produces the app_config_json insert
(the generator that emits server/datastore/mysql/schema.sql) so the generated
default JSON sets "gitops.exceptions.labels": false (or aligns with
ApplyDefaultsForNewInstalls), regenerate schema.sql, and ensure the migration
that seeds the app_config_json row (the migration function that builds the
default app config) is changed rather than hand-editing schema.sql.
🪄 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: 0030ce92-5c34-4673-8af2-12f32a9c6d44

📥 Commits

Reviewing files that changed from the base of the PR and between 6ed3ba6 and 08c67dd.

📒 Files selected for processing (17)
  • cmd/fleetctl/fleetctl/gitops_test.go
  • cmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigJson.json
  • cmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerJson.json
  • cmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerYaml.yml
  • cmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigYaml.yml
  • cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigJson.json
  • cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigYaml.yml
  • cmd/fleetctl/fleetctl/testdata/generateGitops/appConfig.json
  • cmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigEmpty.yml
  • cmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigSet.yml
  • server/datastore/mysql/migrations/tables/20260318210346_AddGitOpsExceptionsToAppConfig.go
  • server/datastore/mysql/migrations/tables/20260318210346_AddGitOpsExceptionsToAppConfig_test.go
  • server/datastore/mysql/schema.sql
  • server/fleet/app.go
  • server/service/client.go
  • server/service/integration_enterprise_test.go
  • tools/cloner-check/generated_files/appconfig.txt

Comment thread server/datastore/mysql/schema.sql
Comment thread server/fleet/app.go
Comment on lines +3343 to +3346
// Enable GitOps mode first
s.Do("PATCH", "/api/latest/fleet/config", json.RawMessage(`{
"gitops": { "gitops_mode_enabled": true, "repository_url": "https://example.com/repo" }
}`), http.StatusOK)

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.

We don't need this anymore right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nope

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

actually yeah but just to check that patching the exceptions doesn't blow away the gitops mode properties.

iansltx
iansltx previously approved these changes Mar 21, 2026
iansltx
iansltx previously approved these changes Mar 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add "exceptions" key to "gitops" app config

3 participants