Skip to content

Commit

Permalink
fix: panic during migration (#21031)
Browse files Browse the repository at this point in the history
> Related issue: #21030

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality

---------

Co-authored-by: Sarah Gillespie <73313222+gillespi314@users.noreply.github.com>
  • Loading branch information
jahzielv and gillespi314 authored Aug 5, 2024
1 parent 20e9ebf commit 18977f3
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,16 @@ func Up_20240802113716(tx *sql.Tx) error {
if err := json.Unmarshal(r.Config, &config); err != nil {
return fmt.Errorf("unmarshal team config: %w", err)
}
softwareData := config["software"]
softwareData, ok := config["software"]
if !ok {
continue
}

rt := reflect.TypeOf(config["software"])
if rt == nil {
continue
}

if rt.Kind() == reflect.Slice {
// then we have an older config without the new fields
// Note: we are setting the new key to be whatever the old key was (if it was null, then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,78 @@ func TestUp_20240802113716(t *testing.T) {
"host_expiry_enabled": true
}
}
`

badCfgNoSoftwareField := `
{
"mdm": {
"ios_updates": {
"deadline": "",
"minimum_version": ""
},
"macos_setup": {
"bootstrap_package": "",
"macos_setup_assistant": "",
"enable_end_user_authentication": false,
"enable_release_device_manually": false
},
"macos_updates": {
"deadline": "",
"minimum_version": ""
},
"ipados_updates": {
"deadline": "",
"minimum_version": ""
},
"macos_settings": {
"custom_settings": []
},
"windows_updates": {
"deadline_days": null,
"grace_period_days": null
},
"windows_settings": {
"custom_settings": []
},
"enable_disk_encryption": false
},
"scripts": [],
"features": {
"enable_host_users": true,
"enable_software_inventory": true
},
"integrations": {
"jira": null,
"zendesk": null,
"google_calendar": {
"webhook_url": "",
"enable_calendar_events": false
}
},
"webhook_settings": {
"host_status_webhook": {
"days_count": 0,
"destination_url": "",
"host_percentage": 0,
"enable_host_status_webhook": false
},
"failing_policies_webhook": {
"policy_ids": null,
"destination_url": "",
"host_batch_size": 0,
"enable_failing_policies_webhook": false
}
},
"host_expiry_settings": {
"host_expiry_window": 30,
"host_expiry_enabled": true
}
}
`

tid1 := execNoErrLastID(t, db, `INSERT INTO teams (name, config) VALUES (?,?)`, "team 1", badCfg)
tid2 := execNoErrLastID(t, db, `INSERT INTO teams (name, config) VALUES (?,?)`, "team 2", badCfgEmptyArr)
tid3 := execNoErrLastID(t, db, `INSERT INTO teams (name, config) VALUES (?,?)`, "team 3", badCfgNoSoftwareField)

// Apply current migration.
applyNext(t, db)
Expand All @@ -180,6 +248,7 @@ func TestUp_20240802113716(t *testing.T) {
require.False(t, team.Config.Software.AppStoreApps.Valid)
require.Len(t, team.Config.Software.AppStoreApps.Value, 0)

team = fleet.Team{}
require.NoError(t, db.Get(&team, "SELECT id, config FROM teams WHERE id = ?", tid2))

// Team with an empty array originally should have JSON null set for packages
Expand All @@ -191,4 +260,9 @@ func TestUp_20240802113716(t *testing.T) {
require.False(t, team.Config.Software.AppStoreApps.Set)
require.False(t, team.Config.Software.AppStoreApps.Valid)
require.Len(t, team.Config.Software.AppStoreApps.Value, 0)

team = fleet.Team{}
require.NoError(t, db.Get(&team, "SELECT id, config FROM teams WHERE id = ?", tid3))

require.Nil(t, team.Config.Software)
}

0 comments on commit 18977f3

Please sign in to comment.