From 16a6c09fa974d335252693e0b4fdc798bf288402 Mon Sep 17 00:00:00 2001 From: Andrea Rosci Date: Sun, 9 Jun 2024 19:29:25 +0200 Subject: [PATCH] is_pinned_on__release: Add translations and only read the new column Change-type: minor --- src/balena.sbvr | 2 +- .../ci-cd/hooks/app-update-trigger.ts | 2 +- .../ci-cd/hooks/device-update-trigger.ts | 2 +- src/features/ci-cd/hooks/service-installs.ts | 8 +++---- .../device-state/routes/state-get-v2.ts | 2 +- .../device-state/routes/state-get-v3.ts | 8 +++---- .../device-state/routes/state-patch-v2.ts | 12 ++++++----- src/features/device-state/state-get-utils.ts | 4 ++-- .../device-state/state-patch-utils.ts | 8 +------ .../devices/hooks/defaults-validation.ts | 16 +++++++------- .../devices/models/device-additions.ts | 2 +- .../hooks/restrict-release-deletion.ts | 2 +- src/lib/auth.ts | 2 +- src/translations/v6/hooks.ts | 21 ++++++++++++++++++- src/translations/v6/v6.ts | 1 + test/14_release-pinning.ts | 14 ++++++++----- test/scenarios/unpin-device-after-release.ts | 12 +++++++++-- 17 files changed, 73 insertions(+), 45 deletions(-) diff --git a/src/balena.sbvr b/src/balena.sbvr index 9492892b38..f4b4b1a6cf 100644 --- a/src/balena.sbvr +++ b/src/balena.sbvr @@ -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". diff --git a/src/features/ci-cd/hooks/app-update-trigger.ts b/src/features/ci-cd/hooks/app-update-trigger.ts index 5012f9d438..e4a6cd8a72 100644 --- a/src/features/ci-cd/hooks/app-update-trigger.ts +++ b/src/features/ci-cd/hooks/app-update-trigger.ts @@ -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, }); diff --git a/src/features/ci-cd/hooks/device-update-trigger.ts b/src/features/ci-cd/hooks/device-update-trigger.ts index fbf24b3ef7..b0808760b7 100644 --- a/src/features/ci-cd/hooks/device-update-trigger.ts +++ b/src/features/ci-cd/hooks/device-update-trigger.ts @@ -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 diff --git a/src/features/ci-cd/hooks/service-installs.ts b/src/features/ci-cd/hooks/service-installs.ts index 0592751453..862302b2d8 100644 --- a/src/features/ci-cd/hooks/service-installs.ts +++ b/src/features/ci-cd/hooks/service-installs.ts @@ -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, @@ -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({ diff --git a/src/features/device-state/routes/state-get-v2.ts b/src/features/device-state/routes/state-get-v2.ts index c48b5e3d5f..f100c46b9c 100644 --- a/src/features/device-state/routes/state-get-v2.ts +++ b/src/features/device-state/routes/state-get-v2.ts @@ -203,7 +203,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: { diff --git a/src/features/device-state/routes/state-get-v3.ts b/src/features/device-state/routes/state-get-v3.ts index c5bf380af7..72094784be 100644 --- a/src/features/device-state/routes/state-get-v3.ts +++ b/src/features/device-state/routes/state-get-v3.ts @@ -226,7 +226,7 @@ const deviceExpand = { device_environment_variable: { $select: ['name', 'value'], }, - should_be_running__release: releaseExpand, + is_pinned_on__release: releaseExpand, service_install: { $select: ['id'], $expand: { @@ -323,7 +323,7 @@ const getStateV3 = async (req: Request, uuid: string): Promise => { 'io.balena.image.store': 'root', }, ), - ...getAppState(device, 'should_be_running__release', config), + ...getAppState(device, 'is_pinned_on__release', config), }; const state: StateV3 = { @@ -368,7 +368,7 @@ 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, @@ -376,7 +376,7 @@ const getAppState = ( ): 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 { diff --git a/src/features/device-state/routes/state-patch-v2.ts b/src/features/device-state/routes/state-patch-v2.ts index 53d54bab67..9533a84241 100644 --- a/src/features/device-state/routes/state-patch-v2.ts +++ b/src/features/device-state/routes/state-patch-v2.ts @@ -8,6 +8,7 @@ import { } from '../../../infra/error-handling/index.js'; import { sbvrUtils, errors } from '@balena/pinejs'; import { getIP } from '../../../lib/utils.js'; +import type { Application, Device } from '../../../balena-model.js'; import { shouldUpdateMetrics, metricsPatchFields, @@ -18,7 +19,6 @@ import { limitMetricNumbers, } from '../state-patch-utils.js'; import type { ResolveDeviceInfoCustomObject } from '../middleware.js'; -import type { Application, Device } from '../../../balena-model.js'; const { BadRequestError, UnauthorizedError } = errors; const { api } = sbvrUtils; @@ -147,10 +147,9 @@ export const statePatchV2: RequestHandler = async (req, res) => { const { apps } = local; let deviceBody: Pick & - Partial> = _.pick( - local, - v2ValidPatchFields, - ); + Partial< + Pick + > = _.pick(local, v2ValidPatchFields); let metricsBody: Pick = _.pick(local, metricsPatchFields); limitMetricNumbers(metricsBody); @@ -164,6 +163,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; } diff --git a/src/features/device-state/state-get-utils.ts b/src/features/device-state/state-get-utils.ts index 04d33fdbc6..d1d59cb5bb 100644 --- a/src/features/device-state/state-get-utils.ts +++ b/src/features/device-state/state-get-utils.ts @@ -68,8 +68,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]; }; diff --git a/src/features/device-state/state-patch-utils.ts b/src/features/device-state/state-patch-utils.ts index 5df1722ca1..906911b6ae 100644 --- a/src/features/device-state/state-patch-utils.ts +++ b/src/features/device-state/state-patch-utils.ts @@ -32,13 +32,7 @@ export const v3ValidPatchFields: Array< export const v2ValidPatchFields: Array< Exclude, '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 = ' '; diff --git a/src/features/devices/hooks/defaults-validation.ts b/src/features/devices/hooks/defaults-validation.ts index 6ff75fd618..d84e1f0ce7 100644 --- a/src/features/devices/hooks/defaults-validation.ts +++ b/src/features/devices/hooks/defaults-validation.ts @@ -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; } }, }); @@ -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) { @@ -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; } }, }); diff --git a/src/features/devices/models/device-additions.ts b/src/features/devices/models/device-additions.ts index eb0b11313b..0e51109cc4 100644 --- a/src/features/devices/models/device-additions.ts +++ b/src/features/devices/models/device-additions.ts @@ -387,7 +387,7 @@ export const addToModel = (abstractSql: AbstractSqlModel) => { ], [ 'Coalesce', - ['ReferencedField', 'device', 'should be running-release'], + ['ReferencedField', 'device', 'is pinned on-release'], [ 'SelectQuery', [ diff --git a/src/features/releases/hooks/restrict-release-deletion.ts b/src/features/releases/hooks/restrict-release-deletion.ts index ebfc6c3a50..6295fec7de 100644 --- a/src/features/releases/hooks/restrict-release-deletion.ts +++ b/src/features/releases/hooks/restrict-release-deletion.ts @@ -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 }, }, }, }); diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 951ae5aba3..4c04b3bbd8 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -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( diff --git a/src/translations/v6/hooks.ts b/src/translations/v6/hooks.ts index db04f03533..2261b67ab2 100644 --- a/src/translations/v6/hooks.ts +++ b/src/translations/v6/hooks.ts @@ -16,6 +16,18 @@ const addReadOnlyHook = ( }); }; +const translatePropertyTo = + (currentModelField: string, newerModelField: string) => + async ({ request }: Pick) => { + 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) => { @@ -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 }); }, }); diff --git a/src/translations/v6/v6.ts b/src/translations/v6/v6.ts index 810d064e6d..10787b77ed 100644 --- a/src/translations/v6/v6.ts +++ b/src/translations/v6/v6.ts @@ -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 diff --git a/test/14_release-pinning.ts b/test/14_release-pinning.ts index c9856eba0f..c2683d49d0 100644 --- a/test/14_release-pinning.ts +++ b/test/14_release-pinning.ts @@ -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; @@ -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); @@ -106,7 +110,7 @@ export default () => { await supertest(admin) .patch(`/${version}/device(${device.id})`) .send({ - should_be_running__release: null, + [pinnedOnReleaseField]: null, }) .expect(200); }); @@ -464,7 +468,7 @@ export default () => { await supertest(device4) .patch(`/${version}/device(${device4.id})`) .send({ - should_be_running__release: app3ReleaseId, + [pinnedOnReleaseField]: app3ReleaseId, }) .expect(200); @@ -491,7 +495,7 @@ export default () => { await supertest(device4) .patch(`/${version}/device(${device4.id})`) .send({ - should_be_running__release: app3ReleaseId, + [pinnedOnReleaseField]: app3ReleaseId, }) .expect(200); }, @@ -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( diff --git a/test/scenarios/unpin-device-after-release.ts b/test/scenarios/unpin-device-after-release.ts index 710921e44f..04677ada86 100644 --- a/test/scenarios/unpin-device-after-release.ts +++ b/test/scenarios/unpin-device-after-release.ts @@ -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; @@ -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); @@ -163,7 +171,7 @@ export default () => { await supertest(admin) .patch(`/${version}/device(${device.id})`) .send({ - should_be_running__release: null, + [pinnedOnReleaseField]: null, }) .expect(200); });