Partially Enforced sets Enforced status to true

[maksymvavilov]

Redo of the change made earlier about the partially enforced state.
Not the "Partially Enforced" will count as Enforced (will set the Enforced Status to true)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.43%. Comparing base (ece13e8) to head (b53d4d8).
Report is 105 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
+ Coverage   80.20%   82.43%   +2.22%     
==========================================
  Files          64       72       +8     
  Lines        4492     5414     +922     
==========================================
+ Hits         3603     4463     +860     
- Misses        600      639      +39     
- Partials      289      312      +23     

Flag	Coverage Δ
integration	73.91% <100.00%> (+2.63%)	⬆️
unit	33.74% <100.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.60% <83.23%> (+3.79%)	⬆️

Files	Coverage Δ
controllers/dnspolicy_status.go	84.50% <ø> (-1.86%)	⬇️
...library/kuadrant/apimachinery_status_conditions.go	96.36% <100.00%> (+0.53%)	⬆️

... and 31 files with indirect coverage changes

[guicassolato]

@makslion, shall we close #677?

[maksymvavilov]

@guicassolato I wanted to wait until checks have passed before posting it in the Slack discussion. For me, it would make sense to close #677 as we don't really want to revert the change. What was fixed in that PR is still valid - it just revealed another issue. I'm using the old gh issue (#611) just for tracking purposes.
The initial issue was about not using the correct status to check subresources of the policy: we will always assume they are ready. Now we are checking it properly.
What was discussed is related to what we mark policy if not all subresources are ready.

A brief recap of the conversation that lead to this:
Initially, it was decided to have Enforced - False on Partially Enforced as a way to highlight that the policy is not fine; or rather subresources that are affected by the policy are not ok. E.g. DNS Policy targeting GW with 2 listeners. We create a DNS Record per listener and one of the records is not ready due to misconfiguration. It was debated that if we mark Enforced - true it will mislead to the belief that all DNS records are ok.

What was discussed in the Slack thread makes more sense tbh. With this change, we will mark the DNS Policy as not enforced only if we fail to retrieve the list of DNS Records or if there is no single DNS Record controlled by the policy and in a ready state. In all other cases the Enforced will always be true.
The rest of the policies will be fine and will stop being Enforced - false once overridden.

[guicassolato]

Thanks @makslion. I'll close #677 then.

[mikenairn]

From what i understand this was the important bit to revert from https://github.com/Kuadrant/kuadrant-operator/pull/677/files#diff-c62f06c8c31cd4ef9b0a3f839225773a42ea10c75dff00563b42b413ec425107, but we still need this https://github.com/Kuadrant/kuadrant-operator/pull/677/files#diff-06c8a42ee2952e17c9b90724b17324a7dfb178dbea00e606ce3c6eb62b3bd73eR118, so i think this is fine.

[maksymvavilov]

Yes, just the Status is the problem. The rest should stay