Clean up Windows profile preprocess parameter passing in advance of adding additional profile variables (#35809)

Should be no functional changes, though in a few cases I was able to
pull some dep declarations outside of loops. Setting this as a separate
PR so refactors can be evaluated without worrying about functional
changes, and vice versa. Functional changes on #34364 will be stacked on
top of this.
This commit is contained in:
Ian Littman 2025-11-17 09:44:24 -06:00 committed by GitHub
parent 0b0c67a5d5
commit 04e7a308d4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 131 additions and 58 deletions

View file

@ -498,6 +498,15 @@ type GroupedCertificateAuthorities struct {
Smallstep []SmallstepSCEPProxyCA `json:"smallstep"`
}
// ToCustomSCEPProxyCAMap converts the CustomScepProxy slice to a map keyed by CA name
func (g *GroupedCertificateAuthorities) ToCustomSCEPProxyCAMap() map[string]*CustomSCEPProxyCA {
customSCEPCAs := make(map[string]*CustomSCEPProxyCA, len(g.CustomScepProxy))
for _, ca := range g.CustomScepProxy {
customSCEPCAs[ca.Name] = &ca
}
return customSCEPCAs
}
func GroupCertificateAuthoritiesByType(cas []*CertificateAuthority) (*GroupedCertificateAuthorities, error) {
grouped := &GroupedCertificateAuthorities{
DigiCert: []DigiCertCA{},

View file

@ -41,7 +41,10 @@ func LoopOverExpectedHostProfiles(
return fmt.Errorf("getting host profiles for verification: %w", err)
}
params := PreprocessingParameters{
deps := ProfilePreprocessDependenciesForVerify{
Context: ctx,
Logger: logger,
DataStore: ds,
HostIDForUUIDCache: make(map[string]uint),
}
@ -53,7 +56,10 @@ func LoopOverExpectedHostProfiles(
// Process Fleet variables if present (similar to how it's done during profile deployment)
// This ensures we compare what was actually sent to the device
processedContent := PreprocessWindowsProfileContentsForVerification(ctx, logger, ds, host.UUID, expectedProf.ProfileUUID, expanded, params)
processedContent := PreprocessWindowsProfileContentsForVerification(deps, ProfilePreprocessParams{
HostUUID: host.UUID,
ProfileUUID: expectedProf.ProfileUUID,
}, expanded)
expectedProf.RawProfile = []byte(processedContent)
var prof fleet.SyncMLCmd
@ -292,43 +298,25 @@ func IsWin32OrDesktopBridgeADMXCSP(locURI string) bool {
return false
}
// PreprocessingParameters holds parameters needed for preprocessing Windows profiles, for both verification and deployment only.
// It should only contain helper stuff, and not core values such as hostUUID, profileUUID, etc.
type PreprocessingParameters struct {
// a lookup map to avoid repeated datastore calls for hostID from hostUUID
HostIDForUUIDCache map[string]uint
}
// PreprocessWindowsProfileContentsForVerification processes Windows configuration profiles to replace Fleet variables
// with the given host UUID for verification purposes.
//
// This function is similar to PreprocessWindowsProfileContentsForDeployment, but it does not require
// a datastore or logger since it only replaces certain fleet variables to avoid datastore unnecessary work.
func PreprocessWindowsProfileContentsForVerification(ctx context.Context, logger kitlog.Logger, ds fleet.Datastore, hostUUID string, profileUUID string, profileContents string, params PreprocessingParameters) string {
replacedContents, _ := preprocessWindowsProfileContents(ctx, logger, ds, nil, true, hostUUID, profileUUID, nil, profileContents, nil, params)
// ^ We ignore the error here, and rely on the fact that the function will return the original contents if no replacements were made.
// So verification fails on individual profile level, instead of entire verification failing.
func PreprocessWindowsProfileContentsForVerification(deps ProfilePreprocessDependenciesForVerify, params ProfilePreprocessParams, profileContents string) string {
replacedContents, _ := preprocessWindowsProfileContents(deps, params, profileContents)
// ^ We ignore the error here and rely on the fact that the function will return the original contents if no replacements were made.
// So verification fails on the individual profile level, instead of the entire verification failing.
return replacedContents
}
// PreprocessWindowsProfileContentsForDeployment processes Windows configuration profiles to replace Fleet variables
// with their actual values for each host during profile deployment.
func PreprocessWindowsProfileContentsForDeployment(ctx context.Context, logger kitlog.Logger, ds fleet.Datastore,
appConfig *fleet.AppConfig, hostCmdUUID string, profileUUID string,
groupedCAs *fleet.GroupedCertificateAuthorities, profileContents string,
managedCertificatePayloads *[]*fleet.MDMManagedCertificate,
params PreprocessingParameters,
) (string, error) {
// TODO: Should we avoid iterating this list for every profile?
customSCEPCAs := make(map[string]*fleet.CustomSCEPProxyCA, len(groupedCAs.CustomScepProxy))
for _, ca := range groupedCAs.CustomScepProxy {
customSCEPCAs[ca.Name] = &ca
}
return preprocessWindowsProfileContents(ctx, logger, ds, appConfig, false, hostCmdUUID, profileUUID, customSCEPCAs, profileContents, managedCertificatePayloads, params)
func PreprocessWindowsProfileContentsForDeployment(deps ProfilePreprocessDependenciesForDeploy, params ProfilePreprocessParams, profileContents string) (string, error) {
return preprocessWindowsProfileContents(deps, params, profileContents)
}
// This error type is used to indicate errors during Microsoft profile processing, such as variable replacement failures.
// MicrosoftProfileProcessingError is used to indicate errors during Microsoft profile processing, such as variable replacement failures.
// It should not break the entire deployment flow, but rather be handled gracefully at the profile level, setting it to failed and detail = Error()
type MicrosoftProfileProcessingError struct {
message string
@ -338,6 +326,48 @@ func (e *MicrosoftProfileProcessingError) Error() string {
return e.message
}
type ProfilePreprocessDependencies interface {
GetContext() context.Context
GetLogger() kitlog.Logger
GetDS() fleet.Datastore
GetHostIdForUUIDCache() map[string]uint
}
type ProfilePreprocessDependenciesForVerify struct {
Context context.Context
Logger kitlog.Logger
DataStore fleet.Datastore
HostIDForUUIDCache map[string]uint
}
func (p ProfilePreprocessDependenciesForVerify) GetContext() context.Context {
return p.Context
}
func (p ProfilePreprocessDependenciesForVerify) GetLogger() kitlog.Logger {
return p.Logger
}
func (p ProfilePreprocessDependenciesForVerify) GetDS() fleet.Datastore {
return p.DataStore
}
func (p ProfilePreprocessDependenciesForVerify) GetHostIdForUUIDCache() map[string]uint {
return p.HostIDForUUIDCache
}
type ProfilePreprocessDependenciesForDeploy struct {
ProfilePreprocessDependenciesForVerify
AppConfig *fleet.AppConfig
CustomSCEPCAs map[string]*fleet.CustomSCEPProxyCA
ManagedCertificatePayloads *[]*fleet.MDMManagedCertificate
}
type ProfilePreprocessParams struct {
HostUUID string
ProfileUUID string
}
// preprocessWindowsProfileContents processes Windows configuration profiles to replace Fleet variables
// with their actual values for each host. This function is used both during profile deployment
// and during profile verification to ensure consistency.
@ -360,12 +390,11 @@ func (e *MicrosoftProfileProcessingError) Error() string {
// thousands of host profiles. Direct string replacement is more efficient for our use case.
// 4. XML escaping: We need XML-specific escaping for values, which is simpler to control with direct
// string replacement rather than template functions.
func preprocessWindowsProfileContents(ctx context.Context, logger kitlog.Logger, ds fleet.Datastore, appConfig *fleet.AppConfig,
isVerifying bool, hostUUID string, profileUUID string,
customSCEPCAs map[string]*fleet.CustomSCEPProxyCA, profileContents string,
managedCertificatePayloads *[]*fleet.MDMManagedCertificate,
params PreprocessingParameters,
) (string, error) {
//
// If you need another dependency that should be reused across profiles, add it to a ProfilePreprocessDependencies
// implementation and to the interface if it's required for both verification and deployment. For new dependencies that
// vary profile-to-profile, add them to ProfilePreprocessParams.
func preprocessWindowsProfileContents(deps ProfilePreprocessDependencies, params ProfilePreprocessParams, profileContents string) (string, error) {
// Check if Fleet variables are present
fleetVars := variables.Find(profileContents)
if len(fleetVars) == 0 {
@ -377,40 +406,42 @@ func preprocessWindowsProfileContents(ctx context.Context, logger kitlog.Logger,
result := profileContents
for _, fleetVar := range fleetVars {
if fleetVar == string(fleet.FleetVarHostUUID) {
result = profiles.ReplaceFleetVariableInXML(fleet.FleetVarHostUUIDRegexp, result, hostUUID)
result = profiles.ReplaceFleetVariableInXML(fleet.FleetVarHostUUIDRegexp, result, params.HostUUID)
} else if slices.Contains(fleet.IDPFleetVariables, fleet.FleetVarName(fleetVar)) {
replacedContents, replacedVariable, err := profiles.ReplaceHostEndUserIDPVariables(ctx, ds, fleetVar, result, hostUUID, params.HostIDForUUIDCache, func(errMsg string) error {
replacedContents, replacedVariable, err := profiles.ReplaceHostEndUserIDPVariables(deps.GetContext(), deps.GetDS(), fleetVar, result, params.HostUUID, deps.GetHostIdForUUIDCache(), func(errMsg string) error {
return &MicrosoftProfileProcessingError{message: errMsg}
})
if err != nil {
return profileContents, err
}
if !replacedVariable {
return profileContents, ctxerr.Wrap(ctx, err, "host end user IDP variable replacement failed for variable")
return profileContents, ctxerr.Wrap(deps.GetContext(), err, "host end user IDP variable replacement failed for variable")
}
result = replacedContents
}
// We skip some variables during verification, to avoid unnecessary datastore calls
// or processing that is not needed for verification.
if isVerifying {
// We skip some variables during verification to avoid unnecessary datastore calls
// or processing that is not needed for verification. We determine whether to process
// or verify based on which set of deps are passed in.
deps, forDeploy := deps.(ProfilePreprocessDependenciesForDeploy)
if !forDeploy {
continue
}
switch {
case fleetVar == string(fleet.FleetVarSCEPWindowsCertificateID):
result = profiles.ReplaceFleetVariableInXML(fleet.FleetVarSCEPWindowsCertificateIDRegexp, result, profileUUID)
result = profiles.ReplaceFleetVariableInXML(fleet.FleetVarSCEPWindowsCertificateIDRegexp, result, params.ProfileUUID)
case strings.HasPrefix(fleetVar, string(fleet.FleetVarCustomSCEPChallengePrefix)):
caName := strings.TrimPrefix(fleetVar, string(fleet.FleetVarCustomSCEPChallengePrefix))
err := profiles.IsCustomSCEPConfigured(ctx, customSCEPCAs, caName, fleetVar, func(errMsg string) error {
err := profiles.IsCustomSCEPConfigured(deps.Context, deps.CustomSCEPCAs, caName, fleetVar, func(errMsg string) error {
return &MicrosoftProfileProcessingError{message: errMsg}
})
if err != nil {
return profileContents, err
}
replacedContents, replacedVariable, err := profiles.ReplaceCustomSCEPChallengeVariable(ctx, logger, fleetVar, customSCEPCAs, result)
replacedContents, replacedVariable, err := profiles.ReplaceCustomSCEPChallengeVariable(deps.Context, deps.Logger, fleetVar, deps.CustomSCEPCAs, result)
if err != nil {
return profileContents, ctxerr.Wrap(ctx, err, "replacing custom SCEP challenge variable")
return profileContents, ctxerr.Wrap(deps.Context, err, "replacing custom SCEP challenge variable")
}
if !replacedVariable {
return profileContents, &MicrosoftProfileProcessingError{message: fmt.Sprintf("Custom SCEP challenge variable replacement failed for variable %s", fleetVar)}
@ -418,22 +449,22 @@ func preprocessWindowsProfileContents(ctx context.Context, logger kitlog.Logger,
result = replacedContents
case strings.HasPrefix(fleetVar, string(fleet.FleetVarCustomSCEPProxyURLPrefix)):
caName := strings.TrimPrefix(fleetVar, string(fleet.FleetVarCustomSCEPProxyURLPrefix))
err := profiles.IsCustomSCEPConfigured(ctx, customSCEPCAs, caName, fleetVar, func(errMsg string) error {
err := profiles.IsCustomSCEPConfigured(deps.Context, deps.CustomSCEPCAs, caName, fleetVar, func(errMsg string) error {
return &MicrosoftProfileProcessingError{message: errMsg}
})
if err != nil {
return profileContents, err
}
replacedContents, managedCertificate, replacedVariable, err := profiles.ReplaceCustomSCEPProxyURLVariable(ctx, logger, ds, appConfig, fleetVar, customSCEPCAs, result, hostUUID, profileUUID)
replacedContents, managedCertificate, replacedVariable, err := profiles.ReplaceCustomSCEPProxyURLVariable(deps.Context, deps.Logger, deps.DataStore, deps.AppConfig, fleetVar, deps.CustomSCEPCAs, result, params.HostUUID, params.ProfileUUID)
if err != nil {
return profileContents, ctxerr.Wrap(ctx, err, "replacing custom SCEP challenge variable")
return profileContents, ctxerr.Wrap(deps.Context, err, "replacing custom SCEP challenge variable")
}
if !replacedVariable {
return profileContents, &MicrosoftProfileProcessingError{message: fmt.Sprintf("Custom SCEP challenge variable replacement failed for variable %s", fleetVar)}
}
result = replacedContents
*managedCertificatePayloads = append(*managedCertificatePayloads, managedCertificate)
*deps.ManagedCertificatePayloads = append(*deps.ManagedCertificatePayloads, managedCertificate)
}
// Add other Fleet variables here as they are implemented, identify if it can be skipped for verification.

View file

@ -928,13 +928,19 @@ func TestPreprocessWindowsProfileContentsForVerification(t *testing.T) {
},
}
params := PreprocessingParameters{
deps := ProfilePreprocessDependenciesForVerify{
Context: t.Context(),
Logger: log.NewNopLogger(),
DataStore: ds,
HostIDForUUIDCache: make(map[string]uint),
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := PreprocessWindowsProfileContentsForVerification(t.Context(), log.NewNopLogger(), ds, tt.hostUUID, uuid.NewString(), tt.profileContents, params)
result := PreprocessWindowsProfileContentsForVerification(deps, ProfilePreprocessParams{
HostUUID: tt.hostUUID,
ProfileUUID: uuid.NewString(),
}, tt.profileContents)
require.Equal(t, tt.expectedContents, result)
})
}
@ -1143,9 +1149,7 @@ func TestPreprocessWindowsProfileContentsForDeployment(t *testing.T) {
},
}
params := PreprocessingParameters{
HostIDForUUIDCache: make(map[string]uint),
}
hostIDForUUIDCache := make(map[string]uint)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -1172,9 +1176,26 @@ func TestPreprocessWindowsProfileContentsForDeployment(t *testing.T) {
groupedCAs, err := ds.GetGroupedCertificateAuthorities(ctx, true)
require.NoError(t, err)
customSCEPCAs := groupedCAs.ToCustomSCEPProxyCAMap()
managedCertificates := &[]*fleet.MDMManagedCertificate{}
result, err := PreprocessWindowsProfileContentsForDeployment(ctx, log.NewNopLogger(), ds, appConfig, tt.hostUUID, profileUUID, groupedCAs, tt.profileContents, managedCertificates, params)
deps := ProfilePreprocessDependenciesForDeploy{
ProfilePreprocessDependenciesForVerify: ProfilePreprocessDependenciesForVerify{
Context: ctx,
Logger: log.NewNopLogger(),
DataStore: ds,
HostIDForUUIDCache: hostIDForUUIDCache,
},
AppConfig: appConfig,
CustomSCEPCAs: customSCEPCAs,
ManagedCertificatePayloads: managedCertificates,
}
result, err := PreprocessWindowsProfileContentsForDeployment(deps, ProfilePreprocessParams{
HostUUID: tt.hostUUID,
ProfileUUID: profileUUID,
}, tt.profileContents)
if tt.expectError {
require.Error(t, err)
if tt.processingError != "" {

View file

@ -135,6 +135,7 @@ func getHostEndUserIDPUser(ctx context.Context, ds fleet.Datastore,
return nil, false, onError(fmt.Sprintf("Unexpected number of hosts (%d) for UUID %s. ", len(ids), hostUUID))
}
hostID = ids[0]
// TODO figure out whether we should be passing a ref around here, as this looks like the cache doesn't persist
hostIDForUUIDCache[hostUUID] = hostID
}

View file

@ -2315,11 +2315,18 @@ func ReconcileWindowsProfiles(ctx context.Context, ds fleet.Datastore, logger ki
return ctxerr.Wrap(ctx, err, "getting grouped certificate authorities")
}
params := microsoft_mdm.PreprocessingParameters{
HostIDForUUIDCache: make(map[string]uint),
}
managedCertificatePayloads := &[]*fleet.MDMManagedCertificate{}
deps := microsoft_mdm.ProfilePreprocessDependenciesForDeploy{
ProfilePreprocessDependenciesForVerify: microsoft_mdm.ProfilePreprocessDependenciesForVerify{
Context: ctx,
Logger: logger,
DataStore: ds,
HostIDForUUIDCache: make(map[string]uint),
},
AppConfig: appConfig,
CustomSCEPCAs: groupedCAs.ToCustomSCEPProxyCAMap(),
ManagedCertificatePayloads: managedCertificatePayloads,
}
for profUUID, target := range installTargets {
p, ok := profileContents[profUUID]
@ -2350,7 +2357,11 @@ func ReconcileWindowsProfiles(ctx context.Context, ds fleet.Datastore, logger ki
}
// Preprocess the profile content for this specific host
processedContent, err := microsoft_mdm.PreprocessWindowsProfileContentsForDeployment(ctx, logger, ds, appConfig, hostUUID, profUUID, groupedCAs, string(p.SyncML), managedCertificatePayloads, params)
processedContent, err := microsoft_mdm.PreprocessWindowsProfileContentsForDeployment(
deps,
microsoft_mdm.ProfilePreprocessParams{HostUUID: hostUUID, ProfileUUID: profUUID},
string(p.SyncML),
)
var profileProcessingError *microsoft_mdm.MicrosoftProfileProcessingError
if err != nil && !errors.As(err, &profileProcessingError) {
return ctxerr.Wrapf(ctx, err, "preprocessing profile contents for host %s and profile %s", hostUUID, profUUID)