Skip to content

Commit

Permalink
is_pinned_on__release: Add translations and only read the new column
Browse files Browse the repository at this point in the history
Change-type: minor
  • Loading branch information
Andrea Rosci authored and Andrea Rosci committed Jul 2, 2024
1 parent 3881a43 commit 310e1a4
Show file tree
Hide file tree
Showing 17 changed files with 73 additions and 45 deletions.
2 changes: 1 addition & 1 deletion src/balena.sbvr
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ Rule: It is necessary that each application that owns a release1 that has a stat
Rule: It is necessary that each image that has a status that is equal to "success", has a push timestamp.
Rule: It is necessary that each application that owns a release1 that has a status that is equal to "success" and has a commit1, owns at most one release2 that has a status that is equal to "success" and has a commit2 that is equal to the commit1.
Rule: It is necessary that each application that owns a release1 that has a revision, owns at most one release2 that has a semver major that is of the release1 and has a semver minor that is of the release1 and has a semver patch that is of the release1 and has a semver prerelease that is of the release1 and has a variant that is of the release1 and has a revision that is of the release1.
Rule: It is necessary that each release that should be running on a device, has a status that is equal to "success" and belongs to an application1 that the device belongs to.
Rule: It is necessary that each release that is pinned to a device, has a status that is equal to "success" and belongs to an application1 that the device belongs to.
Rule: It is necessary that each release that should be running on an application, has a status that is equal to "success" and belongs to the application.
Rule: It is necessary that each application that owns a release that contains at least 2 images, has an application type that supports multicontainer.
Rule: It is necessary that each release that should operate a device, has a status that is equal to "success".
Expand Down
2 changes: 1 addition & 1 deletion src/features/ci-cd/hooks/app-update-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ hooks.addPureHook('PATCH', 'resin', 'application', {
is_running__release: {
$ne: request.values.should_be_running__release,
},
should_be_running__release: null,
is_pinned_on__release: null,
},
wait: false,
});
Expand Down
2 changes: 1 addition & 1 deletion src/features/ci-cd/hooks/device-update-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ hooks.addPureHook('PATCH', 'resin', 'device', {
// * target release changed
// * device name changed - so a user can restart their service and it will pick up the change
if (
(request.values.should_be_running__release !== undefined ||
(request.values.is_pinned_on__release !== undefined ||
request.values.belongs_to__application != null ||
request.values.device_name != null) &&
affectedIds.length !== 0
Expand Down
8 changes: 4 additions & 4 deletions src/features/ci-cd/hooks/service-installs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ hooks.addPureHook('PATCH', 'resin', 'application', {
api,
{
belongs_to__application: { $in: affectedIds },
should_be_running__release: null,
is_pinned_on__release: null,
},
{
id: request.values.should_be_running__release,
Expand Down Expand Up @@ -228,14 +228,14 @@ hooks.addPureHook('PATCH', 'resin', 'device', {
POSTRUN: async ({ api, request }) => {
const affectedIds = request.affectedIds!;
if (
request.values.should_be_running__release !== undefined &&
request.values.is_pinned_on__release !== undefined &&
affectedIds.length !== 0
) {
// If the device was preloaded, and then pinned, service_installs do not exist
// for this device+release combination. We need to create these
if (request.values.should_be_running__release != null) {
if (request.values.is_pinned_on__release != null) {
await createReleaseServiceInstalls(api, affectedIds, {
id: request.values.should_be_running__release,
id: request.values.is_pinned_on__release,
});
} else {
const devices = (await api.get({
Expand Down
2 changes: 1 addition & 1 deletion src/features/device-state/routes/state-get-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ const stateQuery = _.once(() =>
device_environment_variable: {
$select: ['name', 'value'],
},
should_be_running__release: releaseExpand,
is_pinned_on__release: releaseExpand,
service_install: {
$select: ['id'],
$expand: {
Expand Down
8 changes: 4 additions & 4 deletions src/features/device-state/routes/state-get-v3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ const deviceExpand = {
device_environment_variable: {
$select: ['name', 'value'],
},
should_be_running__release: releaseExpand,
is_pinned_on__release: releaseExpand,
service_install: {
$select: ['id'],
$expand: {
Expand Down Expand Up @@ -319,7 +319,7 @@ const getStateV3 = async (req: Request, uuid: string): Promise<StateV3> => {
'io.balena.image.store': 'root',
},
),
...getAppState(device, 'should_be_running__release', config),
...getAppState(device, 'is_pinned_on__release', config),
};

const state: StateV3 = {
Expand Down Expand Up @@ -364,15 +364,15 @@ const getDevice = getStateDelayingEmpty(
const getAppState = (
device: AnyObject,
targetReleaseField:
| 'should_be_running__release'
| 'is_pinned_on__release'
| 'should_be_managed_by__release'
| 'should_be_operated_by__release',
config: Dictionary<string>,
defaultLabels?: Dictionary<string>,
): StateV3[string]['apps'] | null => {
let application: AnyObject;
let release: AnyObject | undefined;
if (targetReleaseField === 'should_be_running__release') {
if (targetReleaseField === 'is_pinned_on__release') {
application = device.belongs_to__application[0];
release = getReleaseForDevice(device);
} else {
Expand Down
12 changes: 7 additions & 5 deletions src/features/device-state/routes/state-patch-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from '../../../infra/error-handling/index.js';
import { sbvrUtils, errors } from '@balena/pinejs';
import { getIP } from '../../../lib/utils.js';
import type { Device } from '../../../balena-model.js';
import {
shouldUpdateMetrics,
metricsPatchFields,
Expand All @@ -17,7 +18,6 @@ import {
limitMetricNumbers,
} from '../state-patch-utils.js';
import type { ResolveDeviceInfoCustomObject } from '../middleware.js';
import type { Device } from '../../../balena-model.js';

const { BadRequestError, UnauthorizedError } = errors;
const { api } = sbvrUtils;
Expand Down Expand Up @@ -141,10 +141,9 @@ export const statePatchV2: RequestHandler = async (req, res) => {
const { apps } = local;

let deviceBody: Pick<LocalBody, (typeof v2ValidPatchFields)[number]> &
Partial<Pick<Device['Write'], 'is_running__release'>> = _.pick(
local,
v2ValidPatchFields,
);
Partial<
Pick<Device['Write'], 'is_running__release' | 'is_pinned_on__release'>
> = _.pick(local, v2ValidPatchFields);
let metricsBody: Pick<LocalBody, (typeof metricsPatchFields)[number]> =
_.pick(local, metricsPatchFields);
limitMetricNumbers(metricsBody);
Expand All @@ -158,6 +157,9 @@ export const statePatchV2: RequestHandler = async (req, res) => {
metricsBody = {};
}

if (local.should_be_running__release !== undefined) {
deviceBody.is_pinned_on__release = local.should_be_running__release;
}
if (local.name != null) {
deviceBody.device_name = local.name;
}
Expand Down
4 changes: 2 additions & 2 deletions src/features/device-state/state-get-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ export const getConfig = (
export const getReleaseForDevice = (
device: AnyObject,
): AnyObject | undefined => {
if (device.should_be_running__release[0] != null) {
return device.should_be_running__release[0];
if (device.is_pinned_on__release[0] != null) {
return device.is_pinned_on__release[0];
}
return device.belongs_to__application[0]?.should_be_running__release[0];
};
Expand Down
8 changes: 1 addition & 7 deletions src/features/device-state/state-patch-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,7 @@ export const v3ValidPatchFields: Array<

export const v2ValidPatchFields: Array<
Exclude<keyof NonNullable<StatePatchV2Body['local']>, 'apps'>
> = [
...v3ValidPatchFields,
'should_be_running__release',
'device_name',
'note',
'download_progress',
];
> = [...v3ValidPatchFields, 'device_name', 'note', 'download_progress'];

const SHORT_TEXT_LENGTH = 255;
const ADDRESS_DELIMITER = ' ';
Expand Down
16 changes: 8 additions & 8 deletions src/features/devices/hooks/defaults-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ hooks.addPureHook('POST', 'resin', 'device', {
}

// TODO[device management next step]: Drop this after re-migrating all data on step 2:
if (request.values.should_be_running__release !== undefined) {
if (request.values.is_pinned_on__release !== undefined) {
// Add an async boundary so that value updates,
// and doesn't remove the properties that we add.
await null;
request.values.is_pinned_on__release =
request.values.should_be_running__release;
request.values.should_be_running__release =
request.values.is_pinned_on__release;
}
},
});
Expand Down Expand Up @@ -69,9 +69,9 @@ hooks.addPureHook('PATCH', 'resin', 'device', {
// build has been targeted, instead of pointing to a build of the wrong application
if (
request.values.belongs_to__application != null &&
request.values.should_be_running__release === undefined
request.values.is_pinned_on__release === undefined
) {
request.values.should_be_running__release = null;
request.values.is_pinned_on__release = null;
}

if (request.values.is_connected_to_vpn != null) {
Expand All @@ -84,12 +84,12 @@ hooks.addPureHook('PATCH', 'resin', 'device', {
}

// TODO[device management next step]: Drop this after re-migrating all data on step 2:
if (request.values.should_be_running__release !== undefined) {
if (request.values.is_pinned_on__release !== undefined) {
// Add an async boundary so that value updates,
// and doesn't remove the properties that we add.
await null;
request.values.is_pinned_on__release =
request.values.should_be_running__release;
request.values.should_be_running__release =
request.values.is_pinned_on__release;
}
},
});
2 changes: 1 addition & 1 deletion src/features/devices/models/device-additions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ export const addToModel = (abstractSql: AbstractSqlModel) => {
],
[
'Coalesce',
['ReferencedField', 'device', 'should be running-release'],
['ReferencedField', 'device', 'is pinned on-release'],
[
'SelectQuery',
[
Expand Down
2 changes: 1 addition & 1 deletion src/features/releases/hooks/restrict-release-deletion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ hooks.addPureHook('DELETE', 'resin', 'release', {
$top: 1,
$select: 'id',
$filter: {
should_be_running__release: { $in: affectedIds },
is_pinned_on__release: { $in: affectedIds },
},
},
});
Expand Down
2 changes: 1 addition & 1 deletion src/lib/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export const DEVICE_API_KEY_PERMISSIONS = [
`device/any(d:${matchesNonFrozenDeviceActor('d')})`,
),
'resin.application_config_variable.read?application/canAccess()',
'resin.release.read?should_be_running_on__device/canAccess() or belongs_to__application/canAccess()',
'resin.release.read?is_pinned_to__device/canAccess() or belongs_to__application/canAccess()',
'resin.release_tag.read?release/canAccess()',
'resin.device_environment_variable.read?device/canAccess()',
...writePerms(
Expand Down
21 changes: 20 additions & 1 deletion src/translations/v6/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@ const addReadOnlyHook = (
});
};

const translatePropertyTo =
(currentModelField: string, newerModelField: string) =>
async ({ request }: Pick<sbvrUtils.HookArgs, 'request'>) => {
if (Object.hasOwn(request.values, currentModelField)) {
// Add an async boundary so that other sync hooks can use
// the untranslated values.
await null;
request.values[newerModelField] = request.values[currentModelField];
delete request.values[currentModelField];
}
};

const translateDeviceTypeTo =
(toField: string) =>
async ({ request, api }: sbvrUtils.HookArgs) => {
Expand Down Expand Up @@ -48,12 +60,19 @@ addReadOnlyHook(['PUT', 'POST', 'PATCH'], 'application', {
}
},
});

const translateDeviceIsPinnedOnRelease = translatePropertyTo(
'should_be_running__release',
'is_pinned_on__release',
);

addReadOnlyHook(['PUT', 'POST', 'PATCH'], 'device', {
POSTPARSE({ request }) {
async POSTPARSE({ request }) {
// Dependent device properties were removed so we block trying to set them
if (request.values.is_managed_by__device != null) {
throw new errors.BadRequestError();
}
await translateDeviceIsPinnedOnRelease({ request });
},
});

Expand Down
1 change: 1 addition & 0 deletions src/translations/v6/v6.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export const getV6Translations = (abstractSqlModel = v6AbstractSqlModel) => {
'is managed by-device': ['Cast', ['Null'], 'Integer'],
'logs channel': ['Cast', ['Null'], 'Short Text'],
'vpn address': ['Cast', ['Null'], 'Short Text'],
'should be running-release': 'is pinned on-release',
// We are redefining the overall_status rather than translating it, so that:
// • the v6 overall_status performances does not degrades from the additional FROMs
// • the behavior does not change if we later add new statuses in the v7 one
Expand Down
14 changes: 9 additions & 5 deletions test/14_release-pinning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ import type { Application, DeviceType, Release } from '../src/balena-model.js';

export default () => {
versions.test((version, pineTest) => {
const pinnedOnReleaseField = versions.gt(version, 'v6')
? 'is_pinned_on__release'
: 'should_be_running__release';

describe(`Tracking latest release`, () => {
let fx: fixtures.Fixtures;
let admin: UserObjectParam;
Expand Down Expand Up @@ -85,7 +89,7 @@ export default () => {
await supertest(admin)
.patch(`/${version}/device(${device.id})`)
.send({
should_be_running__release: pinnedRelease.id,
[pinnedOnReleaseField]: pinnedRelease.id,
})
.expect(200);

Expand All @@ -106,7 +110,7 @@ export default () => {
await supertest(admin)
.patch(`/${version}/device(${device.id})`)
.send({
should_be_running__release: null,
[pinnedOnReleaseField]: null,
})
.expect(200);
});
Expand Down Expand Up @@ -464,7 +468,7 @@ export default () => {
await supertest(device4)
.patch(`/${version}/device(${device4.id})`)
.send({
should_be_running__release: app3ReleaseId,
[pinnedOnReleaseField]: app3ReleaseId,
})
.expect(200);

Expand All @@ -491,7 +495,7 @@ export default () => {
await supertest(device4)
.patch(`/${version}/device(${device4.id})`)
.send({
should_be_running__release: app3ReleaseId,
[pinnedOnReleaseField]: app3ReleaseId,
})
.expect(200);
},
Expand Down Expand Up @@ -579,7 +583,7 @@ export default () => {
it('should not be able to delete a release if a device is pinned to it', async function () {
await supertest(admin)
.patch(`/${version}/device(${device4.id})`)
.send({ should_be_running__release: app4ReleaseId });
.send({ [pinnedOnReleaseField]: app4ReleaseId });
await supertest(admin)
.delete(`/${version}/release(${app4ReleaseId})`)
.expect(
Expand Down
12 changes: 10 additions & 2 deletions test/scenarios/unpin-device-after-release.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,18 @@ import {
addServiceToApp,
addImageToRelease,
} from '../test-lib/api-helpers.js';
import * as versions from '../test-lib/versions.js';

const version = 'resin';

export default () => {
// we don't really need `versions.gt` here since `const version = 'resin';`,
// but used it anyway for consistency and in case we later prefer to run
// the scenarion with multiple versions.
const pinnedOnReleaseField = versions.gt(version, 'v6')
? 'is_pinned_on__release'
: 'should_be_running__release';

describe('Device with missing service installs', () => {
let fx: fixtures.Fixtures;
let admin: UserObjectParam;
Expand Down Expand Up @@ -81,7 +89,7 @@ export default () => {
await supertest(admin)
.patch(`/${version}/device(${device.id})`)
.send({
should_be_running__release: releases['deadbeef'],
[pinnedOnReleaseField]: releases['deadbeef'],
})
.expect(200);

Expand Down Expand Up @@ -163,7 +171,7 @@ export default () => {
await supertest(admin)
.patch(`/${version}/device(${device.id})`)
.send({
should_be_running__release: null,
[pinnedOnReleaseField]: null,
})
.expect(200);
});
Expand Down

0 comments on commit 310e1a4

Please sign in to comment.