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

Don't warn of newer tasks if in grace period #1228

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zregvart
Copy link
Member

@zregvart zregvart commented Nov 21, 2024

Adds new rule data task_expiry_warning_days (default 0) if given and greater than 0 then any Task that will expire in task_expiry_warning_days days will emit a warning.

Also adds effective_on of the newest Task to the message of the policy rules asserting that the Tasks used are up to date.

Reference: https://issues.redhat.com/browse/EC-944

@zregvart
Copy link
Member Author

I'm wondering if we want the grace period, there is a scenario when a newer task is available and within a grace period long enough or often enough that the Task that is used is no longer trusted. So by that point no warning would be seen until a blocking violation. Perhaps I misunderstood what EC-944 want's to achieve.

@lcarva
Copy link
Member

lcarva commented Nov 21, 2024

I'm wondering if we want the grace period, there is a scenario when a newer task is available and within a grace period long enough or often enough that the Task that is used is no longer trusted. So by that point no warning would be seen until a blocking violation. Perhaps I misunderstood what EC-944 want's to achieve.

Yeah, it's a risk. I think we just have to make sure there is enough time between the grace period and the time it becomes a violation. We could make sure that that period is still 30 days. WDYT?

Also... Konflux is decreasing the frequency of updates to once a week so I think that increases the need of this PR as well as maybe requiring us to reassess if users still have at least 30 days to update from a visible warning.

@zregvart
Copy link
Member Author

I'm thinking 8d4943c should stay and 58da16f should go

@lcarva
Copy link
Member

lcarva commented Nov 22, 2024

I'm thinking 8d4943c should stay and 58da16f should go

If we have concerns with EC-944 we should probably discuss in the Konflux Architecture call. The goal of that story was to intentionally hide some warnings on the basis that they are noisy and hide more important warnings.

@zregvart
Copy link
Member Author

I'm reworking this based on the expires_on from the trusted task record

Adds effective_on of the newest Task to the message of the policy rules
asserting that the Tasks used are up to date.

Reference: https://issues.redhat.com/browse/EC-944
Adds new rule data `task_expiry_warning_days` (default 0) if given
and greater than 0 then any Task that will expire in
`task_expiry_warning_days` days will emit a warning.

Reference: https://issues.redhat.com/browse/EC-944
@zregvart
Copy link
Member Author

Updated description to reflect the latest implementation

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.

3 participants