Add the database schema for the compliance events API #126
Conversation
controllers/complianceeventsapi/complianceeventsapi_controller.go
Outdated
Show resolved
Hide resolved
controllers/complianceeventsapi/complianceeventsapi_controller.go
Outdated
Show resolved
Hide resolved
controllers/complianceeventsapi/complianceeventsapi_controller.go
Outdated
Show resolved
Hide resolved
| secret: | ||
| secretName: postgres-cert | ||
| # Postgres requires limited permissions on the private key. | ||
| defaultMode: 0o440 |
There was a problem hiding this comment.
Is this correct? unusual too me
There was a problem hiding this comment.
I believe I had a YAML lint error when I didn't use the octal notation. This is the equivalent of chmod 440 on the files though.
|
|
||
| CREATE INDEX IF NOT EXISTS idx_policies_spec_hash ON policies (spec_hash); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS compliance_events( |
There was a problem hiding this comment.
I'm wondering about compliance operator which could have a standard, category control for the overall policy, but compliance events may be for more specific categories and controls due to the specific rule failure. Just wondering if we may want to account for a scenario like that.
There was a problem hiding this comment.
It's a good point. The metadata field should be used in this case. I added this in the design document:
open-cluster-management-io/enhancements#101
| policy_id INT NOT NULL, | ||
| compliance TEXT NOT NULL, | ||
| message TEXT NOT NULL, | ||
| timestamp TIMESTAMP NOT NULL, |
There was a problem hiding this comment.
I realize severity could be buried in a spec, but I think it should be a specific field in the table to make sure we have a consistent format that is always present. -- and easily available for reporting
There was a problem hiding this comment.
I'll add that to the policies table so that we don't duplicate it for every compliance event.
| api_group TEXT NOT NULL, | ||
| name TEXT NOT NULL, | ||
| parent_policy_id INT NOT NULL, | ||
| spec TEXT, |
There was a problem hiding this comment.
I think we may want a source or reported_by field so we don't have to study the spec to know what may have reported the compliance info. Plus you could then report on gatekeeper or cert-policy specific compliance history.
There was a problem hiding this comment.
It's a good idea, though the latter query can be covered by querying for compliance events for a policy of kind ConfigurationPolicy but it's not as clear. I'll add this to the compliance_events table.
|
|
||
| CREATE TABLE IF NOT EXISTS clusters( | ||
| id serial PRIMARY KEY, | ||
| name TEXT NOT NULL, |
There was a problem hiding this comment.
It may be in the design and I don't remember, are we using the namespaced name so that's why there's no namespace here?
There was a problem hiding this comment.
I think you meant to post this on the parent_policies table. Yes, we are using the <namespace>.<name> format for the name.
I think it makes sense to have a nullable namespace column on the policies table though to later support integrations with namespaced policies such as Kyverno. This would not be set for our current OCM policies.
| CREATE INDEX IF NOT EXISTS idx_compliance_events_compliance ON compliance_events (compliance); | ||
| CREATE INDEX IF NOT EXISTS idx_compliance_events_timestamp ON compliance_events (timestamp); | ||
|
|
||
| COMMIT; |
There was a problem hiding this comment.
Some other questions:
- how do I tell if a policy was deleted or disabled from the data above?
- can I tell if the policy stayed the same but the source changed?
- should we log the policy remediation so changes there are reflected?
- I expect we could expand to include PolicyAutomation that is triggered in the future -- plus any other integrations we may support in the future...
- I think it's good for now to not focus much on policy placement for this historical data, but I wonder just like with the policy being deleted or disabled if we should integrate in "some" form of placement history. With placement becoming more complex, this can be discussed later.
There was a problem hiding this comment.
- I added this to the design in this Updates to the long-term policy compliance history design enhancements#101. It'll be a separate compliance event from the Spec Sync controller.
- Could you please rephrase this question?
- Perhaps, but at the moment I'm thinking not for PolicyAutomation because we are recording the policy state and if the PolicyAutomation changes this state, the compliance event will be recorded through the lens of the policy.
- My solution for the disabled case is the Spec Sync controller would send an additional compliance event where the
complianceis set todeleted. This does not track the placement but tracks the actual state on the cluster which is better because a cluster could not receive its placement update until much later if the cluster is disconnected from the hub. Having this separate compliance in thecompliance_eventstable can give you a sense of when a policy was deleted and then reapplied.
There was a problem hiding this comment.
I don't remember what I was thinking with the comment:
- can I tell if the policy stayed the same but the source changed?
Regarding remediation:
- should we log the policy remediation so changes there are reflected? I think I'm ok ignoring this comment for now too.
This defines the database schema, integrates with a database migration library, and automatically migrates the database when the governance-policy-database secret is defined in the controller namespace. This feature is disabled by default but can be enabled with the -enable-compliance-events-store flag and by defining the WATCH_NAMESPACE_COMPLIANCE_EVENTS_STORE environment variable. Relates: https://issues.redhat.com/browse/ACM-6848 Signed-off-by: mprahl <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mprahl, yiraeChristineKim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This defines the database schema, integrates with a database migration library, and automatically migrates the database when the governance-policy-database secret is defined in the controller namespace.
This feature is disabled by default but can be enabled with the -enable-compliance-events-store flag and by defining the WATCH_NAMESPACE_COMPLIANCE_EVENTS_STORE environment variable.
Relates:
https://issues.redhat.com/browse/ACM-6848