Skip to content
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

Allow audit errors to be ignored #554

Merged
merged 46 commits into from
Dec 19, 2024
Merged

Allow audit errors to be ignored #554

merged 46 commits into from
Dec 19, 2024

Conversation

amstilp
Copy link
Contributor

@amstilp amstilp commented Dec 18, 2024

  • Move auditing functionality into its own sub-app, anvil_consortium_manager.auditor
  • Add a new IgnoredManagedGroupMembership model that stores records of ignored managed group memberships
  • Add views to interact with the new model (e.g., create, list, detail, delete)
  • Update ManagedGroupMembership audit functionality to ignore "not in app" records that are in the IgnoredManagedGroupMembership table
  • Add reporting of ignored records to management command
  • Add list of ignored audits to managed group detail page? or add filterview to list of ignored records?
  • Update docs

This model tracks any records that should be ignored when reporting
the "not in app" results for a ManagedGroupMembership audit.
Move audit classes for BillingProjects, Accounts, MangedGroups, and
Workspaces into their own source files. Rename the remaining audit.py
file to base.py since it contains the base classes used by the classes
in the other, model-specific source files. Update import statements
and namespaces in other files.
The new IgnoredResult class holds an audit results from one of the
Ignored model tables (right now there is only one). Update the
AnVILAudit class to have an attribute list that stores ignored results,
to be able to add these results, and to export them in json format
using the export method. An audit is still considered "ok" when there
are "ignored" results.
It stores both the record and the model instance that matches that
record.
This way, we can report any ignored reuslts in the audit regardless
of whether that record actually exists on AnVIL.
Show all objects in the IgnoredAuditManagedGroupMembership table,
regardless of whether the email actually is in AnVIL. This is a
good reminder that there are still ignored records that we may want
to clean up even if the email is no longer part of the group.
Also add tests for ManagedGroupMembershipAudit. Note that the
current implementation of the audit table is specific to the
managed group membership; it will need to be generalized for other
audit classes.
Set default table classes in the AnVILAudit class, and add methods
to return tables for the verified, not in app, error, or ignored
results. This lets model-specific classes that subclass the AnVILAudit
class to define and set their own table classes, so they can provide
custom table fields.
Refactor audit urls such that they will have their own top-level
namespace of "audit". Update the urls for billing project audits.
Rework the urls for managed group audit and managed group membership
audit.
Also use the new url to link to the object in the audit report for
managed group membership.
…results

Define a new class that subclasses NotInAppResult specifically for
ManagedGroupMembershipAudit results. This new class also stores the
group, the email, and the role that a not_in_app audit result found.
Also add a new NotInApp table for the ManagedGroupMembership audit that
shows these records, plus a link to the view to create a new
IgnoredAuditManagedGroupMembership record for that group and email.
Also add the link to the detail page for the audit result, and
fix the tests checking the delete/update links.
Auditing code will eventually move into this app.
Since these classes will be moving to the new app, rename them
now for ease - they won't need "Audit" in the name if they're already
in the auditor app.
Move the admin and tests as well. Temporarily update import statements
in the main app until everything else can be moved. Many tests are
still broken.
Add another temporary import in the main app for the form.
The other views for performing audits have not been moved yet.
Do not update urls yet.
Split the tests into one test source file per main source file.
This will need to be implemented, but I need to think about how to
do it. For now, add a failing test as a reminder.
Also add a table to show the memberships.
Report the number of ignored records associated with an audit. To
do this, we need to tell the audit which model to look at; we can't
just use the audit results because sometimes the ignored records are
nested below (e.g., from the ManagedGroupMembershipAudit, which is
run by ManagedGroupAudit, and ManagedGroupAudit does not know about
anything ignored by ManagedGroupMembershipAudit). Perhaps there is
a better way to structure this, but we'll leave that as a future problem
to solve.
Also add a corresponding card for audits, instead of having them
be lumped with the other models.
Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 99.89554% with 3 lines in your changes missing coverage. Please review.

Project coverage is 99.76%. Comparing base (60c649f) to head (13b2bd9).
Report is 53 commits behind head on main.

Files with missing lines Patch % Lines
..._consortium_manager/auditor/tests/test_commands.py 98.55% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #554      +/-   ##
==========================================
+ Coverage   99.74%   99.76%   +0.02%     
==========================================
  Files         135      166      +31     
  Lines       24411    26181    +1770     
==========================================
+ Hits        24349    26120    +1771     
+ Misses         62       61       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@amstilp amstilp merged commit 123b8f4 into main Dec 19, 2024
13 checks passed
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.

1 participant