api: update cache for vulnerabilities #3318

lucasmoura · 2024-09-20T17:40:57Z

Why is this needed?

We are now caching the vulnerability result as well. That means that if we detect that there is now new vulnerability JSON data to be used and no dpkg related changes on the system, we can simply reuse the old results.

Test Steps

Verify that the vulnerabilities integration tests are still working
Verify that the result cache is being produced
Verify when running two API commands in a row, the second one is way faster than the first

(un)check this to re-run the checklist action

github-actions · 2024-09-20T17:41:13Z

PR Checklist

How to use this checklist

PR Author

For each section, check a box when it is true.
Uncheck a box if it becomes un-true.
Then check the box at the bottom of the PR description to re-run the action that creates this checklist.
The action that creates and updates this comment will retain your edits.
The action will fail if the checklist is not completed.

PR Reviewer

Check that the PR checklist action did not fail.
Double check that the author filled out the checklist accurately.
If you disagree with a checklist item, start a conversation.
For example, the author may say they don't think integration tests are necessary, but you may disagree.

Bug References

None.

Confirm

I've properly referenced all bugs that this PR fixes

How to properly reference fixed bugs

If this PR is related to a Jira item, include an SC-1234 reference in the PR title
If this PR is fixes a GitHub issue, include a Fixes: #1234 reference in the commit that fixes the issue
If this PR is fixes a Launchpad bug, include a LP: #12345678 reference in the commit that fixes the issue

Test Updates

Unit Tests

I have updated or added any unit tests accordingly
No unit test changes are necessary for this change

Integration Tests

I have updated or added any integration tests accordingly
No integration test changes are necessary for this change

Documentation

Changes here need to be documented and I have referenced the docs PR in the description
No documentation updates are necessary for this change

Does this PR require review from someone outside the core ubuntu-pro-client team?

Yes, and I have requested those reviews via GitHub
No

renanrodrigo · 2024-09-23T17:56:35Z

uaclient/api/u/pro/security/vulnerabilities/usn/v1.py

    ):
        self.all = all
        self.unfixable = unfixable
        self.data_file = data_file
        self.manifest_file = manifest_file
        self.series = series

+        if update is None:


As this is a boolean you can set it to True directly on line 80 there and not make it optional, but this as is doesn't hurt either

renanrodrigo · 2024-09-23T18:11:59Z

uaclient/api/u/pro/security/vulnerabilities/_common/v1.py

@@ -50,25 +64,28 @@ def __init__(
        cfg: UAConfig,
        data_file: Optional[str] = None,
        series: Optional[str] = None,
+        update_data: bool = True,


heh you did it here

renanrodrigo · 2024-09-23T18:45:13Z

uaclient/api/u/pro/security/vulnerabilities/_common/v1.py

        last_published_date: str,
        cache_date_file: DataObjectFile,
-    ) -> bool:
+    ) -> Tuple[bool, Optional[datetime.datetime]]:


I would suggest some comment here explaining what is this datetime's meaning

lucasmoura · 2024-09-24T19:31:59Z

@renanrodrigo updated

orndorffgrant

As part of our process to get ready for v35, I have rebased next-v35 onto main. As a result, next-v35 is now obsolete. Please rebase this on main and target the PR to main.

orndorffgrant

As discussed, we should document the architecture of these caching mechanisms, when they are used, and when they are invalidated in some kind of dev-docs explanation.

orndorffgrant · 2024-09-25T17:48:11Z

uaclient/api/u/pro/security/vulnerabilities/_common/v1.py

+    def _has_apt_state_changed(self):
+        latest_dpkg_status_time = apt.get_dpkg_status_time() or 0
+
+        dpkg_status_cache = DataObjectFile(


I think this can only be defined once as an instance variable and used here and above

orndorffgrant · 2024-09-25T17:48:41Z

uaclient/api/u/pro/security/vulnerabilities/_common/v1.py

+        )
+
+
+class VulnerabilityResultCache:


We discussed not using or saving this cache when run against a manifest

Also if someone provides a series without a manifest it shouldn't work

lucasmoura · 2024-09-25T21:08:29Z

Since we are now adding two more caches, I have created this diagram to simplify the visualization on how those caches are being used:

We are now caching the vulnerability result as well. That means that if we detect that there is now new vulnerability JSON data to be used and no dpkg related changes on the system, we can simply reuse the old results.

lucasmoura · 2024-09-25T21:54:52Z

@orndorffgrant updated

dheyay

Looks good, thanks also for the diagram (helped understanding everything better) 😄

lucasmoura mentioned this pull request Sep 20, 2024

cli: add vulnerability commands #3320

Merged

1 task

renanrodrigo approved these changes Sep 23, 2024

View reviewed changes

lucasmoura force-pushed the update-vulnerability-cache branch from 177d696 to 2dd75d7 Compare September 24, 2024 19:31

orndorffgrant requested changes Sep 24, 2024

View reviewed changes

orndorffgrant reviewed Sep 25, 2024

View reviewed changes

orndorffgrant added the next label Sep 25, 2024

api: update cache for vulnerabilities

19c9268

We are now caching the vulnerability result as well. That means that if we detect that there is now new vulnerability JSON data to be used and no dpkg related changes on the system, we can simply reuse the old results.

lucasmoura force-pushed the update-vulnerability-cache branch from 2dd75d7 to 19c9268 Compare September 25, 2024 21:54

lucasmoura changed the base branch from next-v35 to main September 25, 2024 21:54

dheyay approved these changes Sep 30, 2024

View reviewed changes

orndorffgrant approved these changes Oct 4, 2024

View reviewed changes

lucasmoura enabled auto-merge (rebase) October 4, 2024 19:18

lucasmoura merged commit 97b7777 into main Oct 4, 2024
6 of 7 checks passed

renanrodrigo deleted the update-vulnerability-cache branch October 4, 2024 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: update cache for vulnerabilities #3318

api: update cache for vulnerabilities #3318

lucasmoura commented Sep 20, 2024 •

edited

Loading

github-actions bot commented Sep 20, 2024 •

edited by lucasmoura

Loading

How to use this checklist

PR Author

PR Reviewer

renanrodrigo Sep 23, 2024

renanrodrigo Sep 23, 2024

renanrodrigo Sep 23, 2024

lucasmoura commented Sep 24, 2024

orndorffgrant left a comment

orndorffgrant left a comment

orndorffgrant Sep 25, 2024

orndorffgrant Sep 25, 2024

orndorffgrant Sep 25, 2024

lucasmoura commented Sep 25, 2024

lucasmoura commented Sep 25, 2024

dheyay left a comment

		)


		class VulnerabilityResultCache:

api: update cache for vulnerabilities #3318

api: update cache for vulnerabilities #3318

Conversation

lucasmoura commented Sep 20, 2024 • edited Loading

Why is this needed?

Test Steps

github-actions bot commented Sep 20, 2024 • edited by lucasmoura Loading

PR Checklist

How to use this checklist

PR Author

PR Reviewer

Bug References

Confirm

Test Updates

Unit Tests

Integration Tests

Documentation

Does this PR require review from someone outside the core ubuntu-pro-client team?

renanrodrigo Sep 23, 2024

Choose a reason for hiding this comment

renanrodrigo Sep 23, 2024

Choose a reason for hiding this comment

renanrodrigo Sep 23, 2024

Choose a reason for hiding this comment

lucasmoura commented Sep 24, 2024

orndorffgrant left a comment

Choose a reason for hiding this comment

orndorffgrant left a comment

Choose a reason for hiding this comment

orndorffgrant Sep 25, 2024

Choose a reason for hiding this comment

orndorffgrant Sep 25, 2024

Choose a reason for hiding this comment

orndorffgrant Sep 25, 2024

Choose a reason for hiding this comment

lucasmoura commented Sep 25, 2024

lucasmoura commented Sep 25, 2024

dheyay left a comment

Choose a reason for hiding this comment

lucasmoura commented Sep 20, 2024 •

edited

Loading

github-actions bot commented Sep 20, 2024 •

edited by lucasmoura

Loading