search

ComplianceAsCode / auditree-framework

The Auditree framework tool to run compliance control checks as unit tests.
https://auditree.github.io/
Apache License 2.0
59 stars 23 forks source link

_evidence_ttl_expired should extract ttl from metadata #126

Closed Anhui-tqhuang closed 3 years ago

Anhui-tqhuang commented 3 years ago

bug

_evidence_ttl_expired will cause a bug if it gets ttl from evidence and gets last_update from metadata

simualtion

  1. create a evidence with ttl 3 minutes
  2. 4 minutes later, update the code to set evidence with ttl 1 day
  3. _evidence_ttl_expired checks now - last_update and the result is less than 1 day, so the evidence is not updated, but evidence is expired in fact
  4. since the evidence is not updated and expired, checks will fail

proposal changes

in step 3, use now - last_update and compare the result with ttl of existing evidence

drsm79 commented 3 years ago

I would argue the evidence isn't expired - the ttl is now 1 day, so the behaviour is correct (and if you made the change the other way, the evidence would be expired & re-fetched).

Whats the goal? To be able to force a fetch of fresh data?

Anhui-tqhuang commented 3 years ago

but since the evidence is not updated, ttl kept on old evidence is still 3 minutes, which means checks will fail due to the evidence is epxired

alfinkel commented 3 years ago

I suppose your point is that the ttl set in the metadata is still temporarily 3 minutes but the ttl being applied to the evidence is now 1 day based on a code update. This is confusing to you as your expectation is that the ttl in the metadata is the ttl being applied to the evidence but at the moment it isn't? If so, you can consider the ttl in the metadata as the ttl of the evidence when it was last gathered. It is not necessarily the ttl of record as that will be the ttl set in the code. Eventually the ttl in the metadata will catch up with the ttl in the code.

Anhui-tqhuang commented 3 years ago

@alfinkel yep, after one day, the ttl will catch up

also i had a talk with @drsm79 and his point is ttl should not be changed

all make sense to me

one last comment here is: it might be good if fetcher finds the ttl kept in evidence and ttl kept in code are different, fetcher could set the ttl in the evidence to ttl updated in the code

alfinkel commented 3 years ago

Can we close this then?

drsm79 commented 3 years ago

Yeah I think so.

On 25 Jun 2021, at 13:53, Al Finkelstein @.***> wrote:

 Can we close this then?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.