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.

Sign up for GitHub

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

Address comments from #276 #278

Merged
merged 3 commits into from
Jul 22, 2024
Merged

Address comments from #276 #278

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f00aef0
Remove unneeded usage of reflect.DeepEqual
mprahl Jul 19, 2024
f186592
Add policy name in log messages in helper functions
mprahl Jul 19, 2024
8579f26
Prevent duplicate calls to templates.HasTemplate
mprahl Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions controllers/configurationpolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,8 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc *policyv1.Conf
// validateConfigPolicy returns an error and increments the "invalid-template" error counter metric
// if the configuration is invalid.
func (r *ConfigurationPolicyReconciler) validateConfigPolicy(plc *policyv1.ConfigurationPolicy) error {
log := log.WithValues("policy", plc.GetName())
Copy link
Member

Choose a reason for hiding this comment

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

These will help; my bad for missing that I took away some context they had.

At some point, thanks to the event-driven changes, we could consider passing the reconcile ctx to (some) of these functions - that has a logger which OperatorPolicy uses, and I feel like it's kinda nice. We might want to figure out if we can customize it if it has too many default key+values.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea @JustinKuli! If you're motivated, feel free to make that PR. I'll gladly review it.


var invalidMessage string

if plc.Spec == nil {
Expand Down Expand Up @@ -1000,6 +1002,8 @@ func (r *ConfigurationPolicyReconciler) handleDeletion(plc *policyv1.Configurati
return nil
}

log := log.WithValues("policy", plc.GetName())

parentStatusUpdateNeeded := false

log.Info("Config policy has been deleted, handling child objects")
Expand Down Expand Up @@ -1044,6 +1048,8 @@ func (r *ConfigurationPolicyReconciler) handleDeletion(plc *policyv1.Configurati
func (r *ConfigurationPolicyReconciler) handleTemplatization(
plc *policyv1.ConfigurationPolicy, usingWatch bool,
) (parentStatusUpdateNeeded bool, err error) {
log := log.WithValues("policy", plc.GetName())

if r.EnableMetrics {
startTime := time.Now().UTC()

Expand Down Expand Up @@ -1131,13 +1137,14 @@ func (r *ConfigurationPolicyReconciler) handleTemplatization(

// process object templates for go template usage
for i, rawData := range rawDataList {
if !templates.HasTemplate(rawData, "", true) && !isRawObjTemplate {
hasTemplate := templates.HasTemplate(rawData, "", true)

if !hasTemplate && !isRawObjTemplate {
continue
}

if !templates.HasTemplate(rawData, "", true) {
// Unmarshal raw template YAML into object if that has not already been done by the template
// resolution function
if !hasTemplate {
// Unmarshal raw template YAML into object as it doesn't need template resolution
err := yaml.Unmarshal(rawData, &objTemps)
if err != nil {
addTemplateErrorViolation("Error parsing the YAML in the object-templates-raw field", err.Error())
Expand Down Expand Up @@ -1255,6 +1262,8 @@ func (r *ConfigurationPolicyReconciler) determineDesiredObject(
errEvent *objectTmplEvalEvent,
mappingErr error,
) {
log := log.WithValues("policy", plc.GetName())

_, _, err := unstructured.UnstructuredJSONScheme.Decode(objectT.ObjectDefinition.Raw, nil, &desiredObj)
if err != nil {
log.Error(err, "Could not decode the objectDefinition", "index", index)
Expand Down Expand Up @@ -1470,12 +1479,12 @@ func addConditionToStatus(
updateNeeded = true
} else {
oldCond := currentConds[len(currentConds)-1]
newConditionIsSimilar := reflect.DeepEqual(oldCond.Status, newCond.Status) &&
reflect.DeepEqual(oldCond.Reason, newCond.Reason) &&
reflect.DeepEqual(oldCond.Message, newCond.Message) &&
reflect.DeepEqual(oldCond.Type, newCond.Type)
newConditionIsSame := oldCond.Status == newCond.Status &&
oldCond.Reason == newCond.Reason &&
oldCond.Message == newCond.Message &&
oldCond.Type == newCond.Type

if !newConditionIsSimilar {
if !newConditionIsSame {
plc.Status.CompliancyDetails[index].Conditions[len(currentConds)-1] = *newCond
updateNeeded = true
}
Expand Down