From a7bf7aaa64a2c6f7ea0e70848ee35e513e5fb6ad Mon Sep 17 00:00:00 2001 From: Jens Reimann Date: Wed, 11 May 2022 16:33:08 +0200 Subject: [PATCH] fix: align generation field with k8s api This change only incremenets the generation field when a change of spec section has occurred. Internally there is a revision field, which increments with every change. However, that is not exposed, as the resourceVersion field should be used for detecting any change. While the generation field is intended to track changes of the specification. This fixes the "obverved generation" never being equal to the "generation", as the status section update again increments the generation field. --- authentication-service/src/lib.rs | 2 +- .../tests/sql/10-basic/up.sql | 6 ++ .../tests/sql/20-x509/up.sql | 4 + .../tests/sql/30-hashed/up.sql | 8 ++ .../20220511145200_add_revision/down.sql | 13 +++ .../20220511145200_add_revision/up.sql | 35 ++++++++ database-common/src/error.rs | 6 +- database-common/src/models/app.rs | 25 ++++-- database-common/src/models/device.rs | 25 ++++-- database-common/src/models/gen.rs | 88 +++++++++++++++---- database-common/src/models/outbox.rs | 18 ++-- database-common/tests/outbox.rs | 24 ++--- .../src/service/admin/mod.rs | 6 +- .../src/service/error.rs | 6 +- .../src/service/management/mod.rs | 22 +++-- device-management-service/src/service/mod.rs | 10 ++- device-management-service/tests/apps.rs | 28 +++--- device-management-service/tests/common.rs | 14 +-- device-management-service/tests/devices.rs | 28 +++--- operator-common/src/controller/base/queue.rs | 20 ++--- registry-events/src/db.rs | 12 +-- registry-events/src/lib.rs | 34 +++---- registry-events/src/mock.rs | 6 +- user-auth-service/tests/sql/10-basic/up.sql | 2 + user-auth-service/tests/sql/20-members/up.sql | 2 + 25 files changed, 305 insertions(+), 139 deletions(-) create mode 100644 database-common/migrations/20220511145200_add_revision/down.sql create mode 100644 database-common/migrations/20220511145200_add_revision/up.sql diff --git a/authentication-service/src/lib.rs b/authentication-service/src/lib.rs index c39654360..f43aa8528 100644 --- a/authentication-service/src/lib.rs +++ b/authentication-service/src/lib.rs @@ -8,8 +8,8 @@ use drogue_cloud_service_api::{ health::HealthChecked, webapp::{self as actix_web, prom::PrometheusMetricsBuilder}, }; -use drogue_cloud_service_common::app::run_main; use drogue_cloud_service_common::{ + app::run_main, defaults, health::HealthServerConfig, openid::{Authenticator, AuthenticatorConfig}, diff --git a/authentication-service/tests/sql/10-basic/up.sql b/authentication-service/tests/sql/10-basic/up.sql index 7f894024d..8bb4a51e1 100644 --- a/authentication-service/tests/sql/10-basic/up.sql +++ b/authentication-service/tests/sql/10-basic/up.sql @@ -8,6 +8,7 @@ INSERT INTO APPLICATIONS ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, DATA ) VALUES ( 'app1', @@ -15,6 +16,7 @@ INSERT INTO APPLICATIONS ( '2020-01-01 00:00:00', 'A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11', 0, + 0, '{}'::JSONB ); @@ -39,6 +41,7 @@ INSERT INTO DEVICES ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, DATA ) VALUES ( 'app1', @@ -47,6 +50,7 @@ INSERT INTO DEVICES ( '2020-01-01 00:00:00', 'A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11', 0, + 0, '{ "spec": { "credentials": { @@ -85,6 +89,7 @@ INSERT INTO DEVICES ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, DATA ) VALUES ( 'app1', @@ -93,6 +98,7 @@ INSERT INTO DEVICES ( '2020-01-01 00:00:00', 'A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11', 0, + 0, '{ "spec": { "credentials": { diff --git a/authentication-service/tests/sql/20-x509/up.sql b/authentication-service/tests/sql/20-x509/up.sql index 7f13b068a..df505e6ff 100644 --- a/authentication-service/tests/sql/20-x509/up.sql +++ b/authentication-service/tests/sql/20-x509/up.sql @@ -8,6 +8,7 @@ INSERT INTO APPLICATIONS ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, DATA ) VALUES ( 'app2', @@ -15,6 +16,7 @@ INSERT INTO APPLICATIONS ( '2020-01-01 00:00:00', 'A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11', 0, + 0, '{ "spec": { "trustAnchors": { @@ -69,6 +71,7 @@ INSERT INTO DEVICES ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, DATA ) VALUES ( 'app2', @@ -77,6 +80,7 @@ INSERT INTO DEVICES ( '2020-01-01 00:00:00', 'A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11', 0, + 0, '{ "spec": { "credentials": {} diff --git a/authentication-service/tests/sql/30-hashed/up.sql b/authentication-service/tests/sql/30-hashed/up.sql index d63ec1a92..776e8bc49 100644 --- a/authentication-service/tests/sql/30-hashed/up.sql +++ b/authentication-service/tests/sql/30-hashed/up.sql @@ -8,6 +8,7 @@ INSERT INTO APPLICATIONS ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, DATA ) VALUES ( 'app3', @@ -15,6 +16,7 @@ INSERT INTO APPLICATIONS ( '2021-01-01 00:00:00', '547531d4-c7ad-11eb-abee-d45d6455d2cc', 0, + 0, '{}'::JSONB ); @@ -39,6 +41,7 @@ INSERT INTO DEVICES ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, DATA ) VALUES ( 'app3', @@ -47,6 +50,7 @@ INSERT INTO DEVICES ( '2020-01-01 00:00:00', 'A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11', 0, + 0, '{ "spec": { "credentials": { @@ -82,6 +86,7 @@ INSERT INTO DEVICES ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, DATA ) VALUES ( 'app3', @@ -90,6 +95,7 @@ INSERT INTO DEVICES ( '2020-01-01 00:00:00', 'A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11', 0, + 0, '{ "spec": { "credentials": { @@ -125,6 +131,7 @@ INSERT INTO DEVICES ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, DATA ) VALUES ( 'app3', @@ -133,6 +140,7 @@ INSERT INTO DEVICES ( '2020-01-01 00:00:00', 'A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11', 0, + 0, '{ "spec": { "credentials": { diff --git a/database-common/migrations/20220511145200_add_revision/down.sql b/database-common/migrations/20220511145200_add_revision/down.sql new file mode 100644 index 000000000..9f4adf6eb --- /dev/null +++ b/database-common/migrations/20220511145200_add_revision/down.sql @@ -0,0 +1,13 @@ +ALTER TABLE applications + DROP COLUMN REVISION +; + +ALTER TABLE devices + DROP COLUMN REVISION +; + +ALTER TABLE outbox + RENAME COLUMN REVISION TO GENERATION; + +ALTER TABLE WORKQUEUE + RENAME COLUMN REV TO GEN; diff --git a/database-common/migrations/20220511145200_add_revision/up.sql b/database-common/migrations/20220511145200_add_revision/up.sql new file mode 100644 index 000000000..379f24c14 --- /dev/null +++ b/database-common/migrations/20220511145200_add_revision/up.sql @@ -0,0 +1,35 @@ +BEGIN; + +-- applications + +ALTER TABLE applications + ADD COLUMN REVISION BIGINT NOT NULL DEFAULT 0 +; + +UPDATE applications SET REVISION = GENERATION; + +ALTER TABLE applications + ALTER COLUMN REVISION DROP DEFAULT +; + +-- devices + +ALTER TABLE devices + ADD COLUMN REVISION BIGINT NOT NULL DEFAULT 0 +; + +UPDATE devices SET REVISION = GENERATION; + +ALTER TABLE devices + ALTER COLUMN REVISION DROP DEFAULT +; + +-- commit + +COMMIT; + +ALTER TABLE outbox + RENAME COLUMN GENERATION TO REVISION; + +ALTER TABLE WORKQUEUE + RENAME COLUMN GEN TO REV; diff --git a/database-common/src/error.rs b/database-common/src/error.rs index 2004f5ed3..fdb47dae2 100644 --- a/database-common/src/error.rs +++ b/database-common/src/error.rs @@ -1,4 +1,4 @@ -use crate::models::GenerationError; +use crate::models::AdvanceError; use deadpool_postgres::PoolError; use drogue_cloud_service_api::webapp::{HttpResponse, ResponseError}; use serde::{Deserialize, Serialize}; @@ -27,8 +27,8 @@ pub enum ServiceError { OptimisticLockFailed, } -impl From for ServiceError { - fn from(err: GenerationError) -> Self { +impl From for ServiceError { + fn from(err: AdvanceError) -> Self { ServiceError::Internal(err.to_string()) } } diff --git a/database-common/src/models/app.rs b/database-common/src/models/app.rs index 2d172042f..aba2f7358 100644 --- a/database-common/src/models/app.rs +++ b/database-common/src/models/app.rs @@ -8,7 +8,7 @@ use crate::{ sql::{slice_iter, SelectBuilder}, Lock, TypedAlias, }, - update_aliases, Client, + revision, update_aliases, Client, }; use async_trait::async_trait; use chrono::{DateTime, Utc}; @@ -35,8 +35,12 @@ pub struct Application { pub labels: HashMap, pub annotations: HashMap, pub creation_timestamp: DateTime, + /// Updated with each change pub resource_version: Uuid, + /// Incremented with each change to the .spec section pub generation: u64, + /// Incremented with each change + pub revision: u64, pub deletion_timestamp: Option>, pub finalizers: Vec, @@ -53,6 +57,7 @@ pub struct Application { diffable!(Application); generation!(Application => generation); +revision!(Application => revision); default_resource!(Application); impl Resource for Application { @@ -193,6 +198,7 @@ impl<'c, C: Client> PostgresApplicationAccessor<'c, C> { creation_timestamp: row.try_get("CREATION_TIMESTAMP")?, generation: row.try_get::<_, i64>("GENERATION")? as u64, + revision: row.try_get::<_, i64>("REVISION")? as u64, resource_version: row.try_get("RESOURCE_VERSION")?, labels: super::row_to_map(&row, "LABELS")?, annotations: super::row_to_map(&row, "ANNOTATIONS")?, @@ -250,6 +256,7 @@ SELECT A2.LABELS, A2.CREATION_TIMESTAMP, A2.GENERATION, + A2.REVISION, A2.RESOURCE_VERSION, A2.ANNOTATIONS, A2.DELETION_TIMESTAMP, @@ -301,6 +308,7 @@ SELECT ANNOTATIONS, CREATION_TIMESTAMP, GENERATION, + REVISION, RESOURCE_VERSION, DELETION_TIMESTAMP, FINALIZERS, @@ -359,6 +367,7 @@ INSERT INTO APPLICATIONS ( ANNOTATIONS, CREATION_TIMESTAMP, GENERATION, + REVISION, RESOURCE_VERSION, FINALIZERS, OWNER, @@ -373,7 +382,8 @@ INSERT INTO APPLICATIONS ( $7, $8, $9, - $10 + $10, + $11 )"#, &[ &name, @@ -382,6 +392,7 @@ INSERT INTO APPLICATIONS ( &Json(annotations), &Utc::now(), &(application.generation as i64), + &(application.revision as i64), &Uuid::new_v4(), &application.finalizers, &application.owner, @@ -415,10 +426,11 @@ SET LABELS = $2, ANNOTATIONS = $3, GENERATION = $4, - RESOURCE_VERSION = $5, - DELETION_TIMESTAMP = $6, - FINALIZERS = $7, - DATA = $8 + REVISION = $5, + RESOURCE_VERSION = $6, + DELETION_TIMESTAMP = $7, + FINALIZERS = $8, + DATA = $9 WHERE NAME = $1 "#, @@ -427,6 +439,7 @@ WHERE &Json(labels), &Json(annotations), &(application.generation as i64), + &(application.revision as i64), &Uuid::new_v4(), &application.deletion_timestamp, &application.finalizers, diff --git a/database-common/src/models/device.rs b/database-common/src/models/device.rs index ae5f58290..ce3c1be58 100644 --- a/database-common/src/models/device.rs +++ b/database-common/src/models/device.rs @@ -6,7 +6,7 @@ use crate::{ sql::{slice_iter, SelectBuilder}, Lock, TypedAlias, }, - update_aliases, Client, + revision, update_aliases, Client, }; use async_trait::async_trait; use chrono::{DateTime, Utc}; @@ -33,8 +33,12 @@ pub struct Device { pub labels: HashMap, pub annotations: HashMap, pub creation_timestamp: DateTime, + /// Updated with each change pub resource_version: Uuid, + /// Incremented with each change to the .spec section pub generation: u64, + /// Incremented with each change + pub revision: u64, pub deletion_timestamp: Option>, pub finalizers: Vec, @@ -43,6 +47,7 @@ pub struct Device { diffable!(Device); generation!(Device => generation); +revision!(Device => revision); default_resource!(Device); impl From for registry::v1::Device { @@ -148,6 +153,7 @@ impl<'c, C: Client> PostgresDeviceAccessor<'c, C> { creation_timestamp: row.try_get("CREATION_TIMESTAMP")?, generation: row.try_get::<_, i64>("GENERATION")? as u64, + revision: row.try_get::<_, i64>("REVISION")? as u64, resource_version: row.try_get("RESOURCE_VERSION")?, labels: super::row_to_map(&row, "LABELS")?, annotations: super::row_to_map(&row, "ANNOTATIONS")?, @@ -198,6 +204,7 @@ SELECT D.LABELS, D.CREATION_TIMESTAMP, D.GENERATION, + D.REVISION, D.RESOURCE_VERSION, D.ANNOTATIONS, D.DELETION_TIMESTAMP, @@ -264,6 +271,7 @@ SELECT ANNOTATIONS, CREATION_TIMESTAMP, GENERATION, + REVISION, RESOURCE_VERSION, DELETION_TIMESTAMP, FINALIZERS, @@ -319,6 +327,7 @@ INSERT INTO DEVICES ( ANNOTATIONS, CREATION_TIMESTAMP, GENERATION, + REVISION, RESOURCE_VERSION, FINALIZERS, DATA @@ -331,7 +340,8 @@ INSERT INTO DEVICES ( $6, $7, $8, - $9 + $9, + $10 )"#, &[ &device.application, @@ -340,6 +350,7 @@ INSERT INTO DEVICES ( &Json(&device.annotations), &Utc::now(), &(device.generation as i64), + &(device.revision as i64), &Uuid::new_v4(), &device.finalizers, &Json(&device.data), @@ -369,10 +380,11 @@ SET LABELS = $3, ANNOTATIONS = $4, GENERATION = $5, - RESOURCE_VERSION = $6, - DELETION_TIMESTAMP = $7, - FINALIZERS = $8, - DATA = $9 + REVISION = $6, + RESOURCE_VERSION = $7, + DELETION_TIMESTAMP = $8, + FINALIZERS = $9, + DATA = $10 WHERE APP = $1 AND NAME = $2 "#, @@ -382,6 +394,7 @@ WHERE &Json(device.labels), &Json(device.annotations), &(device.generation as i64), + &(device.revision as i64), &Uuid::new_v4(), &device.deletion_timestamp, &device.finalizers, diff --git a/database-common/src/models/gen.rs b/database-common/src/models/gen.rs index 3b9d81b16..c814c36ad 100644 --- a/database-common/src/models/gen.rs +++ b/database-common/src/models/gen.rs @@ -12,13 +12,13 @@ macro_rules! generation { fn set_generation( &mut self, generation: u64, - ) -> Result { + ) -> Result { let current = self.generation(); if current < generation { self.$f = generation; Ok(generation) } else { - Err($crate::models::gen::GenerationError::NotIncrementing { + Err($crate::models::gen::AdvanceError::NotIncrementing { current, desired: generation, }) @@ -28,29 +28,87 @@ macro_rules! generation { }; } +#[macro_export] +macro_rules! revision { + ($t:ty => $f:ident) => { + impl $crate::models::gen::Revision for $t { + #[inline] + fn revision(&self) -> u64 { + self.$f + } + + fn set_revision( + &mut self, + revision: u64, + ) -> Result { + let current = self.revision(); + if current < revision { + self.$f = revision; + Ok(revision) + } else { + Err($crate::models::gen::AdvanceError::NotIncrementing { + current, + desired: revision, + }) + } + } + } + }; +} + #[derive(Debug, Error)] -pub enum GenerationError { - #[error("Generation not incrementing (was: {current}, desired: {desired})")] +pub enum AdvanceError { + #[error("not incrementing (was: {current}, desired: {desired})")] NotIncrementing { current: u64, desired: u64 }, } -pub trait Generation { - /// Get the generation from the provided resource, increment it by one, and set it to this resource. - fn set_incremented_generation( - &mut self, - current: &dyn Generation, - ) -> Result { - self.set_generation(current.generation() + 1) +pub trait Advance { + /// Advance the generation by one for a spec change, and advance the revision in any case. + fn advance_from(&mut self, paths: &[String], current: &S) -> Result + where + S: Generation + Revision; + + /// Increment the revision of this resource by one + fn advance_revision(&mut self) -> Result; +} + +impl Advance for T +where + T: Generation + Revision, +{ + fn advance_from(&mut self, paths: &[String], current: &S) -> Result + where + S: Generation + Revision, + { + let result = self.set_revision(current.revision() + 1); + + for path in paths { + if path.starts_with(".spec") { + self.set_generation(current.generation() + 1)?; + break; + } + } + + result } - /// Increment the generation of this resource by one - fn increment_generation(&mut self) -> Result { - self.set_generation(self.generation() + 1) + fn advance_revision(&mut self) -> Result { + self.set_revision(self.revision() + 1) } +} +pub trait Generation { /// Get the current generation fn generation(&self) -> u64; /// Set the generation - fn set_generation(&mut self, generation: u64) -> Result; + fn set_generation(&mut self, generation: u64) -> Result; +} + +pub trait Revision { + /// Get the current revision + fn revision(&self) -> u64; + + /// Set the revision + fn set_revision(&mut self, revision: u64) -> Result; } diff --git a/database-common/src/models/outbox.rs b/database-common/src/models/outbox.rs index 77612bd62..25840a4e5 100644 --- a/database-common/src/models/outbox.rs +++ b/database-common/src/models/outbox.rs @@ -16,7 +16,7 @@ pub struct OutboxEntry { pub device: Option, pub path: String, - pub generation: u64, + pub revision: u64, pub uid: String, } @@ -36,7 +36,7 @@ impl TryFrom for OutboxEntry { } }, path: row.try_get("PATH")?, - generation: row.try_get::<_, i64>("GENERATION")? as u64, + revision: row.try_get::<_, i64>("REVISION")? as u64, uid: row.try_get("UID")?, }) } @@ -78,7 +78,7 @@ INSERT INTO outbox ( DEVICE, UID, PATH, - GENERATION, + REVISION, TS ) VALUES ( $1, @@ -92,11 +92,11 @@ INSERT INTO outbox ( ON CONFLICT (APP, DEVICE, PATH) DO UPDATE SET - GENERATION = EXCLUDED.GENERATION, + REVISION = EXCLUDED.REVISION, UID = EXCLUDED.UID, TS = EXCLUDED.TS WHERE - outbox.GENERATION < EXCLUDED.GENERATION + outbox.REVISION < EXCLUDED.REVISION OR outbox.UID != EXCLUDED.UID "#; @@ -126,7 +126,7 @@ DO &entry.device.unwrap_or_default(), &entry.uid, &entry.path, - &(entry.generation as i64), + &(entry.revision as i64), ], ) .await?; @@ -150,7 +150,7 @@ WHERE AND PATH = $3 AND - GENERATION <= $4 + REVISION <= $4 AND UID = $5 "#; @@ -177,7 +177,7 @@ WHERE &entry.app, &entry.device.unwrap_or_default(), &entry.path, - &(entry.generation as i64), + &(entry.revision as i64), &entry.uid, ], ) @@ -196,7 +196,7 @@ WHERE let sql = r#" SELECT - INSTANCE, APP, DEVICE, PATH, GENERATION, UID + INSTANCE, APP, DEVICE, PATH, REVISION, UID FROM outbox WHERE diff --git a/database-common/tests/outbox.rs b/database-common/tests/outbox.rs index 62135e262..a9c464777 100644 --- a/database-common/tests/outbox.rs +++ b/database-common/tests/outbox.rs @@ -41,7 +41,7 @@ async fn test_outbox() -> anyhow::Result<()> { device: None, uid: "a".to_string(), path: ".path".to_string(), - generation: ms1, + revision: ms1, }) .await?; @@ -63,7 +63,7 @@ async fn test_outbox() -> anyhow::Result<()> { device: None, uid: "a".to_string(), path: ".path".to_string(), - generation: ms1, + revision: ms1, }] ); @@ -78,7 +78,7 @@ async fn test_outbox() -> anyhow::Result<()> { device: None, uid: "a".to_string(), path: ".path".to_string(), - generation: ms2, + revision: ms2, }) .await?; @@ -100,7 +100,7 @@ async fn test_outbox() -> anyhow::Result<()> { device: None, uid: "a".to_string(), path: ".path".to_string(), - generation: ms2, + revision: ms2, }] ); @@ -113,7 +113,7 @@ async fn test_outbox() -> anyhow::Result<()> { device: None, uid: "a".to_string(), path: ".path".to_string(), - generation: ms1, + revision: ms1, }) .await?; @@ -135,7 +135,7 @@ async fn test_outbox() -> anyhow::Result<()> { device: None, uid: "a".to_string(), path: ".path".to_string(), - generation: ms2, + revision: ms2, }] ); @@ -148,7 +148,7 @@ async fn test_outbox() -> anyhow::Result<()> { device: None, uid: "a".to_string(), path: ".path".to_string(), - generation: ms2, + revision: ms2, }) .await?; @@ -171,16 +171,16 @@ struct CreateApp { app: String, uid: String, path: String, - generation: u64, + revision: u64, } impl CreateApp { - fn new(app: &str, uid: &str, path: &str, generation: u64) -> Self { + fn new(app: &str, uid: &str, path: &str, revision: u64) -> Self { Self { app: app.to_string(), uid: uid.to_string(), path: path.to_string(), - generation, + revision, } } @@ -192,7 +192,7 @@ impl CreateApp { device: None, uid: self.uid, path: self.path, - generation: self.generation, + revision: self.revision, }) .await?; @@ -245,7 +245,7 @@ async fn test_recreate_resource() -> anyhow::Result<()> { device: None, uid: "b".to_string(), path: ".path".to_string(), - generation: 2, + revision: 2, }] ); diff --git a/device-management-service/src/service/admin/mod.rs b/device-management-service/src/service/admin/mod.rs index 334bddd6a..2a42d16e3 100644 --- a/device-management-service/src/service/admin/mod.rs +++ b/device-management-service/src/service/admin/mod.rs @@ -10,8 +10,10 @@ use drogue_cloud_database_common::{ }, }; use drogue_cloud_registry_events::EventSender; -use drogue_cloud_service_api::admin::{MemberEntry, Members, TransferOwnership}; -use drogue_cloud_service_api::auth::user::{authz::Permission, UserInformation}; +use drogue_cloud_service_api::{ + admin::{MemberEntry, Members, TransferOwnership}, + auth::user::{authz::Permission, UserInformation}, +}; use drogue_cloud_service_common::keycloak::KeycloakClient; use indexmap::map::IndexMap; use tracing::instrument; diff --git a/device-management-service/src/service/error.rs b/device-management-service/src/service/error.rs index df980906f..1dca073d1 100644 --- a/device-management-service/src/service/error.rs +++ b/device-management-service/src/service/error.rs @@ -1,6 +1,6 @@ use actix_web::{HttpResponse, ResponseError}; use drogue_client::error::ErrorInformation; -use drogue_cloud_database_common::{error::ServiceError, models::GenerationError}; +use drogue_cloud_database_common::{error::ServiceError, models::AdvanceError}; use drogue_cloud_registry_events::EventSenderError; use drogue_cloud_service_api::webapp as actix_web; use thiserror::Error; @@ -34,11 +34,11 @@ where } } -impl From for PostgresManagementServiceError +impl From for PostgresManagementServiceError where E: std::error::Error + std::fmt::Debug + 'static, { - fn from(err: GenerationError) -> Self { + fn from(err: AdvanceError) -> Self { PostgresManagementServiceError::Service(err.into()) } } diff --git a/device-management-service/src/service/management/mod.rs b/device-management-service/src/service/management/mod.rs index 4be6fde18..e3678cdca 100644 --- a/device-management-service/src/service/management/mod.rs +++ b/device-management-service/src/service/management/mod.rs @@ -14,7 +14,7 @@ use drogue_cloud_database_common::{ app::{ApplicationAccessor, PostgresApplicationAccessor}, device::{DeviceAccessor, PostgresDeviceAccessor}, diff::diff_paths, - Generation, Lock, + Advance, Lock, }, }; use drogue_cloud_registry_events::{Event, EventSender, SendEvent}; @@ -37,11 +37,13 @@ pub trait ManagementService: Clone { identity: &UserInformation, data: registry::v1::Application, ) -> Result<(), Self::Error>; + async fn get_app( &self, identity: &UserInformation, name: &str, ) -> Result, Self::Error>; + async fn list_apps( &self, identity: UserInformation, @@ -52,11 +54,13 @@ pub trait ManagementService: Clone { Pin> + Send>>, Self::Error, >; + async fn update_app( &self, identity: &UserInformation, data: registry::v1::Application, ) -> Result<(), Self::Error>; + async fn delete_app( &self, identity: &UserInformation, @@ -69,12 +73,14 @@ pub trait ManagementService: Clone { identity: &UserInformation, device: registry::v1::Device, ) -> Result<(), Self::Error>; + async fn get_device( &self, identity: &UserInformation, app: &str, name: &str, ) -> Result, Self::Error>; + async fn list_devices( &self, identity: UserInformation, @@ -86,11 +92,13 @@ pub trait ManagementService: Clone { Pin> + Send>>, Self::Error, >; + async fn update_device( &self, identity: &UserInformation, device: registry::v1::Device, ) -> Result<(), Self::Error>; + async fn delete_device( &self, identity: &UserInformation, @@ -286,7 +294,7 @@ where } // next generation - let generation = current.increment_generation()?; + let revision = current.advance_revision()?; let uid = current.uid; // if there are no finalizers ... @@ -312,7 +320,7 @@ where // create events - let events = Event::new_app(self.instance.clone(), id, uid, generation, paths); + let events = Event::new_app(self.instance.clone(), id, uid, revision, paths); // send events to outbox @@ -519,7 +527,7 @@ where return Ok(()); } - let generation = device.set_incremented_generation(¤t)?; + let revision = device.advance_from(&paths, ¤t)?; let uid = current.uid; accessor @@ -539,7 +547,7 @@ where application, name, uid, - generation, + revision, paths, ); @@ -602,7 +610,7 @@ where // there is no need to use the provided constraints, we as locked the entry "for update" // next generation - let generation = current.increment_generation()?; + let revision = current.advance_revision()?; let uid = current.uid; // if there are no finalizers ... @@ -631,7 +639,7 @@ where application, device, uid, - generation, + revision, path, ); diff --git a/device-management-service/src/service/mod.rs b/device-management-service/src/service/mod.rs index 20d8d2b58..7c8bff1cd 100644 --- a/device-management-service/src/service/mod.rs +++ b/device-management-service/src/service/mod.rs @@ -16,7 +16,7 @@ use drogue_cloud_database_common::{ device::{DeviceAccessor, PostgresDeviceAccessor}, diff::diff_paths, outbox::PostgresOutboxAccessor, - Generation, Lock, TypedAlias, + Advance, Lock, TypedAlias, }, Client, DatabaseService, }; @@ -131,6 +131,7 @@ where labels: app.metadata.labels, annotations: app.metadata.annotations, generation: 0, // will be set internally + revision: 0, // will be set internally creation_timestamp: epoch(), // will be set internally resource_version: Uuid::nil(), // will be set internally deletion_timestamp: None, // will be set internally @@ -193,6 +194,7 @@ where annotations: device.metadata.annotations, creation_timestamp: epoch(), // will be set internally generation: 0, // will be set internally + revision: 0, // will be set internally resource_version: Uuid::nil(), // will be set internally deletion_timestamp: None, // will be set internally finalizers: device.metadata.finalizers, @@ -252,8 +254,8 @@ where return Ok(vec![]); } - // next generation - let generation = app.set_incremented_generation(¤t)?; + // advance generation and revision + let revision = app.advance_from(&paths, ¤t)?; let name = app.name.clone(); let uid = app.uid; @@ -276,7 +278,7 @@ where self.instance.clone(), name, uid, - generation, + revision, paths, )) } diff --git a/device-management-service/tests/apps.rs b/device-management-service/tests/apps.rs index 0e4ec47dd..0f404af48 100644 --- a/device-management-service/tests/apps.rs +++ b/device-management-service/tests/apps.rs @@ -48,7 +48,7 @@ async fn test_create_app() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); }) } @@ -110,7 +110,7 @@ async fn test_crud_app() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // read, must exist @@ -153,7 +153,7 @@ async fn test_crud_app() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".spec.core".into(), - generation: 0, + revision: 0, }]); // read, must exist @@ -194,7 +194,7 @@ async fn test_crud_app() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // try read, must not exist @@ -238,7 +238,7 @@ async fn test_app_labels() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // read, must exist @@ -289,14 +289,14 @@ async fn test_app_labels() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".metadata".into(), - generation: 0, + revision: 0, }, Event::Application { instance: "drogue-instance".into(), application: "app1".into(), uid: "".into(), path: ".spec.core".into(), - generation: 0, + revision: 0, } ]); @@ -355,7 +355,7 @@ async fn test_create_duplicate_app() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); @@ -400,7 +400,7 @@ async fn test_app_trust_anchor() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // read, must exist, with cert @@ -459,13 +459,13 @@ async fn test_app_trust_anchor() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".spec.trustAnchors".into(), - generation: 0, + revision: 0, }, Event::Application { instance: "drogue-instance".into(), application: "app1".into(), uid: "".into(), path: ".status.trustAnchors".into(), - generation: 0, + revision: 0, }]); // read, must exist, but no cert @@ -514,7 +514,7 @@ async fn test_delete_finalizer() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); let resp = TestRequest::delete().uri("/api/registry/v1alpha1/apps/app1").send_request(&app).await; @@ -527,7 +527,7 @@ async fn test_delete_finalizer() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".metadata".into(), - generation: 0, + revision: 0, }]); // read, must exist @@ -567,7 +567,7 @@ async fn test_delete_finalizer() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".metadata".into(), - generation: 0, + revision: 0, }, ]); diff --git a/device-management-service/tests/common.rs b/device-management-service/tests/common.rs index af7fa5cc9..cdd931d0d 100644 --- a/device-management-service/tests/common.rs +++ b/device-management-service/tests/common.rs @@ -97,28 +97,28 @@ pub fn assert_events(actual: Vec>, mut expected: Vec) { for i in actual.iter().zip(expected.iter_mut()) { // this if could be reworked when we have: https://github.com/rust-lang/rust/issues/54883 if let Event::Application { - generation: actual_generation, + revision: actual_revision, uid: actual_uid, .. } | Event::Device { - generation: actual_generation, + revision: actual_revision, uid: actual_uid, .. } = i.0 { if let Event::Application { - generation: expected_generation, + revision: expected_revision, uid: expected_uid, .. } | Event::Device { - generation: expected_generation, + revision: expected_revision, uid: expected_uid, .. } = i.1 { - *expected_generation = *actual_generation; + *expected_revision = *actual_revision; *expected_uid = actual_uid.clone(); } } @@ -248,14 +248,14 @@ mod test { instance: "instance".to_string(), application: "app".to_string(), path: ".".to_string(), - generation: 0, + revision: 0, uid: "a".to_string(), }]; let actual = vec![Event::Application { instance: "instance".to_string(), application: "app".to_string(), path: ".".to_string(), - generation: 12345, + revision: 12345, uid: "b".to_string(), }]; assert_events(vec![actual], expected); diff --git a/device-management-service/tests/devices.rs b/device-management-service/tests/devices.rs index 34414401d..a6bba3786 100644 --- a/device-management-service/tests/devices.rs +++ b/device-management-service/tests/devices.rs @@ -51,7 +51,7 @@ async fn test_create_device() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); let resp = TestRequest::post().uri("/api/registry/v1alpha1/apps/app1/devices").set_json(&json!({ @@ -79,7 +79,7 @@ async fn test_create_device() -> anyhow::Result<()> { device: "device1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); }) @@ -176,7 +176,7 @@ async fn test_create_duplicate_device() -> anyhow::Result<()> { device: "device1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); let resp = TestRequest::post().uri("/api/registry/v1alpha1/apps/app1/devices").set_json(&json!({ @@ -246,7 +246,7 @@ async fn test_crud_device() -> anyhow::Result<()> { device: "device1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // read, must exist now @@ -306,7 +306,7 @@ async fn test_crud_device() -> anyhow::Result<()> { device: "device1".into(), uid: "".into(), path: ".spec.alias".into(), - generation: 0, + revision: 0, }, Event::Device { instance: "drogue-instance".into(), @@ -314,7 +314,7 @@ async fn test_crud_device() -> anyhow::Result<()> { device: "device1".into(), uid: "".into(), path: ".spec.credentials".into(), - generation: 0, + revision: 0, }]); // read, must have changed @@ -362,7 +362,7 @@ async fn test_crud_device() -> anyhow::Result<()> { device: "device1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // read, must no longer not exist @@ -420,7 +420,7 @@ async fn test_delete_app_deletes_device() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // create device @@ -440,7 +440,7 @@ async fn test_delete_app_deletes_device() -> anyhow::Result<()> { device: "device1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // delete application, must succeed @@ -454,7 +454,7 @@ async fn test_delete_app_deletes_device() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // read device, must no longer not exist @@ -484,7 +484,7 @@ async fn test_delete_app_finalizer_device() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // create device @@ -505,7 +505,7 @@ async fn test_delete_app_finalizer_device() -> anyhow::Result<()> { device: "device1".into(), uid: "".into(), path: ".".into(), - generation: 0, + revision: 0, }]); // delete application, must succeed @@ -518,7 +518,7 @@ async fn test_delete_app_finalizer_device() -> anyhow::Result<()> { application: "app1".into(), uid: "".into(), path: ".metadata".into(), - generation: 0, + revision: 0, }]); // the application must still exist @@ -584,7 +584,7 @@ async fn test_delete_app_finalizer_device() -> anyhow::Result<()> { device: "device1".into(), uid: "".into(), path: ".metadata".into(), - generation: 0, + revision: 0, }]); // read device, must still exist diff --git a/operator-common/src/controller/base/queue.rs b/operator-common/src/controller/base/queue.rs index 28c3b6e9e..f56d93b74 100644 --- a/operator-common/src/controller/base/queue.rs +++ b/operator-common/src/controller/base/queue.rs @@ -193,7 +193,7 @@ ON CONFLICT (INSTANCE, TYPE, KEY) DO UPDATE SET TS = EXCLUDED.TS, - GEN = WORKQUEUE.GEN + 1 + REV = WORKQUEUE.REV + 1 WHERE WORKQUEUE.TS > EXCLUDED.TS OR @@ -235,7 +235,7 @@ DO pub struct Entry { pub key: K, pub timestamp: DateTime, - pub gen: u64, + pub rev: u64, } struct InnerReader { @@ -281,7 +281,7 @@ where SELECT KEY, TS, - GEN + REV FROM WORKQUEUE WHERE @@ -301,19 +301,19 @@ LIMIT 1 if let Some(row) = c.query_opt(&stmt, &[&self.instance, &self.r#type]).await? { let key: String = row.try_get("KEY")?; let timestamp = row.try_get("TS")?; - let gen = row.try_get::<_, i64>("GEN")? as u64; + let rev = row.try_get::<_, i64>("REV")? as u64; match K::from_string(key.clone()) { Ok(key) => { return Ok(Some(Entry { key, timestamp, - gen, + rev, })) } Err(_) => { log::info!("Failed to read next entry"); - if let Err(err) = self.do_ack(key, timestamp, gen).await { + if let Err(err) = self.do_ack(key, timestamp, rev).await { // FIXME: circuit breaker log::warn!("Failed to ack invalid entry: {}", err); } @@ -328,7 +328,7 @@ LIMIT 1 #[instrument(skip(self), ret)] async fn ack(&self, entry: Entry) { if let Err(err) = self - .do_ack(entry.key.to_string(), entry.timestamp, entry.gen) + .do_ack(entry.key.to_string(), entry.timestamp, entry.rev) .await { // FIXME: need circuit breaker @@ -337,7 +337,7 @@ LIMIT 1 } #[instrument(skip(self), ret)] - async fn do_ack(&self, key: String, ts: DateTime, gen: u64) -> Result<(), anyhow::Error> { + async fn do_ack(&self, key: String, ts: DateTime, rev: u64) -> Result<(), anyhow::Error> { let c = self.pool.get().await?; let sql = r#" @@ -346,7 +346,7 @@ DELETE FROM WORKQUEUE WHERE TYPE = $2 AND KEY = $3 AND TS <= $4 AND - GEN = $5 + REV = $5 "#; let stmt = c .prepare_typed( @@ -364,7 +364,7 @@ DELETE FROM WORKQUEUE WHERE let r = c .execute( &stmt, - &[&self.instance, &self.r#type, &key, &ts, &(gen as i64)], + &[&self.instance, &self.r#type, &key, &ts, &(rev as i64)], ) .await; diff --git a/registry-events/src/db.rs b/registry-events/src/db.rs index 10233fd0b..6e9d655f7 100644 --- a/registry-events/src/db.rs +++ b/registry-events/src/db.rs @@ -13,14 +13,14 @@ impl From for OutboxEntry { application, uid, path, - generation, + revision, } => OutboxEntry { instance, app: application, device: None, uid, path, - generation, + revision, }, Event::Device { instance, @@ -28,14 +28,14 @@ impl From for OutboxEntry { device, uid, path, - generation, + revision, } => OutboxEntry { instance, app: application, device: Some(device), uid, path, - generation, + revision, }, } } @@ -49,7 +49,7 @@ impl From for Event { application: entry.app, device, path: entry.path, - generation: entry.generation, + revision: entry.revision, uid: entry.uid, } } else { @@ -57,7 +57,7 @@ impl From for Event { instance: entry.instance, application: entry.app, path: entry.path, - generation: entry.generation, + revision: entry.revision, uid: entry.uid, } } diff --git a/registry-events/src/lib.rs b/registry-events/src/lib.rs index eb45a8e8a..ffa025d11 100644 --- a/registry-events/src/lib.rs +++ b/registry-events/src/lib.rs @@ -29,7 +29,7 @@ pub enum Event { application: String, uid: String, path: String, - generation: u64, + revision: u64, }, Device { instance: String, @@ -37,13 +37,13 @@ pub enum Event { device: String, uid: String, path: String, - generation: u64, + revision: u64, }, } #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct EventData { - pub generation: u64, + pub revision: u64, pub uid: String, } @@ -93,7 +93,7 @@ impl Event { .subject() .ok_or_else(|| missing_field("subject"))? .to_string(), - generation: data.generation, + revision: data.revision, uid: data.uid, }) } @@ -117,7 +117,7 @@ impl Event { .subject() .ok_or_else(|| missing_field("subject"))? .to_string(), - generation: data.generation, + revision: data.revision, uid: data.uid, }) } @@ -139,7 +139,7 @@ impl Event { instance: I, app: A, uid: U, - generation: u64, + revision: u64, paths: Vec, ) -> Vec where @@ -152,7 +152,7 @@ impl Event { application: app.to_string(), uid: uid.to_string(), path, - generation, + revision, }) } @@ -162,7 +162,7 @@ impl Event { app_id: A, device_id: D, uid: U, - generation: u64, + revision: u64, paths: Vec, ) -> Vec where @@ -177,7 +177,7 @@ impl Event { device: device_id.to_string(), uid: uid.to_string(), path, - generation, + revision, }) } } @@ -232,7 +232,7 @@ impl TryFrom for cloudevents::Event { instance, application, uid, - generation, + revision, path, } => builder .ty(EVENT_TYPE_APPLICATION) @@ -244,7 +244,7 @@ impl TryFrom for cloudevents::Event { .data( mime::APPLICATION_JSON.to_string(), Data::Json( - serde_json::to_value(&EventData { generation, uid }) + serde_json::to_value(&EventData { revision, uid }) .map_err(EventError::PayloadEncoder)?, ), ), @@ -253,7 +253,7 @@ impl TryFrom for cloudevents::Event { application, device, uid, - generation, + revision, path, } => builder .ty(EVENT_TYPE_DEVICE) @@ -269,7 +269,7 @@ impl TryFrom for cloudevents::Event { .data( mime::APPLICATION_JSON.to_string(), Data::Json( - serde_json::to_value(&EventData { generation, uid }) + serde_json::to_value(&EventData { revision, uid }) .map_err(EventError::PayloadEncoder)?, ), ), @@ -307,7 +307,7 @@ mod test { application: "application".to_string(), uid: "uid".to_string(), path: ".spec.core".to_string(), - generation: 123, + revision: 123, } .try_into()?; @@ -324,7 +324,7 @@ mod test { .extension(EXT_APPLICATION, "application") .data( "application/json", - Data::Json(json!({"generation": 123, "uid": "uid"})) + Data::Json(json!({"revision": 123, "uid": "uid"})) ) .build()? ); @@ -345,7 +345,7 @@ mod test { .extension(EXT_DEVICE, "device") .data( "application/json", - Data::Json(json!({"generation": 321, "uid": "uid"})), + Data::Json(json!({"revision": 321, "uid": "uid"})), ) .build() .context("Failed to build CloudEvent")?; @@ -358,7 +358,7 @@ mod test { application: "application".to_string(), uid: "uid".to_string(), path: ".spec.credentials".to_string(), - generation: 321, + revision: 321, }, event ); diff --git a/registry-events/src/mock.rs b/registry-events/src/mock.rs index 8acc81b4c..3b8f0f6c2 100644 --- a/registry-events/src/mock.rs +++ b/registry-events/src/mock.rs @@ -97,7 +97,7 @@ mod test { assert!(Event::Application { instance: "instance1".into(), application: "app1".into(), - generation: 123, + revision: 123, uid: "a".into(), path: "spec/core".into() } @@ -112,7 +112,7 @@ mod test { vec![Event::Application { instance: "instance1".into(), application: "app1".into(), - generation: 123, + revision: 123, uid: "a".into(), path: "spec/core".into() }] @@ -128,7 +128,7 @@ mod test { vec![Event::Application { instance: "instance1".into(), application: "app1".into(), - generation: 123, + revision: 123, uid: "a".into(), path: "spec/core".into() }] diff --git a/user-auth-service/tests/sql/10-basic/up.sql b/user-auth-service/tests/sql/10-basic/up.sql index 68cb95e7d..31a7c0bb7 100644 --- a/user-auth-service/tests/sql/10-basic/up.sql +++ b/user-auth-service/tests/sql/10-basic/up.sql @@ -8,6 +8,7 @@ INSERT INTO APPLICATIONS ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, OWNER, DATA ) VALUES ( @@ -16,6 +17,7 @@ INSERT INTO APPLICATIONS ( '2020-01-01 00:00:00', 'A0EEBC99-9C0B-4EF8-BB6D-6BB9BD380A11', 0, + 0, 'user1', '{}'::JSONB ); diff --git a/user-auth-service/tests/sql/20-members/up.sql b/user-auth-service/tests/sql/20-members/up.sql index d462d6cc2..09b5de9c7 100644 --- a/user-auth-service/tests/sql/20-members/up.sql +++ b/user-auth-service/tests/sql/20-members/up.sql @@ -8,6 +8,7 @@ INSERT INTO APPLICATIONS ( CREATION_TIMESTAMP, RESOURCE_VERSION, GENERATION, + REVISION, OWNER, MEMBERS, DATA @@ -17,6 +18,7 @@ INSERT INTO APPLICATIONS ( '2020-01-01 00:00:00', 'ab2cefd2-bd6f-11eb-9487-d45d6455d2cc', 0, + 0, 'foo', '{ "bar-admin": { "role": "admin" },