Skip to content

Commit f2ce434

Browse files
committed
fix(supervisor): deduplicate rejected stream snapshots
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
1 parent c1101ec commit f2ce434

1 file changed

Lines changed: 153 additions & 13 deletions

File tree

  • crates/openshell-supervisor/src

crates/openshell-supervisor/src/lib.rs

Lines changed: 153 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4052,11 +4052,38 @@ struct PendingStreamProvider {
40524052
expires_at_ms: Option<i64>,
40534053
}
40544054

4055+
#[derive(Debug)]
4056+
struct RejectedStreamSandbox {
4057+
configuration_instance_id: String,
4058+
requested_revision: openshell_core::proto::ConfigSnapshotRevision,
4059+
environment: EnvironmentIdentity,
4060+
error: String,
4061+
failure_mode: PolicyValidationFailureMode,
4062+
result: openshell_core::proto::ConfigComponentApplyResult,
4063+
}
4064+
4065+
impl RejectedStreamSandbox {
4066+
fn matches(
4067+
&self,
4068+
snapshot: &openshell_core::grpc_client::SettingsPollResult,
4069+
requested_revision: &openshell_core::proto::ConfigSnapshotRevision,
4070+
environment: &EnvironmentIdentity,
4071+
error: &str,
4072+
) -> bool {
4073+
self.configuration_instance_id == snapshot.configuration_instance_id
4074+
&& self.requested_revision == *requested_revision
4075+
&& self.environment == *environment
4076+
&& self.error == error
4077+
&& self.failure_mode == snapshot.policy_validation_failure_mode
4078+
}
4079+
}
4080+
40554081
#[derive(Debug)]
40564082
struct StreamConfigurationState {
40574083
active_environment: EnvironmentIdentity,
40584084
pending_provider: Option<PendingStreamProvider>,
40594085
pending_sandbox: Option<Box<openshell_core::grpc_client::SettingsPollResult>>,
4086+
rejected_sandbox: Option<Box<RejectedStreamSandbox>>,
40604087
}
40614088

40624089
impl StreamConfigurationState {
@@ -4068,6 +4095,7 @@ impl StreamConfigurationState {
40684095
.unwrap_or_default(),
40694096
pending_provider: None,
40704097
pending_sandbox: None,
4098+
rejected_sandbox: None,
40714099
}
40724100
}
40734101
}
@@ -4369,6 +4397,7 @@ async fn apply_stream_sandbox_snapshot<C: PolicyGatewayClient>(
43694397
&& current_stream_revision.as_ref() == Some(&requested_revision)
43704398
&& desired_environment == stream_state.active_environment
43714399
{
4400+
stream_state.rejected_sandbox = None;
43724401
return config_apply_result(
43734402
ConfigComponent::SandboxConfig,
43744403
requested_revision.clone(),
@@ -4394,7 +4423,12 @@ async fn apply_stream_sandbox_snapshot<C: PolicyGatewayClient>(
43944423
} else {
43954424
&snapshot.configuration_error
43964425
};
4397-
return match apply_policy_validation_failure(
4426+
if let Some(rejected) = stream_state.rejected_sandbox.as_ref()
4427+
&& rejected.matches(&snapshot, &requested_revision, &desired_environment, error)
4428+
{
4429+
return rejected.result.clone();
4430+
}
4431+
let (result, cacheable) = match apply_policy_validation_failure(
43984432
&ctx.opa_engine,
43994433
snapshot.policy_validation_failure_mode,
44004434
*has_last_valid_policy,
@@ -4417,22 +4451,39 @@ async fn apply_stream_sandbox_snapshot<C: PolicyGatewayClient>(
44174451
} else {
44184452
ConfigApplyOutcome::FailedClosed
44194453
};
4420-
config_apply_result(
4421-
ConfigComponent::SandboxConfig,
4422-
requested_revision,
4423-
applied_revision,
4424-
outcome,
4425-
Some(("configuration_rejected", error.to_string(), true)),
4454+
(
4455+
config_apply_result(
4456+
ConfigComponent::SandboxConfig,
4457+
requested_revision.clone(),
4458+
applied_revision,
4459+
outcome,
4460+
Some(("configuration_rejected", error.to_string(), true)),
4461+
),
4462+
true,
44264463
)
44274464
}
4428-
Err(failure) => config_apply_result(
4429-
ConfigComponent::SandboxConfig,
4430-
requested_revision,
4431-
None,
4432-
ConfigApplyOutcome::FailedClosed,
4433-
Some(("configuration_rejected", failure.to_string(), true)),
4465+
Err(failure) => (
4466+
config_apply_result(
4467+
ConfigComponent::SandboxConfig,
4468+
requested_revision.clone(),
4469+
None,
4470+
ConfigApplyOutcome::FailedClosed,
4471+
Some(("configuration_rejected", failure.to_string(), true)),
4472+
),
4473+
false,
44344474
),
44354475
};
4476+
if cacheable {
4477+
stream_state.rejected_sandbox = Some(Box::new(RejectedStreamSandbox {
4478+
configuration_instance_id: snapshot.configuration_instance_id,
4479+
requested_revision,
4480+
environment: desired_environment,
4481+
error: error.to_string(),
4482+
failure_mode: snapshot.policy_validation_failure_mode,
4483+
result: result.clone(),
4484+
}));
4485+
}
4486+
return result;
44364487
}
44374488

44384489
if desired_environment != stream_state.active_environment
@@ -4638,6 +4689,7 @@ async fn apply_stream_sandbox_snapshot<C: PolicyGatewayClient>(
46384689

46394690
match outcome {
46404691
Ok(outcome) => {
4692+
stream_state.rejected_sandbox = None;
46414693
if let Ok(generation) = ctx
46424694
.opa_engine
46434695
.generation_guard(ctx.opa_engine.current_generation())
@@ -7982,6 +8034,94 @@ network_policies:
79828034
assert_eq!(current_settings, initial.settings);
79838035
}
79848036

8037+
#[tokio::test]
8038+
async fn duplicate_rejected_stream_snapshot_does_not_advance_fail_closed_generation() {
8039+
use openshell_core::proto::{ConfigApplyOutcome, PolicySource};
8040+
8041+
let mut rejected =
8042+
settings_poll_result(Some(proto_policy_fixture()), 2, PolicySource::Sandbox);
8043+
rejected.config_revision = 200;
8044+
rejected.settings_revision = 2;
8045+
rejected.configuration_admitted = false;
8046+
rejected.configuration_error = "invalid policy".to_string();
8047+
rejected.policy_validation_failure_mode = PolicyValidationFailureMode::FailClosed;
8048+
8049+
let engine =
8050+
Arc::new(OpaEngine::from_proto(&proto_policy_fixture()).expect("build OPA engine"));
8051+
let initial_generation = engine.current_generation();
8052+
let ctx = policy_poll_test_context(
8053+
engine,
8054+
LoadedPolicyOrigin::Gateway {
8055+
revision: None,
8056+
has_last_valid_policy: false,
8057+
},
8058+
default_middleware_connector(),
8059+
);
8060+
let (client, _polls, _reports) = scripted_policy_gateway();
8061+
let mut config_revision = 0;
8062+
let mut stream_revision = None;
8063+
let mut policy_version = 0;
8064+
let mut policy_hash = String::new();
8065+
let mut endpoint_policy = None;
8066+
let mut middleware_services = Vec::new();
8067+
let mut extension_authentication_enabled = false;
8068+
let mut middleware_registry_status = MiddlewareRegistryStatus::Synchronized;
8069+
let mut current_settings = std::collections::HashMap::new();
8070+
let mut stream_state = StreamConfigurationState::new(None);
8071+
let mut provider_revision = 0;
8072+
let mut has_last_valid_policy = false;
8073+
8074+
let result = apply_stream_sandbox_snapshot(
8075+
&ctx,
8076+
&client,
8077+
rejected.clone(),
8078+
&mut config_revision,
8079+
&mut stream_revision,
8080+
&mut policy_version,
8081+
&mut policy_hash,
8082+
&mut endpoint_policy,
8083+
&mut middleware_services,
8084+
&mut extension_authentication_enabled,
8085+
&mut middleware_registry_status,
8086+
&mut current_settings,
8087+
&mut stream_state,
8088+
&mut provider_revision,
8089+
true,
8090+
&mut has_last_valid_policy,
8091+
)
8092+
.await;
8093+
8094+
assert_eq!(
8095+
ConfigApplyOutcome::try_from(result.outcome).unwrap(),
8096+
ConfigApplyOutcome::FailedClosed
8097+
);
8098+
let rejected_generation = ctx.opa_engine.current_generation();
8099+
assert!(rejected_generation > initial_generation);
8100+
8101+
let duplicate = apply_stream_sandbox_snapshot(
8102+
&ctx,
8103+
&client,
8104+
rejected,
8105+
&mut config_revision,
8106+
&mut stream_revision,
8107+
&mut policy_version,
8108+
&mut policy_hash,
8109+
&mut endpoint_policy,
8110+
&mut middleware_services,
8111+
&mut extension_authentication_enabled,
8112+
&mut middleware_registry_status,
8113+
&mut current_settings,
8114+
&mut stream_state,
8115+
&mut provider_revision,
8116+
true,
8117+
&mut has_last_valid_policy,
8118+
)
8119+
.await;
8120+
8121+
assert_eq!(duplicate, result);
8122+
assert_eq!(ctx.opa_engine.current_generation(), rejected_generation);
8123+
}
8124+
79858125
#[tokio::test]
79868126
async fn revision_two_stream_never_polls_gateway_settings() {
79878127
let initial = settings_poll_result(

0 commit comments

Comments
 (0)