Skip to content

Conversation

@smklein
Copy link
Collaborator

@smklein smklein commented Jan 30, 2026

Fixes #9747

login_local (post "/v1/login/{silo_name}/local")
logout (post "/v1/logout")
rack_membership_add_sleds (post "/v1/system/hardware/racks/{rack_id}/membership/add")
networking_switch_port_lldp_config_update (post "/v1/system/hardware/switch-port/{port}/lldp/config")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct but now that these endpoints aren't exempted here, they need audit logging. Which makes me realize the audit log test must only be checking endpoints that not exempted here? Wtf? Looking into that.

Image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. The audit log coverage test does use the schema for getting operation IDs, but for the actual looping through the endpoints, it doesn't. Let me fix it on this branch and I'll fix the endpoints missing audit logging too.

// Build a map from (method, url_regex) to (operation_id, path_template)
let spec_operations: BTreeMap<(String, String), (String, String)> = spec
.operations()
.map(|(path, method, op)| {
// Convert path template to regex pattern
let re = regex::Regex::new("/\\{[^}]+\\}").unwrap();
let regex_path = re.replace_all(path, "/[^/]+");
let regex = format!("^{}$", regex_path);
let label = op
.operation_id
.clone()
.unwrap_or_else(|| String::from("unknown"));
((method.to_uppercase(), regex), (label, path.to_string()))
})
.collect();
// Set up resources needed by many endpoints
DiskTest::new(&ctx).await;
create_default_ip_pools(client).await;
let _project = create_project(client, "demo-project").await;
let t_start = Utc::now();
let mut missing_audit: BTreeMap<String, (String, String)> = BTreeMap::new();
let mut unexpected_get_audit: BTreeMap<String, (String, String)> =
BTreeMap::new();
for endpoint in &*VERIFY_ENDPOINTS {

Copy link
Contributor

@david-crespo david-crespo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are bigger changes to the audit log coverage test I need to make, but after fixing the endpoints I fixed here, I think the only endpoint genuinely missing coverage is logout. Writing up an issue for all that. In the meantime I'm going to merge this so we can get #9671 in.

@david-crespo david-crespo enabled auto-merge (squash) January 31, 2026 14:41
@david-crespo david-crespo merged commit 001cb15 into main Jan 31, 2026
16 checks passed
@david-crespo david-crespo deleted the more-authz branch January 31, 2026 16:16
david-crespo added a commit that referenced this pull request Feb 1, 2026
Depends on #9760
Fixes #9748

---------

Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

many endpoints skipped by authz coverage test that seem like they shouldn't be

3 participants