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

"Traditional" Compliance Events #13

Merged
merged 3 commits into from
Jun 17, 2024
Merged

"Traditional" Compliance Events #13

merged 3 commits into from
Jun 17, 2024

Conversation

JustinKuli
Copy link
Owner

Adds the Kubernetes events that the policy framework uses to track and aggregate compliance, and includes test utilities for them.

GetCondition and something like UpdateCondition have worked well in the
OperatorPolicy. The fakepolicy tests have been updated to use conditions
instead of the various special fields it was collecting in its status.

Signed-off-by: Justin Kulikauskas <[email protected]>
Copy link

Code Coverage Report for this branch (might be slow to update): Go Coverage

Copy link

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5] 4
🏅 Score 85
🧪 Relevant tests Yes
🔒 Security concerns No
⚡ Key issues to review Possible Bug:
The UpdateCondition method in PolicyCoreStatus does not handle the case where the condition exists but has not semantically changed. This could lead to unnecessary updates and event emissions.
Code Clarity:
The use of embedded structs in PolicyCoreSpec and PolicyCoreStatus could be better documented to explain their intended use and how they interact with the Kubernetes API machinery.

api/v1beta1/conditions.go Show resolved Hide resolved
api/v1beta1/conditions.go Show resolved Hide resolved
api/v1beta1/conditions.go Show resolved Hide resolved
api/v1beta1/conditions.go Show resolved Hide resolved
pkg/testutils/courtesies_test.go Show resolved Hide resolved
test/fakepolicy/test/basic/target_test.go Show resolved Hide resolved
test/fakepolicy/test/basic/target_test.go Show resolved Hide resolved
test/fakepolicy/test/basic/target_test.go Show resolved Hide resolved
test/fakepolicy/test/basic/target_test.go Show resolved Hide resolved
@JustinKuli
Copy link
Owner Author

AI Suggestions Score

Category Count Comment
Great 1 Seriously saved me from an embarrassing typo
Good 4 Some generally good advice, which is not necessary in these specific cases
Bad 7 Many inaccurate (but not harmful) suggestions, some of which were just no-ops
Terrible 3 Some suggestions that would not fundamentally work, might not even compile

The test toolkit and courtesies are things that other policies could use
in their tests. The remaining functions in the utils.go file are more
specifically just for fakepolicy.

In preparation for more tests, the existing tests have been moved into a
separate package/suite. In particular, since some of them require very
specific details in the cluster (exact namespaces, configmaps, and no
extras), this lets them be run in a more isolated way.

Signed-off-by: Justin Kulikauskas <[email protected]>
Many details about the event emitted and functions that verify the form
of the fields are taken from other policy repositories. Many of the
helpful functions added to the toolkit are similarly not original, but
are an evolution of functions used in other repositories' tests.

Signed-off-by: Justin Kulikauskas <[email protected]>
@JustinKuli JustinKuli merged commit d0daa79 into main Jun 17, 2024
5 checks passed
@JustinKuli JustinKuli deleted the comp-event-sending branch June 17, 2024 01:56
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.

1 participant