Change Audit models ACLs on Flaw unembargo #857
Conversation
superbuggy
force-pushed
the
feature/OSIDB-3463-reset-flaw-audit-acls-on-unembargo
branch
2 times, most recently
from
82bf250 to
1cfd63a
Compare
| db, model_name = ref.pgh_model.split(".") | ||
| model_audit = apps.get_model(db, model_name).objects.filter( |
There was a problem hiding this comment.
There's no need to split the name, you can pass it directly
| db, model_name = ref.pgh_model.split(".") | |
| model_audit = apps.get_model(db, model_name).objects.filter( | |
| model_audit = apps.get_model(ref.pgh_model).objects.filter( |
osidb/mixins.py
Outdated
| "osidb.FlawCommentAudit:append_only", | ||
| "osidb.FlawCVSSAudit:append_only", | ||
| "osidb.FlawReferenceAudit:append_only", | ||
| ): |
There was a problem hiding this comment.
I do not see the listing of all the subclasses explicitly as a good approach as when we introduce a new one we can easily forget to extend the list here. We should have a programmatic solution to get the list. Also I am not sure that handling Flaw and all of its subclasses here is actually a good idea. Note that the unebmargo method is being called recursively diving deeper into the class relationships. Eg. it first deal with a flaw and calls itself to deal with an affect. I think that the audit changes should work in the same way. This way it seems to me that we deal with everything during every single step of the recursion so basically repeating it all over and over.
There was a problem hiding this comment.
Ah thank you, I can just use the model name here to create the pgtrigger URI. Thank you, just pushed a correction. Will fold that commit into the first commit if the change looks good to you.
| flaw["acls_read"] == settings.PUBLIC_READ_GROUPS | ||
| and flaw["acls_write"] == [settings.PUBLIC_WRITE_GROUP] | ||
| for flaw in Flaw.objects.all() | ||
| ) |
There was a problem hiding this comment.
I am thinking why this is needed above the assert on line 164. Condition on line 165 is exactly the same as here on 180 but here we only check Flaw model while the the other assert checks other models too. If the reason is to also check the writhe group, I would be for extending the previous assert and not introducing a new partially duplicate but less covering one.
There was a problem hiding this comment.
Ah okay, I will just extend the previous assertion to check read and write groups then.
There was a problem hiding this comment.
I was not testing the correct models, and I have fixed now this. though I didnt add the check for the write groups to the original tests since it's outside the scope of this feature.
osidb/mixins.py
Outdated
| @@ -1,9 +1,9 @@ | |||
| import pghistory | |||
There was a problem hiding this comment.
I do not understand this commit in general. It partially seems to fix the previous one but we have fixup for that so there is no reason for a new commit. Then it introduces an import which is not used in the commit. That should be introduced together with its usage so if someone looks at the commit history it is clear why we introduce it.
|
|
||
| ```sh | ||
| $ podman logs -f osidb-service | ||
| ``` |
There was a problem hiding this comment.
I am not strongly against this but I wonder whether documenting Podman is not beyond OSIDB documentation. There are certainly tons of potentially useful Podman commands but it has its own documentation. If it was some specific OSIDB procedure, it was non-intuitive, etc. then probably let us document it here.
| @@ -449,5 +449,5 @@ | |||
| } | |||
| ] | |||
| }, | |||
| "generated_at": "2024-11-27T14:06:13Z" | |||
| "generated_at": "2024-12-11T14:29:26Z" | |||
There was a problem hiding this comment.
(opinionated) nitpick: It would make more sense to me to make the secrets corrections together with the changes they caused them so it is clear why we do them. It may be tedious when there are many commits (which is not the case here) and I sure there are different approaches to this in our team so no strong preference 😸
Make linting corrections Update secrets Make more linting corrections
superbuggy
force-pushed
the
feature/OSIDB-3463-reset-flaw-audit-acls-on-unembargo
branch
from
8672b33 to
1c3bc36
Compare
Closes OSIDB-3463