Skip to content

Commit 1806c9b

Browse files
varshaprasad96sjenning
authored andcommitted
fix(credentials): fix retry loop guard and remove unprotected validation
Remove attempt-count guard from 409/Aborted match arms in retry loops so the post-loop Status::aborted error is reachable after exhausting retries. Previously, last-attempt conflicts fell through to the catch-all error arm, producing misleading Status::unavailable errors. Remove duplicate validation calls that ran after prepare_provider_credential_update but outside the cleanup-protected async block, which would leak pre-stored handles on failure. Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
1 parent 8c39f41 commit 1806c9b

3 files changed

Lines changed: 6 additions & 15 deletions

File tree

  • crates
    • openshell-driver-db-credstore/src
    • openshell-driver-kubernetes-secrets/src
    • openshell-server/src/grpc

crates/openshell-driver-db-credstore/src/lib.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ impl DbCredstoreCredentialDriver {
235235
&request.credential_key,
236236
)?;
237237

238-
for attempt in 0..CONFLICT_RETRY_LIMIT {
238+
for _attempt in 0..CONFLICT_RETRY_LIMIT {
239239
let record = self
240240
.store
241241
.get_credential_object(OBJECT_TYPE, &id, "load credential for deletion")
@@ -263,9 +263,7 @@ impl DbCredstoreCredentialDriver {
263263
.await
264264
{
265265
Ok(()) => return Ok(()),
266-
Err(err)
267-
if err.code() == tonic::Code::Aborted && attempt + 1 < CONFLICT_RETRY_LIMIT => {
268-
}
266+
Err(err) if err.code() == tonic::Code::Aborted => {}
269267
Err(err) => return Err(err),
270268
}
271269
}

crates/openshell-driver-kubernetes-secrets/src/lib.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ impl KubernetesSecretsCredentialDriver {
199199
)?;
200200
let owner_id = credential_owner_id(&request.provider_name, &request.credential_key);
201201
let api: Api<Secret> = Api::namespaced(self.client.clone(), &reference.namespace);
202-
for attempt in 0..CONFLICT_RETRY_LIMIT {
202+
for _attempt in 0..CONFLICT_RETRY_LIMIT {
203203
let secret = match api.get(&reference.secret_name).await {
204204
Ok(secret) => secret,
205205
Err(kube::Error::Api(api_err)) if api_err.code == 404 => return Ok(()),
@@ -222,8 +222,7 @@ impl KubernetesSecretsCredentialDriver {
222222
match api.delete(&reference.secret_name, &delete_params).await {
223223
Ok(_) => return Ok(()),
224224
Err(kube::Error::Api(api_err)) if api_err.code == 404 => return Ok(()),
225-
Err(kube::Error::Api(api_err))
226-
if api_err.code == 409 && attempt + 1 < CONFLICT_RETRY_LIMIT => {}
225+
Err(kube::Error::Api(api_err)) if api_err.code == 409 => {}
227226
Err(kube::Error::Api(api_err)) if api_err.code == 403 => {
228227
return Err(Status::permission_denied(format!(
229228
"gateway is not allowed to delete Kubernetes Secret '{}' in namespace '{}'",
@@ -298,7 +297,7 @@ impl KubernetesSecretsCredentialDriver {
298297
value: &str,
299298
) -> Result<(), Status> {
300299
let api: Api<Secret> = Api::namespaced(self.client.clone(), &reference.namespace);
301-
for attempt in 0..CONFLICT_RETRY_LIMIT {
300+
for _attempt in 0..CONFLICT_RETRY_LIMIT {
302301
let secret = match api.get(&reference.secret_name).await {
303302
Ok(secret) => secret,
304303
Err(kube::Error::Api(api_err)) if api_err.code == 404 => {
@@ -325,8 +324,7 @@ impl KubernetesSecretsCredentialDriver {
325324
.await
326325
{
327326
Ok(_) => return Ok(()),
328-
Err(kube::Error::Api(api_err))
329-
if api_err.code == 409 && attempt + 1 < CONFLICT_RETRY_LIMIT => {}
327+
Err(kube::Error::Api(api_err)) if api_err.code == 409 => {}
330328
Err(err) => {
331329
return Err(kube_write_error_to_status(
332330
&reference.namespace,

crates/openshell-server/src/grpc/provider.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -377,11 +377,6 @@ async fn update_provider_record_validating(
377377
for key in credential_update.deferred_store_values.keys() {
378378
candidate.credentials.remove(key);
379379
}
380-
validate_provider_mutable_fields(&candidate)?;
381-
validate_provider_update_against_attached_sandboxes_with_catalog(
382-
store, catalog, workspace, &candidate,
383-
)
384-
.await?;
385380
if credentials.is_some_and(crate::credentials::CredentialRuntime::stores_provider_credentials) {
386381
for key in updated_credential_values.keys() {
387382
candidate.credentials.remove(key);

0 commit comments

Comments
 (0)