-
Notifications
You must be signed in to change notification settings - Fork 66
[authz] Add coverage for more endpoints #9760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| 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") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
omicron/nexus/tests/integration_tests/audit_log.rs
Lines 418 to 445 in c765b35
| // 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 { |
There was a problem hiding this 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.

Fixes #9747