Skip to content

Commit

Permalink
Use handleKeys for operator-policy
Browse files Browse the repository at this point in the history 
This allows the operator-policy to use some functionality from the
config-policy-controller in order to determine if the pieces specified
in an OperatorPolicy for a Subscription or OperatorGroup match what is
already on the cluster. For example, it handles things like defaulted
fields that are not specified by the user in the OperatorPolicy.

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
  • Loading branch information
@JustinKuli
JustinKuli committed Jan 30, 2024
1 parent 6454bb7 commit 4e78199
Show file tree
Hide file tree
Showing 4 changed files with 208 additions and 86 deletions.
132 changes: 75 additions & 57 deletions controllers/configurationpolicy_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2586,72 +2586,28 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource(
res = r.TargetK8sDynamicClient.Resource(obj.gvr)
}

updateSucceeded = false
// Use a copy since some values can be directly assigned to mergedObj in handleSingleKey.
existingObjectCopy := obj.existingObj.DeepCopy()
removeFieldsForComparison(existingObjectCopy)

var statusMismatch bool

for key := range obj.desiredObj.Object {
isStatus := key == "status"

// use metadatacompliancetype to evaluate metadata if it is set
keyComplianceType := complianceType
if key == "metadata" && mdComplianceType != "" {
keyComplianceType = mdComplianceType
}

// check key for mismatch
errorMsg, keyUpdateNeeded, mergedObj, skipped := handleSingleKey(
key, obj.desiredObj, existingObjectCopy, keyComplianceType, !r.DryRunSupported,
)
if errorMsg != "" {
log.Info(errorMsg)

return true, errorMsg, true, false
}

if mergedObj == nil && skipped {
continue
}

// only look at labels and annotations for metadata - configurationPolicies do not update other metadata fields
if key == "metadata" {
// if it's not the right type, the map will be empty
mdMap, _ := mergedObj.(map[string]interface{})
throwSpecViolation, message, updateNeeded, statusMismatch := handleKeys(
obj.desiredObj, obj.existingObj, existingObjectCopy, complianceType, mdComplianceType, !r.DryRunSupported,
)
if message != "" {
return true, message, true, false
}

// if either isn't found, they'll just be empty
mergedAnnotations, _, _ := unstructured.NestedStringMap(mdMap, "annotations")
mergedLabels, _, _ := unstructured.NestedStringMap(mdMap, "labels")
if updateNeeded {
mismatchLog := "Detected value mismatch"

obj.existingObj.SetAnnotations(mergedAnnotations)
obj.existingObj.SetLabels(mergedLabels)
} else {
obj.existingObj.UnstructuredContent()[key] = mergedObj
// Add a configuration breadcrumb for users that might be looking in the logs for a diff
if objectT.RecordDiff != policyv1.RecordDiffLog {
mismatchLog += " (Diff disabled. To log the diff, " +
"set 'spec.object-tempates[].recordDiff' to 'Log' for this object-template.)"
}

if keyUpdateNeeded {
if isStatus {
throwSpecViolation = true
statusMismatch = true

log.Info("Ignoring an update to the object status", "key", key)
} else {
updateNeeded = true
log.Info(mismatchLog)

mismatchLog := "Detected value mismatch for object key: " + key
// Add a configuration breadcrumb for users that might be looking in the logs for a diff
if objectT.RecordDiff != policyv1.RecordDiffLog {
mismatchLog += " (Diff disabled. To log the diff, " +
"set 'spec.object-tempates[].recordDiff' to 'Log' for this object-template.)"
}
log.Info(mismatchLog)
}
}
}

if updateNeeded {
// FieldValidation is supported in k8s 1.25 as beta release
// so if the version is below 1.25, we need to use client side validation to validate the object
if semver.Compare(r.serverVersion, "v1.25.0") < 0 {
Expand Down Expand Up @@ -2783,6 +2739,68 @@ func (r *ConfigurationPolicyReconciler) checkAndUpdateResource(
return throwSpecViolation, "", updateNeeded, updateSucceeded
}

// handleKeys goes through all of the fields in the desired object and checks if the existing object
// matches. When a field is a map or slice, the value in the existing object will be updated with
// the result of merging its current value with the desired value.
func handleKeys(
desiredObj unstructured.Unstructured,
existingObj *unstructured.Unstructured,
existingObjectCopy *unstructured.Unstructured,
compType string,
mdCompType string,
zeroValueEqualsNil bool,
) (throwSpecViolation bool, message string, updateNeeded bool, statusMismatch bool) {
for key := range desiredObj.Object {
isStatus := key == "status"

// use metadatacompliancetype to evaluate metadata if it is set
keyComplianceType := compType
if key == "metadata" && mdCompType != "" {
keyComplianceType = mdCompType
}

// check key for mismatch
errorMsg, keyUpdateNeeded, mergedObj, skipped := handleSingleKey(
key, desiredObj, existingObjectCopy, keyComplianceType, zeroValueEqualsNil,
)
if errorMsg != "" {
log.Info(errorMsg)

return true, errorMsg, true, statusMismatch
}

if mergedObj == nil && skipped {
continue
}

// only look at labels and annotations for metadata - configurationPolicies do not update other metadata fields
if key == "metadata" {
// if it's not the right type, the map will be empty
mdMap, _ := mergedObj.(map[string]interface{})

// if either isn't found, they'll just be empty
mergedAnnotations, _, _ := unstructured.NestedStringMap(mdMap, "annotations")
mergedLabels, _, _ := unstructured.NestedStringMap(mdMap, "labels")

existingObj.SetAnnotations(mergedAnnotations)
existingObj.SetLabels(mergedLabels)
} else {
existingObj.UnstructuredContent()[key] = mergedObj
}

if keyUpdateNeeded {
if isStatus {
throwSpecViolation = true
statusMismatch = true
} else {
updateNeeded = true
}
}
}

return
}

func removeFieldsForComparison(obj *unstructured.Unstructured) {
unstructured.RemoveNestedField(obj.Object, "metadata", "managedFields")
unstructured.RemoveNestedField(
Expand Down
131 changes: 110 additions & 21 deletions controllers/operatorpolicy_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package controllers
import (
"context"
"encoding/json"
"errors"
"fmt"
"reflect"

Expand All @@ -14,6 +15,7 @@ import (
depclient "github.com/stolostron/kubernetes-dependency-watches/client"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -173,22 +175,24 @@ func (r *OperatorPolicyReconciler) handleOpGroup(ctx context.Context, policy *po
case 1:
opGroup := foundOpGroups[0]

if policy.Spec.OperatorGroup == nil {
// There is one operator group in the namespace, but the policy doesn't specify what it should look like.
// FUTURE: check if the one operator group is compatible with the desired subscription.
// For an initial implementation, assume if an OperatorGroup already exists, then it's a good one.
err := r.updateStatus(ctx, policy, opGroupPreexistingCond, matchedObj(&opGroup))
if err != nil {
return fmt.Errorf("error updating the status for an OperatorGroup that matches: %w", err)
}
// Check if what's on the cluster matches what the policy wants (whether it's specified or not)

return nil
}

// OperatorGroup is specified in the policy, check if what's on the cluster matches.
emptyNameMatch := desiredOpGroup.Name == "" && opGroup.GetGenerateName() == desiredOpGroup.GenerateName

if !(opGroup.GetName() == desiredOpGroup.Name || emptyNameMatch) {
if policy.Spec.OperatorGroup == nil {
// The policy doesn't specify what the OperatorGroup should look like, but what is already
// there is not the default one the policy would create.
// FUTURE: check if the one operator group is compatible with the desired subscription.
// For an initial implementation, assume if an OperatorGroup already exists, then it's a good one.
err := r.updateStatus(ctx, policy, opGroupPreexistingCond, matchedObj(&opGroup))
if err != nil {
return fmt.Errorf("error updating the status for a pre-existing OperatorGroup: %w", err)
}

return nil
}

// There is an OperatorGroup in the namespace that does not match the name of what is in the policy.
// Just creating a new one would cause the "TooManyOperatorGroups" failure.
// So, just report a NonCompliant status.
Expand All @@ -209,9 +213,14 @@ func (r *OperatorPolicyReconciler) handleOpGroup(ctx context.Context, policy *po
return fmt.Errorf("error converting desired OperatorGroup to an Unstructured: %w", err)
}

// NOTE: this is more like mustonlyhave than musthave, and won't quite work.
// something from config-policy will be needed to check if the specs match.
if reflect.DeepEqual(desiredUnstruct["spec"], opGroup.Object["spec"]) {
updateNeeded, skipUpdate, err := r.mergeObjects(
ctx, desiredUnstruct, &opGroup, string(policy.Spec.ComplianceType),
)
if err != nil {
return fmt.Errorf("error checking if the OperatorGroup needs an update: %w", err)
}

if !updateNeeded {
// Everything relevant matches!
err := r.updateStatus(ctx, policy, matchesCond("OperatorGroup"), matchedObj(&opGroup))
if err != nil {
Expand All @@ -221,6 +230,30 @@ func (r *OperatorPolicyReconciler) handleOpGroup(ctx context.Context, policy *po
return nil
}

// Specs don't match.

if policy.Spec.OperatorGroup == nil {
// The policy doesn't specify what the OperatorGroup should look like, but what is already
// there is not the default one the policy would create.
// FUTURE: check if the one operator group is compatible with the desired subscription.
// For an initial implementation, assume if an OperatorGroup already exists, then it's a good one.
err := r.updateStatus(ctx, policy, opGroupPreexistingCond, matchedObj(&opGroup))
if err != nil {
return fmt.Errorf("error updating the status for a pre-existing OperatorGroup: %w", err)
}

return nil
}

if policy.Spec.RemediationAction.IsEnforce() && skipUpdate {
err = r.updateStatus(ctx, policy, mismatchCondUnfixable("OperatorGroup"), mismatchedObj(&opGroup))
if err != nil {
return fmt.Errorf("error updating status for an unenforceable mismatched OperatorGroup: %w", err)
}

return nil
}

// The names match, but the specs don't: report NonCompliance
err = r.updateStatus(ctx, policy, mismatchCond("OperatorGroup"), mismatchedObj(&opGroup))
if err != nil {
Expand Down Expand Up @@ -367,9 +400,12 @@ func (r *OperatorPolicyReconciler) handleSubscription(ctx context.Context, polic
return fmt.Errorf("error converting desired Subscription to an Unstructured: %w", err)
}

// NOTE: this is more like mustonlyhave than musthave, and won't quite work.
// something from config-policy will be needed to check if the specs match.
if reflect.DeepEqual(desiredUnstruct["spec"], foundSub.Object["spec"]) {
updateNeeded, skipUpdate, err := r.mergeObjects(ctx, desiredUnstruct, foundSub, string(policy.Spec.ComplianceType))
if err != nil {
return fmt.Errorf("error checking if the Subscription needs an update: %w", err)
}

if !updateNeeded {
// FUTURE: Check more details about the *status* of the Subscription
// For now, just mark it as compliant
err := r.updateStatus(ctx, policy, matchesCond("Subscription"), matchedObj(foundSub))
Expand All @@ -381,15 +417,22 @@ func (r *OperatorPolicyReconciler) handleSubscription(ctx context.Context, polic
}

// Specs don't match.
if policy.Spec.RemediationAction.IsEnforce() && skipUpdate {
err = r.updateStatus(ctx, policy, mismatchCondUnfixable("Subscription"), mismatchedObj(foundSub))
if err != nil {
return fmt.Errorf("error updating status for a mismatched Subscription that can't be enforced: %w", err)
}

return nil
}

err = r.updateStatus(ctx, policy, mismatchCond("Subscription"), mismatchedObj(foundSub))
if err != nil {
return fmt.Errorf("error updating status for a mismatched Subscription: %w", err)
}

if policy.Spec.RemediationAction.IsEnforce() {
desiredSub.ResourceVersion = foundSub.GetResourceVersion()

err := r.Update(ctx, desiredSub)
err := r.Update(ctx, foundSub)
if err != nil {
return fmt.Errorf("error updating the Subscription: %w", err)
}
Expand Down Expand Up @@ -448,3 +491,49 @@ func opPolIdentifier(namespace, name string) depclient.ObjectIdentifier {
Name: name,
}
}

// mergeObjects takes fields from the desired object and sets/merges them on the
// existing object. It checks and returns whether an update is really necessary
// with a server-side dry-run.
func (r *OperatorPolicyReconciler) mergeObjects(
ctx context.Context,
desired map[string]interface{},
existing *unstructured.Unstructured,
complianceType string,
) (updateNeeded, updateIsForbidden bool, err error) {
desiredObj := unstructured.Unstructured{Object: desired}

// Use a copy since some values can be directly assigned to mergedObj in handleSingleKey.
existingObjectCopy := existing.DeepCopy()
removeFieldsForComparison(existingObjectCopy)

_, errMsg, updateNeeded, _ := handleKeys(
desiredObj, existing, existingObjectCopy, complianceType, "", false,
)
if errMsg != "" {
return updateNeeded, false, errors.New(errMsg)
}

if updateNeeded {
err := r.Update(ctx, existing, client.DryRunAll)
if err != nil {
if k8serrors.IsForbidden(err) {
// This indicates the update would make a change, but the change is not allowed,
// for example, the changed field might be immutable.
// The policy should be marked as noncompliant, but an enforcement update would fail.
return true, true, nil
}

return updateNeeded, false, err
}

removeFieldsForComparison(existing)

if reflect.DeepEqual(existing.Object, existingObjectCopy.Object) {
// The dry run indicates that there is not *really* a mismatch.
updateNeeded = false
}
}

return updateNeeded, false, nil
}
11 changes: 11 additions & 0 deletions controllers/operatorpolicy_status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,17 @@ func mismatchCond(kind string) metav1.Condition {
}
}

// mismatchCondUnfixable returns a NonCompliant condition with a Reason like '____Mismatch',
// and a Message like 'the ____ found on the cluster does not match the policy and can't be enforced'
func mismatchCondUnfixable(kind string) metav1.Condition {
return metav1.Condition{
Type: condType(kind),
Status: metav1.ConditionFalse,
Reason: kind + "Mismatch",
Message: "the " + kind + " found on the cluster does not match the policy and can't be enforced",
}
}

// updatedCond returns a Compliant condition, with a Reason like'____Updated',
// and a Message like 'the ____ was updated to match the policy'
func updatedCond(kind string) metav1.Condition {
Expand Down
Loading

0 comments on commit 4e78199

Please sign in to comment.