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

use correct status type when enforcing policy #594

Merged
merged 1 commit into from
May 23, 2024
Merged

Conversation

maksymvavilov
Copy link
Contributor

@maksymvavilov maksymvavilov commented Apr 30, 2024

Now Enforced will be false even on partially enforced records. If there are no ready records it will still say "partially enforced" (which implies it is enforced on some of them which is not true). This will be addressed in #586
Adding a check to one of the tests to ensure we are not marking policy as Enforced - True when the record is not ready.

Aslo a tinny refactor - moved some vars around for convenience

@maksymvavilov maksymvavilov requested a review from a team as a code owner April 30, 2024 13:23
Copy link

codecov bot commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.60%. Comparing base (ece13e8) to head (e2bbd68).
Report is 97 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #594      +/-   ##
==========================================
+ Coverage   80.20%   82.60%   +2.39%     
==========================================
  Files          64       72       +8     
  Lines        4492     5416     +924     
==========================================
+ Hits         3603     4474     +871     
- Misses        600      634      +34     
- Partials      289      308      +19     
Flag Coverage Δ
integration 73.96% <100.00%> (+2.67%) ⬆️
unit 33.75% <50.00%> (+3.71%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
api/v1beta1 (u) 71.42% <ø> (ø)
api/v1beta2 (u) 93.58% <100.00%> (+2.16%) ⬆️
pkg/common (u) 83.78% <ø> (-5.05%) ⬇️
pkg/istio (u) 75.86% <ø> (+1.94%) ⬆️
pkg/log (u) 94.73% <ø> (ø)
pkg/reconcilers (u) ∅ <ø> (∅)
pkg/rlptools (u) 79.84% <ø> (+0.39%) ⬆️
controllers (i) 80.83% <82.81%> (+4.03%) ⬆️
Files Coverage Δ
controllers/dnspolicy_status.go 84.50% <100.00%> (-1.86%) ⬇️
...library/kuadrant/apimachinery_status_conditions.go 96.49% <100.00%> (+0.65%) ⬆️

... and 29 files with indirect coverage changes

@maksymvavilov maksymvavilov linked an issue May 9, 2024 that may be closed by this pull request
@maksymvavilov maksymvavilov force-pushed the enforced_status branch 3 times, most recently from c7398f4 to a0d35a5 Compare May 16, 2024 13:46
Copy link
Contributor

@KevFan KevFan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks good to me 👍

@maksymvavilov maksymvavilov merged commit ad84ac6 into main May 23, 2024
23 checks passed
@guicassolato
Copy link
Contributor

This is incorrect and should be reverted

guicassolato added a commit that referenced this pull request May 29, 2024
@eguzki eguzki deleted the enforced_status branch June 3, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

DNSPolicy Enforced condition is not updated
4 participants