Skip to content

Commit

Permalink
Merge pull request #5058 from rancher-sandbox/5047-check-locked-versi…
Browse files Browse the repository at this point in the history
…on-field-change

Handle case where set/start proposed changed field kubernetes.version is locked.
  • Loading branch information
ericpromislow committed Jul 3, 2023
2 parents 015cd39 + f76c207 commit 3bed868
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 16 deletions.
31 changes: 25 additions & 6 deletions background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ function validateEarlySettings(cfg: settings.Settings, newSettings: RecursivePar
// We'd have to add more code to report that.
// It isn't worth adding that code yet. It might never be needed.
const newSettingsForValidation = _.omit(newSettings, 'kubernetes.version');
const [_needToUpdate, errors] = new SettingsValidator().validateSettings(cfg, newSettingsForValidation, lockedFields);
const [, errors] = new SettingsValidator().validateSettings(cfg, newSettingsForValidation, lockedFields);

if (errors.length > 0) {
throw new LockedFieldError(`Error in deployment profiles:\n${ errors.join('\n') }`);
Expand All @@ -952,20 +952,39 @@ function validateEarlySettings(cfg: settings.Settings, newSettings: RecursivePar
* The `requestShutdown` method is a special case that never returns.
*/
class BackgroundCommandWorker implements CommandWorkerInterface {
protected k8sVersions: string[] = [];
protected settingsValidator = new SettingsValidator();

/**
* Use the settings validator to validate settings after doing any
* initialization.
*/
protected async validateSettings(existingSettings: settings.Settings, newSettings: RecursivePartial<settings.Settings>) {
if (this.k8sVersions.length === 0) {
this.k8sVersions = (await k8smanager.kubeBackend.availableVersions).map(entry => entry.version.version);
this.settingsValidator.k8sVersions = this.k8sVersions;
let clearVersionsAfterTesting = false;

if (newSettings.kubernetes?.version && this.settingsValidator.k8sVersions.length === 0) {
// If we're starting up (by running `rdctl start...`) we probably haven't loaded all the k8s versions yet.
// We don't want to verify if the proposed version makes sense (if it doesn't, we'll assign the default version later).
// Here we just want to make sure that if we're changing the version to a different value from the current one,
// the field isn't locked.
let currentK8sVersions = (await k8smanager.kubeBackend.availableVersions).map(entry => entry.version.version);

if (currentK8sVersions.length === 0) {
clearVersionsAfterTesting = true;
currentK8sVersions = [newSettings.kubernetes.version];
if (existingSettings.kubernetes.version) {
currentK8sVersions.push(existingSettings.kubernetes.version);
}
}
this.settingsValidator.k8sVersions = currentK8sVersions;
}

const result = this.settingsValidator.validateSettings(existingSettings, newSettings, settings.getLockedSettings());

if (clearVersionsAfterTesting) {
this.settingsValidator.k8sVersions = [];
}

return this.settingsValidator.validateSettings(existingSettings, newSettings, settings.getLockedSettings());
return result;
}

getSettings() {
Expand Down
14 changes: 13 additions & 1 deletion e2e/lockedFields.e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ test.describe('Locked fields', () => {
}

test.describe.configure({ mode: 'serial' });
const lockedK8sVersion = '1.26.3';
const proposedK8sVersion = '1.26.1';

test.beforeAll(async() => {
await tool('rdctl', 'factory-reset', '--verbose');
Expand All @@ -85,7 +87,7 @@ test.describe('Locked fields', () => {
await saveUserProfile();
await createUserProfile(
{ containerEngine: { allowedImages: { enabled: true } } },
{ containerEngine: { allowedImages: { enabled: true, patterns: ['c', 'd', 'f'] } } },
{ containerEngine: { allowedImages: { enabled: true, patterns: ['c', 'd', 'f'] } }, kubernetes: { version: lockedK8sVersion } },
);
electronApp = await startRancherDesktop(__filename);
context = electronApp.context();
Expand Down Expand Up @@ -123,5 +125,15 @@ test.describe('Locked fields', () => {
stdout: '',
stderr: expect.stringContaining("field 'containerEngine.allowedImages.enabled' is locked"),
});
await expect(rdctl(['set', `--kubernetes.version=${ proposedK8sVersion }`]))
.resolves.toMatchObject({
stdout: '',
stderr: expect.stringContaining("field 'kubernetes.version' is locked"),
});
await expect(rdctl(['set', `--kubernetes.version=${ lockedK8sVersion }`]))
.resolves.toMatchObject({
stdout: expect.stringContaining('Status: no changes necessary.'),
stderr: '',
});
});
});
2 changes: 1 addition & 1 deletion pkg/rancher-desktop/backend/mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export default class MockBackend extends events.EventEmitter implements VMExecut
}

class MockKubernetesBackend extends events.EventEmitter implements KubernetesBackend {
readonly availableVersions = Promise.resolve([{ version: new semver.SemVer('0.0.0'), channels: ['latest'] }]);
readonly availableVersions = Promise.resolve([]);
version = '';
desiredPort = 9443;

Expand Down
25 changes: 17 additions & 8 deletions pkg/rancher-desktop/config/commandLineOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,22 @@ export function updateFromCommandLine(cfg: Settings, lockedFields: LockedSetting
}
newSettings = _.merge(newSettings, getObjectRepresentation(fqFieldName as RecursiveKeys<Settings>, finalValue));
}

// RD hasn't loaded the supported k8s versions yet, so have it defer actually checking the specified version.
// If it can't find this version, it will silently move to the closest version.
// We'd have to add more code to report that.
// It isn't worth adding that code yet. It might never be needed.
const newSettingsForValidation = _.omit(newSettings, 'kubernetes.version');
const [needToUpdate, errors] = (new SettingsValidator()).validateSettings(cfg, newSettingsForValidation, lockedFields);
const settingsValidator = new SettingsValidator();
const newKubernetesVersion = newSettings.kubernetes?.version;

if (newKubernetesVersion) {
// RD hasn't loaded the supported k8s versions yet, so fake the list.
// If the field is locked, we don't need to know what it's locked to,
// just that the proposed version is different from the current version.
// The current version doesn't have to be the locked version, but will be after processing ends.
const limitedK8sVersionList: Array<string> = [newKubernetesVersion];

if (cfg.kubernetes.version) {
limitedK8sVersionList.push(cfg.kubernetes.version);
}
settingsValidator.k8sVersions = limitedK8sVersionList;
}
const [needToUpdate, errors] = settingsValidator.validateSettings(cfg, newSettings, lockedFields);

if (errors.length > 0) {
const errorString = `Error in command-line options:\n${ errors.join('\n') }`;
Expand All @@ -131,7 +140,7 @@ export function updateFromCommandLine(cfg: Settings, lockedFields: LockedSetting
}
throw new Error(errorString);
}
if (needToUpdate || _.has(newSettings, 'kubernetes.version')) {
if (needToUpdate) {
cfg = _.merge(cfg, newSettings);
save(cfg);
} else {
Expand Down

0 comments on commit 3bed868

Please sign in to comment.