Add support for custom compliance messages in ConfigurationPolicy #176
Conversation
|
@zyjjay it seems the inheritance isn't taken into account (e.g. if set on policyDefaults but on the policy, policyDefaults is used). Take a look at the |
|
@mprahl Actually I was thinking if the |
There was a problem hiding this comment.
The custom message can also be set at the policyDefaults or policies[] levels, which should propagate down using the applyDefaults() function. And the compliant and noncompliant message overrides should be handled separately. It's more complex but brings greater configurability for users.
I just noticed you can see this (and even reuse some/all of the code logic) for evaluationInterval if you search the repo for that:
https://github.com/search?q=repo%3Aopen-cluster-management-io%2Fpolicy-generator-plugin+evaluationinterval+path%3A%2F%5Einternal%5C%2Fplugin.go%2F&type=code
internal/plugin.go
Outdated
| @@ -1079,6 +1079,10 @@ func (p *Plugin) assertValidConfig() error { | |||
| return fmt.Errorf(errorMsgFmt, "severity") | |||
| } | |||
|
|
|||
| if manifest.CustomMessage != nil { | |||
There was a problem hiding this comment.
This is fine if the messages are the same.
| if manifest.CustomMessage != nil { | |
| if !reflect.DeepEqual(manifest.CustomMessage, policy.CustomMessage) { |
internal/types/types.go
Outdated
| @@ -45,6 +45,7 @@ type ConfigurationPolicyOptions struct { | |||
| PruneObjectBehavior string `json:"pruneObjectBehavior,omitempty" yaml:"pruneObjectBehavior,omitempty"` | |||
| RecordDiff string `json:"recordDiff,omitempty" yaml:"recordDiff,omitempty"` | |||
| RecreateOption string `json:"recreateOption,omitempty" yaml:"recreateOption,omitempty"` | |||
| CustomMessage *CustomMessage `json:"customMessage,omitempty" yaml:"customMessage,omitempty"` | |||
There was a problem hiding this comment.
Similar to EvaluationInterval, I'm not sure there's a benefit to making this a pointer--unless there was a reason to do so?
| CustomMessage *CustomMessage `json:"customMessage,omitempty" yaml:"customMessage,omitempty"` | |
| CustomMessage CustomMessage `json:"customMessage,omitempty" yaml:"customMessage,omitempty"` |
internal/utils.go
Outdated
| var customMessage *types.CustomMessage | ||
| if policyConf.ConsolidateManifests { | ||
| customMessage = policyConf.CustomMessage | ||
| } else { | ||
| customMessage = configPolicyOptionsOverrides.CustomMessage | ||
| } | ||
|
|
There was a problem hiding this comment.
This logic should live in the applyDefaults() function.
internal/utils.go
Outdated
| if customMessage != nil { | ||
| policySpec["customMessage"] = customMessage | ||
| } | ||
|
|
There was a problem hiding this comment.
This is oversimplified. It should look more like the evaluationInterval conditional further down.
I see, it does seem like a lot of code can be reused. I'm wondering if I should just combine some helper functions together to avoid adding functions with minimal differences (i.e., checking the Edit: I went with the latter and kept them separate |
The new `.spec.customMesssage.Compliant` and `spec.customMessage.noncompliant` fields of ConfigurationPolicy are now supported. Note that when consolidateManifests is set to true, the custom message configuration should be set at the policy level and not at the manifest level. ref: https://issues.redhat.com/browse/ACM-13134 Signed-off-by: Jason Zhang <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zyjjay The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Superseded by #178 |
The new
.spec.customMesssage.Compliantandspec.customMessage.noncompliantfields of ConfigurationPolicy are now supported. Note that when consolidateManifests is set to true, the custom message configuration should be set at the policy level and not at the manifest level.ref: https://issues.redhat.com/browse/ACM-13134