Skip to content

Commit 1141dee

Browse files
passcodreview-hero[bot]
andauthored
fix: auto-reset analytics credentials on auth failure (#18)
Co-authored-by: review-hero[bot] <2896273+review-hero[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6
1 parent dba7aca commit 1141dee

3 files changed

Lines changed: 341 additions & 4 deletions

File tree

src/controllers/replica.rs

Lines changed: 206 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::{collections::HashSet, sync::Arc, time::Duration};
33
use jiff::{SignedDuration, Timestamp};
44
use k8s_openapi::{
55
api::{
6+
apps::v1::Deployment,
67
batch::v1::Job,
78
core::v1::{Pod, Secret},
89
},
@@ -25,6 +26,7 @@ use super::{
2526
jobs::{JobStatus, classify_job},
2627
postgres,
2728
postgres::DEFAULT_PG_VERSION,
29+
restore::{build_credential_reset_job, credential_reset_job_name},
2830
};
2931
use crate::{
3032
context::Context,
@@ -844,6 +846,166 @@ pub async fn reconcile(replica: Arc<PostgresPhysicalReplica>, ctx: Arc<Context>)
844846
///
845847
/// Returns `Ok(true)` when migration is complete (or skipped), `Ok(false)` when
846848
/// in progress, or `Err` on permanent failure.
849+
/// Returns true if a tokio_postgres error is an authentication failure.
850+
/// We detect this by inspecting the SQLSTATE code (28P01 = invalid_password,
851+
/// 28000 = invalid_authorization_specification) and the error message text as
852+
/// a fallback for cases where the code is not surfaced.
853+
fn is_auth_error(e: &Error) -> bool {
854+
let Error::Postgres(pg_err) = e else {
855+
return false;
856+
};
857+
if let Some(db_err) = pg_err.as_db_error() {
858+
let code = db_err.code();
859+
return code == &tokio_postgres::error::SqlState::INVALID_PASSWORD
860+
|| code == &tokio_postgres::error::SqlState::INVALID_AUTHORIZATION_SPECIFICATION;
861+
}
862+
// Fallback: match the error message text that Postgres emits for auth failures.
863+
let msg = pg_err.to_string().to_lowercase();
864+
msg.contains("password authentication failed") || msg.contains("no password assigned")
865+
}
866+
867+
/// Ensure the credential-reset Job exists and has run for the given restore.
868+
///
869+
/// The sequence is:
870+
/// 1. Scale the restore deployment to 0 (so the PVC is free).
871+
/// 2. Create a Job that uses `postgres --single` to ALTER ROLE.
872+
/// 3. While the job is running, return `Ok(false)` (caller retries).
873+
/// 4. When the job succeeds, scale the deployment back to 1 and return
874+
/// `Ok(true)` so the caller re-attempts the connection on the next loop.
875+
/// 5. If the job fails, log a warning and return `Ok(false)` — it will be
876+
/// retried on the next reconcile once the job has been deleted.
877+
///
878+
/// Idempotent: safe to call repeatedly while the job is in flight.
879+
async fn ensure_credential_reset(
880+
client: &Client,
881+
namespace: &str,
882+
restore: &PostgresPhysicalRestore,
883+
replica: &PostgresPhysicalReplica,
884+
) -> Result<bool> {
885+
let restore_name = restore.name_any();
886+
let job_name = credential_reset_job_name(&restore_name);
887+
let jobs: Api<Job> = Api::namespaced(client.clone(), namespace);
888+
let deployments: Api<Deployment> = Api::namespaced(client.clone(), namespace);
889+
890+
// Check for an existing reset job first.
891+
if let Some(job) = jobs.get_opt(&job_name).await? {
892+
match classify_job(&job) {
893+
JobStatus::Active => {
894+
// Still running — nothing to do yet.
895+
return Ok(false);
896+
}
897+
JobStatus::Succeeded => {
898+
info!(
899+
restore = %restore_name,
900+
job = %job_name,
901+
"credential reset job succeeded, scaling deployment back up"
902+
);
903+
904+
// Scale the deployment back to 1.
905+
let scale_patch = serde_json::json!({
906+
"spec": { "replicas": 1 }
907+
});
908+
if let Err(e) = deployments
909+
.patch(
910+
&restore_name,
911+
&PatchParams::apply("postgres-restore-operator"),
912+
&Patch::Merge(&scale_patch),
913+
)
914+
.await
915+
{
916+
warn!(restore = %restore_name, error = %e, "failed to scale deployment back up after credential reset");
917+
}
918+
919+
// Delete the completed job.
920+
if let Err(e) = jobs.delete(&job_name, &Default::default()).await {
921+
warn!(job = %job_name, error = %e, "failed to delete completed credential reset job");
922+
}
923+
924+
// Return true: credentials are fixed, caller should retry connection.
925+
return Ok(true);
926+
}
927+
JobStatus::Failed => {
928+
warn!(
929+
restore = %restore_name,
930+
job = %job_name,
931+
"credential reset job failed; will retry on next reconcile"
932+
);
933+
934+
// Delete the failed job so we create a fresh one next time.
935+
if let Err(e) = jobs.delete(&job_name, &Default::default()).await {
936+
warn!(job = %job_name, error = %e, "failed to delete failed credential reset job");
937+
}
938+
939+
// Scale back up so the restore remains accessible in the meantime.
940+
let scale_patch = serde_json::json!({
941+
"spec": { "replicas": 1 }
942+
});
943+
if let Err(e) = deployments
944+
.patch(
945+
&restore_name,
946+
&PatchParams::apply("postgres-restore-operator"),
947+
&Patch::Merge(&scale_patch),
948+
)
949+
.await
950+
{
951+
warn!(restore = %restore_name, error = %e, "failed to scale deployment back up after failed credential reset");
952+
}
953+
954+
return Ok(false);
955+
}
956+
}
957+
}
958+
959+
// No job exists yet. Scale down the deployment first so the PVC is free.
960+
info!(
961+
restore = %restore_name,
962+
"auth failure detected; scaling deployment to 0 for credential reset"
963+
);
964+
let scale_patch = serde_json::json!({
965+
"spec": { "replicas": 0 }
966+
});
967+
deployments
968+
.patch(
969+
&restore_name,
970+
&PatchParams::apply("postgres-restore-operator"),
971+
&Patch::Merge(&scale_patch),
972+
)
973+
.await?;
974+
975+
// Wait for the pod to terminate so the PVC is released before we mount it
976+
// in the job. We check once; if it's not gone yet we'll be called again on
977+
// the next reconcile and will skip straight to the job-exists branch above.
978+
let pods: Api<Pod> = Api::namespaced(client.clone(), namespace);
979+
let label = format!("pgro.bes.au/restore={restore_name}");
980+
let running_pods = pods
981+
.list(&kube::api::ListParams::default().labels(&label).limit(2))
982+
.await?;
983+
let any_running = running_pods.items.iter().any(|p| {
984+
p.status
985+
.as_ref()
986+
.and_then(|s| s.phase.as_deref())
987+
.is_some_and(|ph| ph == "Running" || ph == "Pending")
988+
});
989+
if any_running {
990+
info!(
991+
restore = %restore_name,
992+
"waiting for pod to terminate before creating credential reset job"
993+
);
994+
return Ok(false);
995+
}
996+
997+
// Pod is gone — create the reset job.
998+
info!(
999+
restore = %restore_name,
1000+
job = %job_name,
1001+
"creating credential reset job"
1002+
);
1003+
let job = build_credential_reset_job(restore, replica, &job_name, namespace)?;
1004+
jobs.create(&PostParams::default(), &job).await?;
1005+
1006+
Ok(false)
1007+
}
1008+
8471009
async fn reconcile_schema_migration(
8481010
client: &Client,
8491011
ctx: &Arc<Context>,
@@ -987,15 +1149,35 @@ async fn reconcile_schema_migration(
9871149
let reader_user = postgres::read_secret_field(&reader_secret, "username")?;
9881150
let reader_password = postgres::read_secret_field(&reader_secret, "password")?;
9891151

990-
let source_dbname = postgres::discover_restore_database(
1152+
let source_dbname = match postgres::discover_restore_database(
9911153
client,
9921154
namespace,
9931155
&old_restore_name,
9941156
&reader_user,
9951157
&reader_password,
9961158
ctx.use_port_forward(),
9971159
)
998-
.await?;
1160+
.await
1161+
{
1162+
Ok(db) => db,
1163+
Err(e) if is_auth_error(&e) => {
1164+
warn!(
1165+
replica = %replica_name,
1166+
restore = %old_restore_name,
1167+
error = %e,
1168+
"auth failure connecting to active restore; triggering credential reset"
1169+
);
1170+
if ensure_credential_reset(client, namespace, old_restore, replica).await? {
1171+
info!(
1172+
replica = %replica_name,
1173+
restore = %old_restore_name,
1174+
"credential reset complete, retrying on next reconcile"
1175+
);
1176+
}
1177+
return Ok(false);
1178+
}
1179+
Err(e) => return Err(e),
1180+
};
9991181

10001182
// Filter out schemas that don't exist on the source. This happens when the
10011183
// user adds a schema to persistent_schemas before actually creating it.
@@ -1081,15 +1263,35 @@ async fn reconcile_schema_migration(
10811263
)
10821264
.await?;
10831265

1084-
let target_dbname = postgres::discover_restore_database(
1266+
let target_dbname = match postgres::discover_restore_database(
10851267
client,
10861268
namespace,
10871269
&new_restore_name,
10881270
&reader_user,
10891271
&reader_password,
10901272
ctx.use_port_forward(),
10911273
)
1092-
.await?;
1274+
.await
1275+
{
1276+
Ok(db) => db,
1277+
Err(e) if is_auth_error(&e) => {
1278+
warn!(
1279+
replica = %replica_name,
1280+
restore = %new_restore_name,
1281+
error = %e,
1282+
"auth failure connecting to switching restore; triggering credential reset"
1283+
);
1284+
if ensure_credential_reset(client, namespace, new_restore, replica).await? {
1285+
info!(
1286+
replica = %replica_name,
1287+
restore = %new_restore_name,
1288+
"credential reset complete, retrying on next reconcile"
1289+
);
1290+
}
1291+
return Ok(false);
1292+
}
1293+
Err(e) => return Err(e),
1294+
};
10931295

10941296
// Check that none of the persistent schemas already exist in the snapshot.
10951297
// If they do, the pg_dump|psql migration would conflict, so we must fail

src/controllers/restore.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ use crate::{
2828

2929
mod builders;
3030

31+
pub(crate) use builders::{build_credential_reset_job, credential_reset_job_name};
32+
3133
#[cfg(test)]
3234
mod tests;
3335

0 commit comments

Comments
 (0)