RSDK-14181: Make managed-security the default for os_auto_upgrade_type - #267
Conversation
Josh Matthews (jmatth)
changed the title
| "logging_journald_runtime_max_use_megabytes": 512, | ||
| "logging_journald_storage": "persistent", | ||
| "os_auto_upgrade_type": "", | ||
| "os_auto_upgrade_type": "managed-security", |
There was a problem hiding this comment.
Would it be better to leave this as "" and check if it defaults to managed?
There was a problem hiding this comment.
Probably, but it would need to go through validateConfig where that's actually enforced. I'm actually not sure why this test doesn't already do that. I'll try adding it and see if anything breaks
There was a problem hiding this comment.
Ok, doing that breaks the test and requires other changes to make it work. I think the way this test is written is just generally really confusing. According to the comment it's just testing that the JSON unmarshaling works, but then it's using a modified copy of the default config to compare against? I'm guessing the default config is used to avoid all the code it would take to construct the expected state manually. I think this should be left as-is since it's just for the JSON behavior, and I'll add a separate test to make sure tha validate method overwrites an empty string with the default
| if *v < minTimeout { | ||
| //nolint:lll | ||
| *v = DefaultConfiguration.NetworkConfiguration.OfflineBeforeStartingHotspotMinutes | ||
| errOut = errors.Join(errOut, errors.New(k+": timeout value cannot be less than 1 minute")) |
There was a problem hiding this comment.
perhaps add "set to default x" here
|
Hey Josh Matthews (@jmatth) — this PR has been approved and CI has been green for 3+ business days. Ready to merge? Auto-comment from overwatch. Will not re-nudge for 7 days. |
3 participants
What
Changes the default value for
os_manageds_upgrade_typetomanaged-security. This replaces the previous default behavior, which was to do nothing.Why
We are migrating agent managed machines to use
managed-securityby default. Updating the way agent treats an empty value in the config is consistent with how other default config values are handled. This PR will need to wait to be merged + deployed until we have finished informing users of the upcoming change and allowing them time to opt out.