-
Notifications
You must be signed in to change notification settings - Fork 478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clear bootstrap package and enrollment profile with GitOps #26095
Conversation
…th-in-no-team' into victor/25648-gitops-bootstrap # Conflicts: # cmd/fleetctl/gitops.go
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #26095 +/- ##
==========================================
- Coverage 63.64% 63.64% -0.01%
==========================================
Files 1631 1631
Lines 156363 156387 +24
Branches 4132 4104 -28
==========================================
+ Hits 99523 99533 +10
- Misses 49000 49010 +10
- Partials 7840 7844 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…ootstrap # Conflicts: # cmd/fleetctl/gitops.go # pkg/spec/gitops.go
) | ||
} | ||
controlsFilePath := yamlFilename | ||
err := processControlsPathIfNeeded(controlsTop, result, &controlsFilePath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactoring: extract method, except for resolveScriptPaths
@@ -248,7 +265,7 @@ func (c *Client) validateMacOSSetupAssistant(fileName string) ([]byte, error) { | |||
} | |||
|
|||
func (c *Client) uploadMacOSSetupAssistant(data []byte, teamID *uint, name string) error { | |||
verb, path := "POST", "/api/latest/fleet/mdm/apple/enrollment_profile" | |||
verb, path := http.MethodPost, "/api/latest/fleet/enrollment_profiles/automatic" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating to non-deprected path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. It might be helpful to future readers to expand on the comments a little more to clarify the assumptions and business rules going into each case.
return nil, nil, nil, nil, fmt.Errorf("applying fleet config: %w", err) | ||
} | ||
} | ||
case macosSetup.BootstrapPackage.Valid && !opts.DryRun && | ||
appconfig != nil && appconfig.MDM.EnabledAndConfigured && appconfig.License.IsPremium(): | ||
// bootstrap package is explicitly empty (only for GitOps) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just making sure that I'm following flows, are you saying that we know it is explicitly empty because the first case was not triggered? If so, what do you think about expanding the comment to something along the lines below?
// we know bootstrap package is explicitly empty because the first case was not triggered so we process according to the GitOps default rule, which is to delete any prior bootstrap package
Similarly, the other comments in the PR could be expanded. And for the non-GitOps apply
flows, the comments can spell out the business rule, which by contrast is to leave any prior value in place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gillespi314 How about this: https://github.com/fleetdm/fleet/pull/26184/files ?
We need to refactor ApplyGroup
into ApplyGroup
and ApplyGitOps
. apply
is effectively deprecated since we are not adding new features to it.
For #25648
Fixed issue where
fleetctl gitops
was NOT deleting macOS setup experience bootstrap package and enrollment profile. GitOps should clear all settings that are not explicitly set in YAML config files.Checklist for submitter
changes/
,orbit/changes/
oree/fleetd-chrome/changes
.See Changes files for more information.